Schedule posttask to decide Uma optin option offering

chrome/browser/ui/views/session_crashed_bubble_view.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/session_crashed_bubble_view.h"
 
 #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"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/pref_names.h"
 #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"
 #include "grit/generated_resources.h"
 #include "grit/google_chrome_strings.h"
 #include "grit/ui_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/button/label_button.h"
@@ skipping 33 matching lines @@
       prefs::kMetricsReportingEnabled)->IsUserModifiable();
 #else
   return false;
 #endif  // defined(GOOGLE_CHROME_BUILD)
 }
 
 }  // namespace
 
 // 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),
       web_contents_(web_contents),
       restore_button_(NULL),
       close_(NULL),
       uma_option_(NULL),
+      offer_uma_optin_(offer_uma_optin),
       started_navigation_(false) {
   set_close_on_deactivate(false);
   registrar_.Add(
       this,
       chrome::NOTIFICATION_TAB_CLOSING,
       content::Source<content::NavigationController>(&(
           web_contents->GetController())));
   browser->tab_strip_model()->AddObserver(this);
 }
 
@@ skipping 77 matching lines @@
 
   layout->StartRow(0, kTextColumnSetId);
   layout->AddView(text_label);
   layout->AddPaddingRow(0, kMarginHeight);
 
   layout->StartRow(0, kButtonColumnSetId);
   layout->AddView(restore_button_);
   layout->AddPaddingRow(0, kMarginHeight);
 
   // Metrics reporting option.
-  if (ShouldOfferMetricsReporting())
+  if (offer_uma_optin_)
     CreateUmaOptinView(layout);
 
   set_color(kWhiteBackgroundColor);
   set_margins(gfx::Insets());
   Layout();
 }
 
 void SessionCrashedBubbleView::CreateUmaOptinView(GridLayout* layout) {
   // Checkbox for metric reporting setting.
   // Since the text to the right of the checkbox can't be a simple string (needs
@@ skipping 129 matching lines @@
 }
 
 void SessionCrashedBubbleView::CloseBubble() {
   GetWidget()->Close();
 }
 
 bool ShowSessionCrashedBubble(Browser* browser) {
   SessionCrashedBubbleView::Show(browser);
   return true;
 }