Componentize sad_tab

components/sad_tab/sad_tab_helper.cc

Index: components/sad_tab/sad_tab_helper.cc
diff --git a/chrome/browser/ui/sad_tab_helper.cc b/components/sad_tab/sad_tab_helper.cc
similarity index 43%
rename from chrome/browser/ui/sad_tab_helper.cc
rename to components/sad_tab/sad_tab_helper.cc
index ab8a6e034ea0d62936f59147d5f36a64856817e1..3da1bdff310f0a8587bd652e248e94aa393d7877 100644
--- a/chrome/browser/ui/sad_tab_helper.cc
+++ b/components/sad_tab/sad_tab_helper.cc
@@ -2,20 +2,32 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/sad_tab_helper.h"
+#include "components/sad_tab/sad_tab_helper.h"
 
-#include "base/logging.h"
-#include "chrome/browser/browser_shutdown.h"
-#include "chrome/browser/ui/sad_tab.h"
-#include "content/public/browser/web_contents.h"
+#include "components/sad_tab/sad_tab.h"
+#include "components/sad_tab/sad_tab_client.h"
 
-DEFINE_WEB_CONTENTS_USER_DATA_KEY(SadTabHelper);
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(sad_tab::SadTabHelper);
+
+namespace sad_tab {
+
+// static
+void SadTabHelper::CreateForWebContentsWithClient(
+    content::WebContents* contents,
+    scoped_ptr<SadTabClient> client) {
+  if (!FromWebContents(contents)) {
+    contents->SetUserData(UserDataKey(),
+                          new SadTabHelper(contents, client.Pass()));
+  }
+}
 
 SadTabHelper::~SadTabHelper() {
 }
 
-SadTabHelper::SadTabHelper(content::WebContents* web_contents)
-    : content::WebContentsObserver(web_contents) {
+SadTabHelper::SadTabHelper(content::WebContents* web_contents,
+                           scoped_ptr<SadTabClient> client)
+    : content::WebContentsObserver(web_contents),
+      client_(client.Pass()) {
 }
 
 void SadTabHelper::RenderViewReady() {
@@ -29,20 +41,21 @@ void SadTabHelper::RenderProcessGone(base::TerminationStatus status) {
   // Only show the sad tab if we're not in browser shutdown, so that WebContents
   // objects that are not in a browser (e.g., HTML dialogs) and thus are
   // visible do not flash a sad tab page.
-  if (browser_shutdown::GetShutdownType() != browser_shutdown::NOT_VALID)
+  if (client_->IsInBrowserShutdown())
     return;
 
   if (sad_tab_)
     return;
 
-  if (chrome::SadTab::ShouldShow(status))
+  if (SadTab::ShouldShow(status))
     InstallSadTab(status);
 }
 
 void SadTabHelper::InstallSadTab(base::TerminationStatus status) {
-  chrome::SadTabKind kind =
-      (status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED) ?
-          chrome::SAD_TAB_KIND_KILLED : chrome::SAD_TAB_KIND_CRASHED;
-  sad_tab_.reset(chrome::SadTab::Create(web_contents(), kind));
+  SadTabKind kind = (status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED) ?
+      SAD_TAB_KIND_KILLED : SAD_TAB_KIND_CRASHED;
+  sad_tab_ = client_->CreateSadTab(web_contents(), kind);

[sadrul, 2014/09/22 16:48:22]

My suggestion would be to: (1) move SadTabCocoa inside
//components/sad_tab/cocoa/, (2) remove Client::CreateSadTab, (3) have
SadTab::Create() create the views/cocoa version as appropriate. This way, the
app (i.e. chrome, athena) only needs to create the SadTabHelper, and provide the
implementations of IsInBrowserShutdown() and ShowFeedbackPage(), instead of also
having to know about the internal implementations of SadTabViews, or provide an
implementation of it (as this patch does with SadTabCocoa). This way, we also
don't need extra code for ChromeSadTabClient on mac.

[hashimoto, 2014/09/26 08:43:35]

Done.

   sad_tab_->Show();
 }
+
+}  // namespace sad_tab