Some first run fixes:

Add missing first run bubble back to opening page.
Wait for search engines to load before displaying first run bubble, so correct search engine is displayed in first run.
Because some distributions use "skip-first-run-ui", add back the "import_items" code that passes values to this code path.  Because this code path is obsolete (most import is silent), this needs to be refactored in the long term.
Fix some spelling issues.

BUG= 42612
TEST= import with --skip-first-run-ui works 


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53198
Review URL: http://codereview.chromium.org/3042010

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

3 files changed, 80 insertions, 61 deletions

diff --git a/chrome/browser/first_run_win.cc b/chrome/browser/first_run_win.cc
index ed43854..32866ff 100644
--- a/chrome/browser/first_run_win.cc
+++ b/chrome/browser/first_run_win.cc
@@ -163,15 +163,15 @@ bool WriteEULAtoTempFile(FilePath* eula_path) {
 }
 
 // Helper class that performs delayed first-run tasks that need more of the
-// chrome infrastructure to be up an running before they can be attempted.
-class FirsRunDelayedTasks : public NotificationObserver {
+// chrome infrastructure to be up and running before they can be attempted.
+class FirstRunDelayedTasks : public NotificationObserver {
  public:
   enum Tasks {
     NO_TASK,
     INSTALL_EXTENSIONS
   };
 
-  explicit FirsRunDelayedTasks(Tasks task) {
+  explicit FirstRunDelayedTasks(Tasks task) {
     if (task == INSTALL_EXTENSIONS) {
       registrar_.Add(this, NotificationType::EXTENSIONS_READY,
                      NotificationService::AllSources());
@@ -192,7 +192,7 @@ class FirsRunDelayedTasks : public NotificationObserver {
 
  private:
   // Private ctor forces it to be created only in the heap.
-  ~FirsRunDelayedTasks() {}
+  ~FirstRunDelayedTasks() {}
 
   // The extension work is to basically trigger an extension update check.
   // If the extension specified in the master pref is older than the live
@@ -304,24 +304,26 @@ bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
   DictionaryValue* extensions = 0;
   if (installer_util::HasExtensionsBlock(prefs.get(), &extensions)) {
     LOG(INFO) << "Extensions block found in master preferences";
-    new FirsRunDelayedTasks(FirsRunDelayedTasks::INSTALL_EXTENSIONS);
+    new FirstRunDelayedTasks(FirstRunDelayedTasks::INSTALL_EXTENSIONS);
   }
 
-  // Add a special exception for import_search_engine preference.
-  // Even though we skip all other import_* preferences below, if
-  // skip-first-run-ui is not specified, we make exception for this one
-  // preference.
+  // If the search engine dialog is not shown, the search engine is set to
+  // Google unless master_preferences specifically turns on search engine
+  // import.
   int import_items = 0;
   if (installer_util::GetDistroBooleanPreference(prefs.get(),
-      installer_util::master_preferences::kDistroImportSearchPref, &value)) {
-    if (value) {
-      import_items += importer::SEARCH_ENGINES;
-      out_prefs->do_import_items += importer::SEARCH_ENGINES;
-    } else {
-      out_prefs->dont_import_items += importer::SEARCH_ENGINES;
-    }
+      installer_util::master_preferences::kDistroImportSearchPref, &value)
+      && value) {
+    import_items |= importer::SEARCH_ENGINES;
+    out_prefs->do_import_items |= importer::SEARCH_ENGINES;
   }
 
+  // Check to see if search engine logos should be randomized.
+  if (installer_util::GetDistroBooleanPreference(prefs.get(),
+      installer_util::master_preferences::kSearchEngineExperimentRandomizePref,
+      &value) && value)
+    out_prefs->randomize_search_engine_experiment = true;
+
   // If we're suppressing the first-run bubble, set that preference now.
   // Otherwise, wait until the user has completed first run to set it, so the
   // user is guaranteed to see the bubble iff he or she has completed the first
@@ -331,45 +333,42 @@ bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
       &value) && value)
     FirstRun::SetShowFirstRunBubblePref(false);
 
-  if (InSearchExperimentLocale() &&
-      installer_util::GetDistroBooleanPreference(prefs.get(),
-      installer_util::master_preferences::kSearchEngineExperimentPref,
-      &value) && value) {
-    // Set the first run dialog to include the search choice window.
-    out_prefs->run_search_engine_experiment = true;
-    // Check to see if search engine logos should be randomized.
-    if (installer_util::GetDistroBooleanPreference(prefs.get(),
-        installer_util::master_preferences::
-            kSearchEngineExperimentRandomizePref,
-        &value) && value) {
-      out_prefs->randomize_search_engine_experiment = true;
-    }
-    // Set the first run bubble to minimal.
-    FirstRun::SetMinimalFirstRunBubblePref();
-  }
 
-  // History is imported automatically, unless turned off in master_prefs.
   if (installer_util::GetDistroBooleanPreference(prefs.get(),
-      installer_util::master_preferences::kDistroImportHistoryPref, &value)
-      && !value) {
-    out_prefs->dont_import_items |= importer::HISTORY;
+      installer_util::master_preferences::kDistroImportHistoryPref, &value)) {
+    if (value) {
+      import_items |= importer::HISTORY;
+    } else {
+      // Automatic history import can be turned off in master_prefs.
+      out_prefs->dont_import_items |= importer::HISTORY;
+    }
   }
 
-  // Home page is imported automatically only in organic builds, and can be
-  // turned off in master_prefs.
   if (installer_util::GetDistroBooleanPreference(prefs.get(),
-      installer_util::master_preferences::kDistroImportHomePagePref, &value)
-      && !value) {
-    out_prefs->dont_import_items |= importer::HOME_PAGE;
+      installer_util::master_preferences::kDistroImportHomePagePref, &value)) {
+    if (value) {
+      import_items |= importer::HOME_PAGE;
+    } else {
+      // Automatic home page import can be turned off in master_prefs.
+      out_prefs->dont_import_items |= importer::HOME_PAGE;
+    }
   }
 
   // Bookmarks are never imported unless specifically turned on.
   if (installer_util::GetDistroBooleanPreference(prefs.get(),
       installer_util::master_preferences::kDistroImportBookmarksPref, &value)
       && value) {
+    import_items |= importer::FAVORITES;
     out_prefs->do_import_items |= importer::FAVORITES;
   }
 
+  if (installer_util::GetDistroBooleanPreference(prefs.get(),
+      installer_util::master_preferences::kMakeChromeDefaultForUser, &value) &&
+      value)
+    ShellIntegration::SetAsDefaultBrowser();
+
+  // TODO(mirandac): Refactor skip-first-run-ui process into regular first run
+  // import process.  http://crbug.com/49647
   // Note we are skipping all other master preferences if skip-first-run-ui
   // is *not* specified. (That is, we continue only if skipping first run ui.)
   if (!installer_util::GetDistroBooleanPreference(prefs.get(),
@@ -377,10 +376,6 @@ bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
       !value)
     return true;
 
-  // From here on we won't show first run so we need to do the work to show the
-  // bubble anyway, unless it's already been explicitly suppressed.
-  FirstRun::SetShowFirstRunBubblePref(true);
-
   // We need to be able to create the first run sentinel or else we cannot
   // proceed because ImportSettings will launch the importer process which
   // would end up here if the sentinel is not present.
@@ -408,11 +403,6 @@ bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
     }
   }
 
-  if (installer_util::GetDistroBooleanPreference(prefs.get(),
-      installer_util::master_preferences::kMakeChromeDefaultForUser, &value) &&
-      value)
-    ShellIntegration::SetAsDefaultBrowser();
-
   return false;
 }
 
@@ -678,6 +668,10 @@ void FirstRun::AutoImport(Profile* profile,
     }
   }
 
+  FirstRun::SetShowFirstRunBubblePref(true);
+  // Set the first run bubble to minimal.
+  FirstRun::SetMinimalFirstRunBubblePref();
+
   process_singleton->Unlock();
   FirstRun::CreateSentinel();
 }
diff --git a/chrome/browser/views/location_bar/location_bar_view.cc b/chrome/browser/views/location_bar/location_bar_view.cc
index 4072c58..b9be6a5 100644
--- a/chrome/browser/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/views/location_bar/location_bar_view.cc
@@ -99,7 +99,8 @@ LocationBarView::LocationBarView(Profile* profile,
       star_view_(NULL),
       mode_(mode),
       show_focus_rect_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(first_run_bubble_(this)) {
+      bubble_type_(FirstRun::MINIMALBUBBLE),
+      template_url_model_(NULL) {
   DCHECK(profile_);
   SetID(VIEW_ID_LOCATION_BAR);
   SetFocusable(true);
@@ -109,6 +110,8 @@ LocationBarView::LocationBarView(Profile* profile,
 }
 
 LocationBarView::~LocationBarView() {
+  if (template_url_model_)
+    template_url_model_->RemoveObserver(this);
 }
 
 void LocationBarView::Init() {
@@ -946,10 +949,15 @@ bool LocationBarView::CanStartDrag(View* sender,
 // LocationBarView, LocationBar implementation:
 
 void LocationBarView::ShowFirstRunBubble(FirstRun::BubbleType bubble_type) {
-  // We wait 30 milliseconds to open. It allows less flicker.
-  Task* task = first_run_bubble_.NewRunnableMethod(
-      &LocationBarView::ShowFirstRunBubbleInternal, bubble_type);
-  MessageLoop::current()->PostDelayedTask(FROM_HERE, task, 30);
+  // Wait until search engines have loaded to show the first run bubble.
+  if (!profile_->GetTemplateURLModel()->loaded()) {
+    bubble_type_ = bubble_type;
+    template_url_model_ = profile_->GetTemplateURLModel();
+    template_url_model_->AddObserver(this);
+    template_url_model_->Load();
+    return;
+  }
+  ShowFirstRunBubbleInternal(bubble_type);
 }
 
 std::wstring LocationBarView::GetInputString() const {
@@ -1035,3 +1043,10 @@ void LocationBarView::TestPageActionPressed(size_t index) {
 
   NOTREACHED();
 }
+
+void LocationBarView::OnTemplateURLModelChanged() {
+  template_url_model_->RemoveObserver(this);
+  template_url_model_ = NULL;
+  ShowFirstRunBubble(bubble_type_);
+}
+
diff --git a/chrome/browser/views/location_bar/location_bar_view.h b/chrome/browser/views/location_bar/location_bar_view.h
index ca092bd..93dd54d 100644
--- a/chrome/browser/views/location_bar/location_bar_view.h
+++ b/chrome/browser/views/location_bar/location_bar_view.h
@@ -13,6 +13,7 @@
 #include "chrome/browser/extensions/extension_context_menu_model.h"
 #include "chrome/browser/first_run.h"
 #include "chrome/browser/location_bar.h"
+#include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/toolbar_model.h"
 #include "chrome/browser/views/extensions/extension_popup.h"
@@ -58,7 +59,8 @@ class LocationBarView : public LocationBar,
                         public LocationBarTesting,
                         public views::View,
                         public views::DragController,
-                        public AutocompleteEditController {
+                        public AutocompleteEditController,
+                        public TemplateURLModelObserver {
  public:
   // The location bar view's class name.
   static const char kViewClassName[];
@@ -103,8 +105,8 @@ class LocationBarView : public LocationBar,
 
   void Init();
 
-  // Returns whether this instance has been initialized by callin Init. Init can
-  // only be called when the receiving instance is attached to a view container.
+  // True if this instance has been initialized by calling Init, which can only
+  // be called when the receiving instance is attached to a view container.
   bool IsInitialized() const;
 
   // Returns the appropriate color for the desired kind, based on the user's
@@ -220,6 +222,9 @@ class LocationBarView : public LocationBar,
   virtual ExtensionAction* GetVisiblePageAction(size_t index);
   virtual void TestPageActionPressed(size_t index);
 
+  // Overridden from TemplateURLModelObserver
+  virtual void OnTemplateURLModelChanged();
+
   static const int kVertMargin;     // Space above and below the edit.
   static const int kEdgeThickness;  // Unavailable space at horizontal edges.
   static const int kItemPadding;    // Space between items within the bar.
@@ -351,13 +356,18 @@ class LocationBarView : public LocationBar,
   // focused. Used when the toolbar is in full keyboard accessibility mode.
   bool show_focus_rect_;
 
+  // Whether bubble text is short or long.
+  FirstRun::BubbleType bubble_type_;
+
+  // This is in case we're destroyed before the model loads. We store the model
+  // because calling profile_->GetTemplateURLModel() in the destructor causes a
+  // crash.
+  TemplateURLModel* template_url_model_;
+
 #if defined(OS_LINUX)
   scoped_ptr<AccessibleWidgetHelper> accessible_widget_helper_;
 #endif
 
-  // Used to schedule a task for the first run info bubble.
-  ScopedRunnableMethodFactory<LocationBarView> first_run_bubble_;
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(LocationBarView);
 };