Schedule posttask to decide Uma optin option offering

chrome/browser/ui/views/session_crashed_bubble_view.cc

Index: chrome/browser/ui/views/session_crashed_bubble_view.cc
diff --git a/chrome/browser/ui/views/session_crashed_bubble_view.cc b/chrome/browser/ui/views/session_crashed_bubble_view.cc
index 6fdd0a2bd455b5018d8e2023c09436bcf88e2a7a..dd60118cb78879d5a73e6f487be34bde2a4c69a5 100644
--- a/chrome/browser/ui/views/session_crashed_bubble_view.cc
+++ b/chrome/browser/ui/views/session_crashed_bubble_view.cc
@@ -6,10 +6,13 @@
 
 #include <vector>
 
+#include "base/bind.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/sessions/session_restore.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/options/options_util.h"
 #include "chrome/browser/ui/startup/session_crashed_bubble.h"
 #include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
@@ -20,6 +23,7 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/chromium_strings.h"
@@ -71,24 +75,88 @@ bool ShouldOfferMetricsReporting() {
 
 }  // namespace
 
+class SessionCrashedBubbleView::BrowserRemovalObserver

[sky, 2014/05/21 16:13:09]

Add description.

[yao, 2014/05/22 14:48:35]

Done.

+    : public chrome::BrowserListObserver {
+ public:
+  explicit BrowserRemovalObserver(Browser* browser);
+  ~BrowserRemovalObserver();

[sky, 2014/05/21 16:13:09]

virtual (because BrowserRemovalObserver subclasses BrowserListObserver).

[yao, 2014/05/22 14:48:35]

Done.

+
+  // Overridden from chrome::BrowserListObserver.
+  virtual void OnBrowserRemoved(Browser* browser) OVERRIDE;
+
+  Browser* browser() const;
+
+ private:
+  Browser* browser_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserRemovalObserver);
+};
+
+SessionCrashedBubbleView::BrowserRemovalObserver::BrowserRemovalObserver(
+    Browser* browser) : browser_(browser) {

[sky, 2014/05/21 16:13:09]

nit: ':' on next line (see style guide).

[yao, 2014/05/22 14:48:35]

Done.

+  DCHECK(browser_);
+  BrowserList::AddObserver(this);
+}
+
+SessionCrashedBubbleView::BrowserRemovalObserver::~BrowserRemovalObserver() {
+  BrowserList::RemoveObserver(this);
+}
+
+void SessionCrashedBubbleView::BrowserRemovalObserver::OnBrowserRemoved(
+    Browser* browser) {
+  if (browser == browser_)
+    browser_ = NULL;
+}
+
+Browser* SessionCrashedBubbleView::BrowserRemovalObserver::browser() const {
+  return browser_;
+}
+
 // static
 void SessionCrashedBubbleView::Show(Browser* browser) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (browser->profile()->IsOffTheRecord())
     return;
 
+  // Observes browser removal event and will be deallocated in ShowForReal.
+  BrowserRemovalObserver* browser_observer =
+      new BrowserRemovalObserver(browser);
+
+  // Schedule a task to run ShouldOfferMetricsReporting() on FILE thread, since
+  // GoogleUpdateSettings::GetCollectStatsConsent() does IO. Then, call
+  // SessionCrashedBubbleView::ShowForReal with the result.
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&ShouldOfferMetricsReporting),
+      base::Bind(&SessionCrashedBubbleView::ShowForReal, browser_observer));

[sky, 2014/05/21 16:13:09]

Pass a scoped_ptr to make ownership clear. 122 should be in a scoped_ptr too.

[Alexei Svitkine (slow), 2014/05/21 16:25:27]

Neat, I didn't realise you could do this, so I didn't suggest this.

But looks like it's indeed a thing - you just need to use
base::Passed(&browser_observer) (with browser_observer being a scoped_ptr).

[yao, 2014/05/22 14:48:35]

Done.

[yao, 2014/05/22 14:48:35]

Done.

+}
+
+// static
+void SessionCrashedBubbleView::ShowForReal(
+    BrowserRemovalObserver* browser_observer,
+    bool offer_uma_optin) {
+  Browser* browser = browser_observer->browser();
+  delete browser_observer;
+
+  if (!browser)
+    return;
+
   views::View* anchor_view =
       BrowserView::GetBrowserViewForBrowser(browser)->toolbar()->app_menu();
   content::WebContents* web_contents =
       browser->tab_strip_model()->GetActiveWebContents();
   SessionCrashedBubbleView* crash_bubble =
-      new SessionCrashedBubbleView(anchor_view, browser, web_contents);
+      new SessionCrashedBubbleView(anchor_view, browser, web_contents,
+                                   offer_uma_optin);
   views::BubbleDelegateView::CreateBubble(crash_bubble)->Show();
 }
 
 SessionCrashedBubbleView::SessionCrashedBubbleView(
     views::View* anchor_view,
     Browser* browser,
-    content::WebContents* web_contents)
+    content::WebContents* web_contents,
+    bool offer_uma_optin)
     : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
       content::WebContentsObserver(web_contents),
       browser_(browser),
@@ -96,6 +164,7 @@ SessionCrashedBubbleView::SessionCrashedBubbleView(
       restore_button_(NULL),
       close_(NULL),
       uma_option_(NULL),
+      offer_uma_optin_(offer_uma_optin),
       started_navigation_(false) {
   set_close_on_deactivate(false);
   registrar_.Add(
@@ -193,7 +262,7 @@ void SessionCrashedBubbleView::Init() {
   layout->AddPaddingRow(0, kMarginHeight);
 
   // Metrics reporting option.
-  if (ShouldOfferMetricsReporting())
+  if (offer_uma_optin_)
     CreateUmaOptinView(layout);
 
   set_color(kWhiteBackgroundColor);