Revert 63005 - If default search is managed, we should not asked the user to choose it at First Run. Make sure the minimum bubble is not showed if there is no default search.

BUG=49306

TEST=Set a managed default search provider.  Clear your Chromium user data directory (~/Library/Chromium, ~/.config/chromium, %localappdata%\Chromium) and the "First Run" file found next to the executable.  Start Chrome.  It should not ask you to choose a default search provider.  Disable the default search provider via policy.  Make sure the minimal bubble is not shown. Redo these tests for the GOOGLE_CHROME_BUILD to make sure that we still ask about usage stats.

Review URL: http://codereview.chromium.org/3565013

TBR=jeanluc@google.com
Review URL: http://codereview.chromium.org/3747009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63016 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 51 insertions, 97 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index c26ff0f..76c94fd 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -1506,16 +1506,11 @@ int BrowserMain(const MainFunctionParams& parameters) {
   if (FirstRun::InSearchExperimentLocale() && record_search_engine) {
     const TemplateURL* default_search_engine =
         profile->GetTemplateURLModel()->GetDefaultSearchProvider();
-    // The default engine can be NULL if the administrator has disabled
-    // default search.
-    TemplateURLPrepopulateData::SearchEngineType search_engine_type =
-        default_search_engine ? default_search_engine->search_engine_type() :
-                                TemplateURLPrepopulateData::SEARCH_ENGINE_OTHER;
     // Record the search engine chosen.
     if (master_prefs.run_search_engine_experiment) {
       UMA_HISTOGRAM_ENUMERATION(
           "Chrome.SearchSelectExperiment",
-          search_engine_type,
+          default_search_engine->search_engine_type(),
           TemplateURLPrepopulateData::SEARCH_ENGINE_MAX);
       // If the selection has been randomized, also record the winner by slot.
       if (master_prefs.randomize_search_engine_experiment) {
@@ -1527,7 +1522,7 @@ int BrowserMain(const MainFunctionParams& parameters) {
           experiment_type.push_back('1' + engine_pos);
           UMA_HISTOGRAM_ENUMERATION(
               experiment_type,
-              search_engine_type,
+              default_search_engine->search_engine_type(),
               TemplateURLPrepopulateData::SEARCH_ENGINE_MAX);
         } else {
           NOTREACHED() << "Invalid search engine selection slot.";
@@ -1536,7 +1531,7 @@ int BrowserMain(const MainFunctionParams& parameters) {
     } else {
       UMA_HISTOGRAM_ENUMERATION(
           "Chrome.SearchSelectExempt",
-          search_engine_type,
+          default_search_engine->search_engine_type(),
           TemplateURLPrepopulateData::SEARCH_ENGINE_MAX);
     }
   }
diff --git a/chrome/browser/cocoa/search_engine_list_model.mm b/chrome/browser/cocoa/search_engine_list_model.mm
index c235fc0..802a667 100644
--- a/chrome/browser/cocoa/search_engine_list_model.mm
+++ b/chrome/browser/cocoa/search_engine_list_model.mm
@@ -87,7 +87,7 @@ class SearchEngineObserver : public TemplateURLModelObserver {
 
 // The index into |-searchEngines| of the current default search engine.
 - (NSUInteger)defaultIndex {
-  if (!model_) return -1;
+  if (!model_) return 0;
 
   NSUInteger index = 0;
   const TemplateURL* defaultSearchProvider = model_->GetDefaultSearchProvider();
@@ -103,7 +103,7 @@ class SearchEngineObserver : public TemplateURLModelObserver {
         ++index;
     }
   }
-  return -1;
+  return 0;
 }
 
 - (void)setDefaultIndex:(NSUInteger)index {
diff --git a/chrome/browser/first_run/first_run.cc b/chrome/browser/first_run/first_run.cc
index 663c52d..596419f 100644
--- a/chrome/browser/first_run/first_run.cc
+++ b/chrome/browser/first_run/first_run.cc
@@ -22,7 +22,6 @@
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/process_singleton.h"
 #include "chrome/browser/profile_manager.h"
-#include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -565,7 +564,7 @@ void FirstRun::AutoImport(
   UserMetrics::RecordAction(UserMetricsAction("FirstRunDef_Accept"));
 
   // Launch the search engine dialog only if build is organic, and user has not
-  // already set preferences.
+  // already set search preferences.
   if (IsOrganic() && !local_state_file_exists) {
     // The home page string may be set in the preferences, but the user should
     // initially use Chrome with the NTP as home page in organic builds.
@@ -576,14 +575,9 @@ void FirstRun::AutoImport(
   if (make_chrome_default)
     ShellIntegration::SetAsDefaultBrowser();
 
-  // Don't display the minimal bubble if there is no default search provider.
-  TemplateURLModel* search_engines_model = profile->GetTemplateURLModel();
-  if (search_engines_model &&
-      search_engines_model->GetDefaultSearchProvider()) {
-    FirstRun::SetShowFirstRunBubblePref(true);
-    // Set the first run bubble to minimal.
-    FirstRun::SetMinimalFirstRunBubblePref();
-  }
+  FirstRun::SetShowFirstRunBubblePref(true);
+  // Set the first run bubble to minimal.
+  FirstRun::SetMinimalFirstRunBubblePref();
   FirstRun::SetShowWelcomePagePref();
   FirstRun::SetPersonalDataManagerFirstRunPref();
 
diff --git a/chrome/browser/first_run/first_run_mac.mm b/chrome/browser/first_run/first_run_mac.mm
index b297d15..3159e9d 100644
--- a/chrome/browser/first_run/first_run_mac.mm
+++ b/chrome/browser/first_run/first_run_mac.mm
@@ -9,8 +9,6 @@
 #import "chrome/browser/cocoa/first_run_dialog.h"
 #import "chrome/browser/cocoa/search_engine_dialog_controller.h"
 #include "chrome/browser/prefs/pref_service.h"
-#include "chrome/browser/profile.h"
-#include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/installer/util/google_update_constants.h"
@@ -73,12 +71,7 @@ void ShowFirstRun(Profile* profile) {
   FirstRun::CreateSentinel();
 
   // Set preference to show first run bubble and welcome page.
-  // Don't display the minimal bubble if there is no default search provider.
-  TemplateURLModel* search_engines_model = profile->GetTemplateURLModel();
-  if (search_engines_model &&
-      search_engines_model->GetDefaultSearchProvider()) {
-    FirstRun::SetShowFirstRunBubblePref(true);
-  }
+  FirstRun::SetShowFirstRunBubblePref(true);
   FirstRun::SetShowWelcomePagePref();
 }
 
@@ -87,13 +80,8 @@ void ShowFirstRun(Profile* profile) {
 // static
 void FirstRun::ShowFirstRunDialog(Profile* profile,
                                   bool randomize_search_engine_experiment) {
-  // If the default search is not managed via policy, ask the user to
-  // choose a default.
-  TemplateURLModel* model = profile->GetTemplateURLModel();
-  if (model && !model->is_default_search_managed()) {
-    ShowSearchEngineSelectionDialog(profile,
-                                    randomize_search_engine_experiment);
-  }
+  ShowSearchEngineSelectionDialog(profile,
+                                  randomize_search_engine_experiment);
   ShowFirstRun(profile);
 }
 
diff --git a/chrome/browser/first_run/first_run_win.cc b/chrome/browser/first_run/first_run_win.cc
index a7f7a3c..4f9175d 100644
--- a/chrome/browser/first_run/first_run_win.cc
+++ b/chrome/browser/first_run/first_run_win.cc
@@ -31,7 +31,6 @@
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/process_singleton.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/views/first_run_search_engine_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_service.h"
@@ -454,12 +453,6 @@ bool FirstRun::IsOrganic() {
 // static
 void FirstRun::ShowFirstRunDialog(Profile* profile,
                                   bool randomize_search_engine_experiment) {
-  // If the default search is managed via policy, we don't ask the user to
-  // choose.
-  TemplateURLModel* model = profile->GetTemplateURLModel();
-  if (NULL == model || model->is_default_search_managed())
-    return;
-
   views::Window* search_engine_dialog = views::Window::CreateChromeWindow(
       NULL,
       gfx::Rect(),
diff --git a/chrome/browser/gtk/first_run_dialog.cc b/chrome/browser/gtk/first_run_dialog.cc
index 45b0589..c30da15 100644
--- a/chrome/browser/gtk/first_run_dialog.cc
+++ b/chrome/browser/gtk/first_run_dialog.cc
@@ -86,32 +86,9 @@ void SetWelcomePosition(GtkFloatingContainer* container,
 // static
 bool FirstRunDialog::Show(Profile* profile,
                           bool randomize_search_engine_order) {
-  // Figure out which dialogs we will show.
-  // If the default search is managed via policy, we won't ask.
-  const TemplateURLModel* search_engines_model = profile->GetTemplateURLModel();
-  bool show_search_engines_dialog = search_engines_model &&
-      !search_engines_model->is_default_search_managed();
-
-#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();
-#else
-  bool show_reporting_dialog = false;
-#endif
-
-  if (!show_search_engines_dialog && !show_reporting_dialog)
-    return true;  // Nothing to do
-
   int response = -1;
   // Object deletes itself.
-  new FirstRunDialog(profile,
-                     show_reporting_dialog,
-                     show_search_engines_dialog,
-                     &response);
+  new FirstRunDialog(profile, randomize_search_engine_order, response);
 
   // TODO(port): it should be sufficient to just run the dialog:
   // int response = gtk_dialog_run(GTK_DIALOG(dialog));
@@ -124,23 +101,16 @@ bool FirstRunDialog::Show(Profile* profile,
 }
 
 FirstRunDialog::FirstRunDialog(Profile* profile,
-                               bool show_reporting_dialog,
-                               bool show_search_engines_dialog,
-                               int* response)
+                               bool randomize_search_engine_order,
+                               int& response)
     : search_engine_window_(NULL),
       dialog_(NULL),
       report_crashes_(NULL),
       make_default_(NULL),
       profile_(profile),
       chosen_search_engine_(NULL),
-      show_reporting_dialog_(show_reporting_dialog),
       response_(response) {
-  if (!show_search_engines_dialog) {
-    ShowReportingDialog();
-    return;
-  }
   search_engines_model_ = profile_->GetTemplateURLModel();
-
   ShowSearchEngineWindow();
 
   search_engines_model_->AddObserver(this);
@@ -221,17 +191,21 @@ void FirstRunDialog::ShowSearchEngineWindow() {
   gtk_window_present(GTK_WINDOW(search_engine_window_));
 }
 
-void FirstRunDialog::ShowReportingDialog() {
+void FirstRunDialog::ShowDialog() {
   // 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_) {
+  const PrefService::Preference* metrics_reporting_pref =
+      g_browser_process->local_state()->FindPreference(
+          prefs::kMetricsReportingEnabled);
+  if (metrics_reporting_pref && metrics_reporting_pref->IsManaged()) {
     OnResponseDialog(NULL, GTK_RESPONSE_ACCEPT);
     return;
   }
 
+#if defined(GOOGLE_CHROME_BUILD)
   dialog_ = gtk_dialog_new_with_buttons(
       l10n_util::GetStringUTF8(IDS_FIRSTRUN_DLG_TITLE).c_str(),
       NULL,  // No parent
@@ -277,6 +251,10 @@ void FirstRunDialog::ShowReportingDialog() {
   g_signal_connect(dialog_, "response",
                    G_CALLBACK(OnResponseDialogThunk), this);
   gtk_widget_show_all(dialog_);
+#else  // !defined(GOOGLE_CHROME_BUILD)
+  // We don't show the dialog in chromium. Pretend the user accepted.
+  OnResponseDialog(NULL, GTK_RESPONSE_ACCEPT);
+#endif  // !defined(GOOGLE_CHROME_BUILD)
 }
 
 void FirstRunDialog::OnTemplateURLModelChanged() {
@@ -370,7 +348,7 @@ void FirstRunDialog::OnSearchEngineWindowDestroy(GtkWidget* sender) {
   search_engine_window_ = NULL;
   if (chosen_search_engine_) {
     search_engines_model_->SetDefaultSearchProvider(chosen_search_engine_);
-    ShowReportingDialog();
+    ShowDialog();
   } else {
     FirstRunDone();
   }
@@ -379,7 +357,7 @@ void FirstRunDialog::OnSearchEngineWindowDestroy(GtkWidget* sender) {
 void FirstRunDialog::OnResponseDialog(GtkWidget* widget, int response) {
   if (dialog_)
     gtk_widget_hide_all(dialog_);
-  *response_ = response;
+  response_ = response;
 
   // Mark that first run has ran.
   FirstRun::CreateSentinel();
diff --git a/chrome/browser/gtk/first_run_dialog.h b/chrome/browser/gtk/first_run_dialog.h
index 234a64b..fdd47a7 100644
--- a/chrome/browser/gtk/first_run_dialog.h
+++ b/chrome/browser/gtk/first_run_dialog.h
@@ -25,9 +25,8 @@ class FirstRunDialog : public TemplateURLModelObserver {
 
  private:
   FirstRunDialog(Profile* profile,
-                 bool show_reporting_dialog,
-                 bool show_search_engines_dialog,
-                 int* response);
+                 bool randomize_search_engine_order,
+                 int& response);
   virtual ~FirstRunDialog();
 
   CHROMEGTK_CALLBACK_1(FirstRunDialog, void, OnResponseDialog, int);
@@ -36,7 +35,7 @@ class FirstRunDialog : public TemplateURLModelObserver {
   CHROMEG_CALLBACK_0(FirstRunDialog, void, OnLearnMoreLinkClicked, GtkButton*);
 
   void ShowSearchEngineWindow();
-  void ShowReportingDialog();
+  void ShowDialog();
 
   // This method closes the first run window and quits the message loop so that
   // the Chrome startup can continue. This should be called when all the
@@ -69,12 +68,8 @@ class FirstRunDialog : public TemplateURLModelObserver {
   // search engine.
   TemplateURL* chosen_search_engine_;
 
-  // Whether we should show the dialog asking the user whether to report
-  // crashes and usage stats.
-  bool show_reporting_dialog_;
-
   // User response (accept or cancel) is returned through this.
-  int* response_;
+  int& response_;
 
   DISALLOW_COPY_AND_ASSIGN(FirstRunDialog);
 };
diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc
index e7bcb92..4d0d33c 100644
--- a/chrome/browser/search_engines/template_url_model.cc
+++ b/chrome/browser/search_engines/template_url_model.cc
@@ -803,8 +803,10 @@ bool TemplateURLModel::LoadDefaultSearchProviderFromPrefs(
   if (!prefs || !prefs->HasPrefPath(prefs::kDefaultSearchProviderSearchURL))
     return false;
 
+  // By default, kDefaultSearchProviderEnabled is true.  Users of previous
+  // versions will transition correctly.
   const PrefService::Preference* pref =
-      prefs->FindPreference(prefs::kDefaultSearchProviderSearchURL);
+      prefs->FindPreference(prefs::kDefaultSearchProviderEnabled);
   *is_managed = pref && pref->IsManaged();
 
   bool enabled =
diff --git a/chrome/browser/search_engines/template_url_model_unittest.cc b/chrome/browser/search_engines/template_url_model_unittest.cc
index 15c4b12..7f4dc72 100644
--- a/chrome/browser/search_engines/template_url_model_unittest.cc
+++ b/chrome/browser/search_engines/template_url_model_unittest.cc
@@ -186,8 +186,7 @@ class TemplateURLModelTest : public testing::Test {
 
   // Set the managed preferences for the default search provider and trigger
   // notification.
-  void SetManagedDefaultSearchPreferences(bool enabled,
-                                          const char* name,
+  void SetManagedDefaultSearchPreferences(const char* name,
                                           const char* search_url,
                                           const char* suggest_url,
                                           const char* icon_url,
@@ -196,7 +195,7 @@ class TemplateURLModelTest : public testing::Test {
     TestingPrefService* service = profile()->GetTestingPrefService();
     service->SetManagedPrefWithoutNotification(
         prefs::kDefaultSearchProviderEnabled,
-        Value::CreateBooleanValue(enabled));
+        Value::CreateBooleanValue(true));
     service->SetManagedPrefWithoutNotification(
         prefs::kDefaultSearchProviderName,
         Value::CreateStringValue(name));
@@ -223,6 +222,16 @@ class TemplateURLModelTest : public testing::Test {
     NotifyManagedPrefsHaveChanged();
   }
 
+  // Set the managed preferences for the default search provider and trigger
+  // notification.
+  void DisableManagedDefaultSearchProvider() {
+    TestingPrefService* service = profile()->GetTestingPrefService();
+    service->SetManagedPrefWithoutNotification(
+        prefs::kDefaultSearchProviderEnabled,
+        Value::CreateBooleanValue(false));
+    NotifyManagedPrefsHaveChanged();
+  }
+
   // Remove all the managed preferences for the default search provider and
   // trigger notification.
   void RemoveManagedDefaultSearchPreferences() {
@@ -1111,7 +1120,7 @@ TEST_F(TemplateURLModelTest, TestManagedDefaultSearch) {
   const char kSearchURL[] = "http://test.com/search?t={searchTerms}";
   const char kIconURL[] = "http://test.com/icon.jpg";
   const char kEncodings[] = "UTF-16;UTF-32";
-  SetManagedDefaultSearchPreferences(true, kName, kSearchURL, "", kIconURL,
+  SetManagedDefaultSearchPreferences(kName, kSearchURL, "", kIconURL,
                                      kEncodings, "");
   VerifyObserverCount(1);
   EXPECT_TRUE(model()->is_default_search_managed());
@@ -1135,8 +1144,8 @@ TEST_F(TemplateURLModelTest, TestManagedDefaultSearch) {
   const char kNewName[] = "test2";
   const char kNewSearchURL[] = "http://other.com/search?t={searchTerms}";
   const char kNewSuggestURL[] = "http://other.com/suggest?t={searchTerms}";
-  SetManagedDefaultSearchPreferences(true, kNewName, kNewSearchURL,
-                                     kNewSuggestURL, "", "", "");
+  SetManagedDefaultSearchPreferences(kNewName, kNewSearchURL, kNewSuggestURL,
+                                     "", "", "");
   VerifyObserverCount(1);
   EXPECT_TRUE(model()->is_default_search_managed());
   EXPECT_EQ(2 + initial_count, model()->GetTemplateURLs().size());
@@ -1165,14 +1174,14 @@ TEST_F(TemplateURLModelTest, TestManagedDefaultSearch) {
   EXPECT_EQ(actual_final_managed_default->show_in_default_list(), true);
 
   // Disable the default search provider through policy.
-  SetManagedDefaultSearchPreferences(false, "", "", "", "", "", "");
+  DisableManagedDefaultSearchProvider();
   VerifyObserverCount(1);
   EXPECT_TRUE(model()->is_default_search_managed());
   EXPECT_TRUE(NULL == model()->GetDefaultSearchProvider());
   EXPECT_EQ(1 + initial_count, model()->GetTemplateURLs().size());
 
   // Re-enable it.
-  SetManagedDefaultSearchPreferences(true, kName, kSearchURL, "", kIconURL,
+  SetManagedDefaultSearchPreferences(kName, kSearchURL, "", kIconURL,
                                      kEncodings, "");
   VerifyObserverCount(1);
   EXPECT_TRUE(model()->is_default_search_managed());