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..fb88d2707cd0edf2db8415803440d87c7c9973a8 100644
--- a/chrome/browser/ui/views/session_crashed_bubble_view.cc
+++ b/chrome/browser/ui/views/session_crashed_bubble_view.cc
@@ -6,10 +6,12 @@
 
 #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/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 +22,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"
@@ -73,22 +76,48 @@ bool ShouldOfferMetricsReporting() {
 
 // static
 void SessionCrashedBubbleView::Show(Browser* browser) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
   if (browser->profile()->IsOffTheRecord())
     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);
-  views::BubbleDelegateView::CreateBubble(crash_bubble)->Show();
+  // 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,
+                 browser->host_desktop_type()));
+}
+
+// static
+void SessionCrashedBubbleView::ShowForReal(Browser* browser,
+                                           chrome::HostDesktopType type,
+                                           bool offer_uma_optin) {
+  // Check if |browser| is still live.
+  BrowserList* browser_list = BrowserList::GetInstance(type);

[sky, 2014/05/15 22:57:38]

This is not a good way to check if the browser is still valid. For example,
tcmalloc makes it easy for deletes rapidly followed by a new to get the same
address. Additionally the browser may be in the process of being deleted so that
if you attempt to use it weird things may result.

Instead of passing a raw browser* pass around an object that contains the
browser and listens for destruction. That way if it gets here and the browser is
no longer valid you'll know.

[yao, 2014/05/20 15:48:40]

Done.

+  for (BrowserList::const_iterator it = browser_list->begin();
+       it != browser_list->end(); ++it ) {

[Alexei Svitkine (slow), 2014/05/15 22:50:49]

Can you use std:find() on browser_list->begin() and browser_list->end() instead
and do an early return otherwise?

[yao, 2014/05/20 15:48:40]

Done.

+    if (*it == browser) {
+      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,
+                                       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 +125,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 +223,7 @@ void SessionCrashedBubbleView::Init() {
   layout->AddPaddingRow(0, kMarginHeight);
 
   // Metrics reporting option.
-  if (ShouldOfferMetricsReporting())
+  if (offer_uma_optin_)
     CreateUmaOptinView(layout);
 
   set_color(kWhiteBackgroundColor);