Record first run startup metrics.

chrome/browser/ui/gtk/first_run_dialog.cc

 // Copyright (c) 2012 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/gtk/first_run_dialog.h"
 
 #include <string>
 #include <vector>
 
 #include "base/i18n/rtl.h"
@@ skipping 50 matching lines @@
   g_value_set_int(&value, y);
   gtk_container_child_set_property(GTK_CONTAINER(container),
                                    label, "y", &value);
   g_value_unset(&value);
 }
 
 }  // namespace
 
 namespace first_run {
 
-void ShowFirstRunDialog(Profile* profile) {
-  FirstRunDialog::Show();
+bool ShowFirstRunDialog(Profile* profile) {
+  return FirstRunDialog::Show();
 }
 
 }  // namespace first_run
 
 // static
 bool FirstRunDialog::Show() {
-#if !defined(GOOGLE_CHROME_BUILD)
-  return true;  // Nothing to do
-#else
+  bool dialog_shown = false;
+#if defined(GOOGLE_CHROME_BUILD)
   // If the metrics reporting is managed, we won't ask.
   const PrefService::Preference* metrics_reporting_pref =
       g_browser_process->local_state()->FindPreference(
           prefs::kMetricsReportingEnabled);
   bool show_reporting_dialog = !metrics_reporting_pref ||
       !metrics_reporting_pref->IsManaged();
 
-  if (!show_reporting_dialog)
-    return true;  // Nothing to do
+  if (show_reporting_dialog) {
+    // Object deletes itself.
+    new FirstRunDialog();
+    dialog_shown = true;
 
-  int response = -1;
-  // Object deletes itself.
-  new FirstRunDialog(show_reporting_dialog, &response);
-
-  // TODO(port): it should be sufficient to just run the dialog:
-  // int response = gtk_dialog_run(GTK_DIALOG(dialog));
-  // but that spins a nested message loop and hoses us.  :(
-  // http://code.google.com/p/chromium/issues/detail?id=12552
-  // Instead, run a loop and extract the response manually.
-  MessageLoop::current()->Run();
-
-  return (response == GTK_RESPONSE_ACCEPT);
+    // TODO(port): it should be sufficient to just run the dialog:
+    // int response = gtk_dialog_run(GTK_DIALOG(dialog));
+    // but that spins a nested message loop and hoses us.  :(
+    // http://code.google.com/p/chromium/issues/detail?id=12552
+    // Instead, run a loop directly here.
+    MessageLoop::current()->Run();
+  }

[Nico, 2013/05/06 21:15:09]

Is this right? Previously the dialog would call OnResponseDialog() immediately,
which would call first_run::SetShouldShowWelcomePage() and
first_run::CreateSentinel() – is this no longer necessary?

[gab, 2013/05/06 21:28:36]

The response wasn't used previously.

This method also only creates a FirstRunDialog if |show_reporting_dialog| so
that passing it in the constructor was superfluous.

first_run::CreateSentinel() is not required here, but I realize that this is the
case in other places in the code and I'll fix that in a separate CL instead of
squeezing it in here (added it back to this CL).

[Nico, 2013/05/06 21:31:50]

No, but it might've been called for its side effects (such as creating the
sentinel file and setting up the welcome page. Now these side effect aren't
done.

[gab, 2013/05/06 21:40:10]

Hmmm, I only removed the storing of |response| into |response_|
OnResponseDialog() is still called, no? Is there some subtle GTK magic I don't
see here?

[Nico, 2013/05/06 21:55:40]

Ah, I didn't see tge early return if (!show_reporting_dialog) slightly higher
up. What threw me off is that FirstRunDialog::ShowReportingDialog() checks if
(!show_reporting_dialog) too, but due to the early exit that branch will never
be taken. +mnissler who wrote the lengthy-ish comment about this block of dead
code, +jeanluc which added the early exit here in
https://codereview.chromium.org/3847006 and +msw who touched it somewhat
recently (but doesn't really look all that related:
https://codereview.chromium.org/9285002)

[Mattias Nissler (ping if slow), 2013/05/07 08:27:51]

I suspect I wrote that comment back in a time when the check in line 126 wasn't
dead code (or at least I hope so). The new code looks fine from my side.

 #endif  // defined(GOOGLE_CHROME_BUILD)
+  return dialog_shown;
 }
 
-FirstRunDialog::FirstRunDialog(bool show_reporting_dialog, int* response)
+FirstRunDialog::FirstRunDialog()
     : dialog_(NULL),
       report_crashes_(NULL),
-      make_default_(NULL),
-      show_reporting_dialog_(show_reporting_dialog),
-      response_(response) {
+      make_default_(NULL) {
   ShowReportingDialog();
 }
 
 FirstRunDialog::~FirstRunDialog() {
 }
 
 void FirstRunDialog::ShowReportingDialog() {
-  // The purpose of the dialog is to ask the user to enable stats and crash
-  // reporting. This setting may be controlled through configuration management
-  // in enterprise scenarios. If that is the case, skip the dialog entirely,
-  // it's not worth bothering the user for only the default browser question
-  // (which is likely to be forced in enterprise deployments anyway).
-  if (!show_reporting_dialog_) {
-    OnResponseDialog(NULL, GTK_RESPONSE_ACCEPT);
-    return;
-  }
-
   dialog_ = gtk_dialog_new_with_buttons(
       l10n_util::GetStringUTF8(IDS_FIRSTRUN_DLG_TITLE).c_str(),
       NULL,  // No parent
       (GtkDialogFlags) (GTK_DIALOG_MODAL | GTK_DIALOG_NO_SEPARATOR),
       NULL);
   gtk_util::AddButtonToDialog(dialog_,
       l10n_util::GetStringUTF8(IDS_FIRSTRUN_DLG_OK).c_str(),
       GTK_STOCK_APPLY, GTK_RESPONSE_ACCEPT);
   gtk_window_set_deletable(GTK_WINDOW(dialog_), FALSE);
 
@@ skipping 30 matching lines @@
   gtk_box_pack_start(GTK_BOX(content_area), learn_more_vbox, FALSE, FALSE, 0);
 
   g_signal_connect(dialog_, "response",
                    G_CALLBACK(OnResponseDialogThunk), this);
   gtk_widget_show_all(dialog_);
 }
 
 void FirstRunDialog::OnResponseDialog(GtkWidget* widget, int response) {
   if (dialog_)
     gtk_widget_hide_all(dialog_);
-  *response_ = response;
-
-  // Mark that first run has ran.
-  first_run::CreateSentinel();
 
   // Check if user has opted into reporting.
   if (report_crashes_ &&
       gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(report_crashes_))) {
 #if defined(USE_LINUX_BREAKPAD)
     if (GoogleUpdateSettings::SetCollectStatsConsent(true))
       InitCrashReporter();
 #endif
   } else {
     GoogleUpdateSettings::SetCollectStatsConsent(false);
@@ skipping 13 matching lines @@
 }
 
 void FirstRunDialog::FirstRunDone() {
   first_run::SetShouldShowWelcomePage();
 
   if (dialog_)
     gtk_widget_destroy(dialog_);
   MessageLoop::current()->Quit();
   delete this;
 }