Use the DefaultSearchManager as the exclusive authority on DSE, ignoring Web Data.

chrome/browser/search_engines/template_url_service.h

Index: chrome/browser/search_engines/template_url_service.h
diff --git a/chrome/browser/search_engines/template_url_service.h b/chrome/browser/search_engines/template_url_service.h
index 60bcd6c21f7bd806d5f670095fb6f574863fa678..4b0ba7430dce8362c7f60b0074e4eaa58a02d7fb 100644
--- a/chrome/browser/search_engines/template_url_service.h
+++ b/chrome/browser/search_engines/template_url_service.h
@@ -101,17 +101,6 @@ class TemplateURLService : public WebDataServiceConsumer,
   TemplateURLService(const Initializer* initializers, const int count);
   virtual ~TemplateURLService();
 
-  // Creates a TemplateURLData that was previously saved to |prefs| via
-  // SaveDefaultSearchProviderToPrefs or set via policy.
-  // Returns true if successful, false otherwise.
-  // If the user or the policy has opted for no default search, this
-  // returns true but default_provider is set to NULL.
-  // |*is_managed| specifies whether the default is managed via policy.
-  static bool LoadDefaultSearchProviderFromPrefs(
-      PrefService* prefs,
-      scoped_ptr<TemplateURLData>* default_provider_data,
-      bool* is_managed);
-
   // Generates a suitable keyword for the specified url, which must be valid.
   // This is guaranteed not to return an empty string, since TemplateURLs should
   // never have an empty keyword.
@@ -135,11 +124,6 @@ class TemplateURLService : public WebDataServiceConsumer,
       const TemplateURL* t_url,
       const SearchTermsData& search_terms_data);
 
-  // Saves enough of url to |prefs| so that it can be loaded from preferences on
-  // start up.
-  void SaveDefaultSearchProviderToPrefs(const TemplateURL* url,
-                                        PrefService* prefs) const;
-
   // Returns true if there is no TemplateURL that conflicts with the
   // keyword/url pair, or there is one but it can be replaced. If there is an
   // existing keyword that can be replaced and template_url_to_replace is
@@ -156,7 +140,7 @@ class TemplateURLService : public WebDataServiceConsumer,
   // TemplateURLs that support replacement are returned.
   void FindMatchingKeywords(const base::string16& prefix,
                             bool support_replacement_only,
-                            TemplateURLVector* matches) const;
+                            TemplateURLVector* matches);
 
   // Looks up |keyword| and returns the element it maps to.  Returns NULL if
   // the keyword was not found.
@@ -175,7 +159,7 @@ class TemplateURLService : public WebDataServiceConsumer,
 
   // Takes ownership of |template_url| and adds it to this model.  For obvious
   // reasons, it is illegal to Add() the same |template_url| pointer twice.
-  void Add(TemplateURL* template_url);
+  bool Add(TemplateURL* template_url);

[gab, 2014/05/07 20:25:09]

Add comment about return statement.

[erikwright (departed), 2014/05/07 22:26:47]

Done.

 
   // Like Add(), but overwrites the |template_url|'s values with the provided
   // ones.
@@ -261,7 +245,9 @@ class TemplateURLService : public WebDataServiceConsumer,
   bool IsSearchResultsPageFromDefaultSearchProvider(const GURL& url);
 
   // Returns true if the default search is managed through group policy.
-  bool is_default_search_managed() const { return is_default_search_managed_; }
+  bool is_default_search_managed() const {
+    return default_search_provider_source_ == DefaultSearchManager::FROM_POLICY;
+  }
 
   // Returns true if the default search provider is controlled by an extension.
   bool IsExtensionControlledDefaultSearch();
@@ -276,7 +262,7 @@ class TemplateURLService : public WebDataServiceConsumer,
   // revved: all existing prepopulated entries are checked against the current
   // prepopulate data, any now-extraneous safe_for_autoreplace() entries are
   // removed, any existing engines are reset to the provided data (except for
-  // user-edited names or keywords), and any new prepopulated anegines are
+  // user-edited names or keywords), and any new prepopulated engines are
   // added.
   //
   // After this, the default search engine is reset to the default entry in the
@@ -384,6 +370,15 @@ class TemplateURLService : public WebDataServiceConsumer,
   static SyncDataMap CreateGUIDToSyncDataMap(
       const syncer::SyncDataList& sync_data);
 
+  // Used to determine if the pre-populated TemplateURLs (this will be true for
+  // some tests, specifically TemplateURLServiceSyncTests).
+  static bool FallbackSearchEnginesDisabled() {
+    return fallback_search_engines_disabled_;
+  }
+
+  // Used to disable the pre-populated TemplateURLs for testing purposes.
+  static void DisableFallbackSearchEngines();
+
 #if defined(UNIT_TEST)
   // Set a different time provider function, such as
   // base::MockTimeProvider::StaticNow, when testing calls to base::Time::Now.
@@ -422,35 +417,6 @@ class TemplateURLService : public WebDataServiceConsumer,
   typedef std::map<base::string16, TemplateURL*> KeywordToTemplateMap;
   typedef std::map<std::string, TemplateURL*> GUIDToTemplateMap;
 
-  // Declaration of values to be used in an enumerated histogram to tally
-  // changes to the default search provider from various entry points. In
-  // particular, we use this to see what proportion of changes are from Sync
-  // entry points, to help spot erroneous Sync activity.
-  enum DefaultSearchChangeOrigin {
-    // Various known Sync entry points.
-    DSP_CHANGE_SYNC_PREF,
-    DSP_CHANGE_SYNC_ADD,
-    DSP_CHANGE_SYNC_DELETE,
-    DSP_CHANGE_SYNC_NOT_MANAGED,
-    // "Other" origins. We differentiate between Sync and not Sync so we know if
-    // certain changes were intentionally from the system, or possibly some
-    // unintentional change from when we were Syncing.
-    DSP_CHANGE_SYNC_UNINTENTIONAL,
-    // All changes that don't fall into another category; we can't reorder the
-    // list for clarity as this would screw up stat collection.
-    DSP_CHANGE_OTHER,
-    // Changed through "Profile Reset" feature.
-    DSP_CHANGE_PROFILE_RESET,
-    // Changed by an extension through the Override Settings API.
-    DSP_CHANGE_OVERRIDE_SETTINGS_EXTENSION,
-    // New DSP during database/prepopulate data load, which was not previously
-    // in the known engine set, and with no previous value in prefs.  The
-    // typical time to see this is during first run.
-    DSP_CHANGE_NEW_ENGINE_NO_PREFS,
-    // Boundary value.
-    DSP_CHANGE_MAX,
-  };
-
   // Helper functor for FindMatchingKeywords(), for finding the range of
   // keywords which begin with a prefix.
   class LessWithPrefix;
@@ -472,8 +438,26 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Transitions to the loaded state.
   void ChangeToLoadedState();
 
-  // Clears user preferences describing the default search engine.
-  void ClearDefaultProviderFromPrefs();
+  // Called when we transition to the loaded state without having loaded the web
+  // database from disk.  This may be because of profile corruption, or because
+  // we're in a test.  Disables further attempts to access the web database and
+  // ensures we won't attempt to sync any changes, then copies
+  // |initial_default_search_provider_| in as the default search provider, so
+  // the TemplateURLService consistently reports the same default provider
+  // before and after the attempted load.
+  void OnFailedLoad();
+
+  // Receives notice that the DSE has changed.
+  void OnDefaultSearchChange(const TemplateURLData* new_dse_data,
+                             DefaultSearchManager::Source source);
+
+  // Applies a DSE change. May be called at startup or after transitioning to
+  // loaded state.
+  void ApplyDefaultSearchChange(const TemplateURLData* new_dse_data,
+                                DefaultSearchManager::Source source);
+
+  // Receives notice that the DSE GUID in prefs has changed.
+  void OnDSEGUIDPrefChanged();
 
   // Returns true if there is no TemplateURL that has a search url with the
   // specified host, or the only TemplateURLs matching the specified host can
@@ -500,7 +484,7 @@ class TemplateURLService : public WebDataServiceConsumer,
   // NOTE: This should not be called with an extension keyword as there are no
   // updates needed in that case.
   bool UpdateNoNotify(TemplateURL* existing_turl,
-                      const TemplateURL& new_values,
+                      const TemplateURLData& new_values,
                       const SearchTermsData& old_search_terms_data);
 
   // If the TemplateURL comes from a prepopulated URL available in the current
@@ -527,26 +511,6 @@ class TemplateURLService : public WebDataServiceConsumer,
   // {google:baseSuggestURL}.
   void GoogleBaseURLChanged(const GURL& old_base_url);
 
-  // Update the default search.  Called at initialization or when a managed
-  // preference has changed.
-  void UpdateDefaultSearch();
-
-  // Set the default search provider.  |url| may be user-selected or
-  // automatically selected and may be null.
-  // This will assert if the default search is managed.
-  void SetDefaultSearchProvider(TemplateURL* url);
-
-  // Set the default search provider even if it is managed. |url| may be null.
-  // Caller is responsible for notifying observers.  Returns whether |url| was
-  // found in |template_urls_|.
-  // If |url| is an extension-controlled search engine then preferences and the
-  // database are left untouched.
-  // If |url| is a normal search engine and the existing default search engine
-  // is controlled by an extension then |url| is propagated to the database and
-  // prefs but the extension-controlled default engine will continue to hide
-  // this value until the extension is uninstalled.
-  bool SetDefaultSearchProviderNoNotify(TemplateURL* url);
-
   // Adds a new TemplateURL to this model. TemplateURLService will own the
   // reference, and delete it when the TemplateURL is removed.
   // If |newly_adding| is false, we assume that this TemplateURL was already
@@ -568,14 +532,12 @@ class TemplateURLService : public WebDataServiceConsumer,
   // model is loaded.
   void NotifyObservers();
 
-  // Removes from the vector any template URL that was created because of
-  // policy.  These TemplateURLs are freed and removed from the database.
-  // Sets default_search_provider to NULL if it was one of them, unless it is
-  // the same as the current default from preferences and it is managed.
-  void RemoveProvidersCreatedByPolicy(
+  // Updates |template_urls| so that the only "created by policy" entry is
+  // |default_from_prefs|. |default_from_prefs| may be NULL if there is no
+  // policy-defined DSE in effect.
+  void UpdateProvidersCreatedByPolicy(
       TemplateURLVector* template_urls,
-      TemplateURL** default_search_provider,
-      TemplateURLData* default_from_prefs);
+      const TemplateURLData* default_from_prefs);
 
   // Resets the sync GUID of the specified TemplateURL and persists the change
   // to the database. This does not notify observers.
@@ -633,58 +595,41 @@ class TemplateURLService : public WebDataServiceConsumer,
                               SyncDataMap* local_data,
                               syncer::SyncMergeResult* merge_result);
 
-  // Checks a newly added TemplateURL from Sync by its sync_guid and sets it as
-  // the default search provider if we were waiting for it.
-  void SetDefaultSearchProviderIfNewlySynced(const std::string& guid);
-
-  // Retrieve the pending default search provider according to Sync. Returns
-  // NULL if there was no pending search provider from Sync.
-  TemplateURL* GetPendingSyncedDefaultSearchProvider();
-
   // Goes through a vector of TemplateURLs and ensure that both the in-memory
   // and database copies have valid sync_guids. This is to fix crbug.com/102038,
   // where old entries were being pushed to Sync without a sync_guid.
   void PatchMissingSyncGUIDs(TemplateURLVector* template_urls);
 
-  void OnSyncedDefaultSearchProviderGUIDChanged();
-
-  // Adds |template_urls| to |template_urls_| and sets up the default search
-  // provider.  If |default_search_provider| is non-NULL, it must refer to one
-  // of the |template_urls|, and will be used as the new default.
+  // Adds |template_urls| to |template_urls_|.
   //
   // This transfers ownership of the elements in |template_urls| to |this|, and
   // may delete some elements, so it's not safe for callers to access any
   // elements after calling; to reinforce this, this function clears
   // |template_urls| on exit.
-  void AddTemplateURLsAndSetupDefaultEngine(
-      TemplateURLVector* template_urls,
-      TemplateURL* default_search_provider);
-
-  // If there is no current default search provider, sets the default to the
-  // result of calling FindNewDefaultSearchProvider().
-  void EnsureDefaultSearchProviderExists();
+  void AddTemplateURLs(TemplateURLVector* template_urls);
 
   // Returns a new TemplateURL for the given extension.
   TemplateURL* CreateTemplateURLForExtension(
-      const ExtensionKeyword& extension_keyword) const;
+      const ExtensionKeyword& extension_keyword);
+
+  // Returns the TemplateURL corresponding to |prepopulated_id|, if any.
+  TemplateURL* FindPrepopulatedTemplateURL(int prepopulated_id);
 
   // Returns the TemplateURL associated with |extension_id|, if any.
   TemplateURL* FindTemplateURLForExtension(const std::string& extension_id,
-                                           TemplateURL::Type type) const;
+                                           TemplateURL::Type type);
 
-  // Finds the most recently-installed NORMAL_CONTROLLED_BY_EXTENSION engine
-  // that supports replacement and wants to be default, if any.
-  TemplateURL* FindExtensionDefaultSearchEngine() const;
+  // Finds the extension-supplied TemplateURL that matches |data|, if any.
+  TemplateURL* FindMatchingExtensionTemplateURL(const TemplateURLData& data,
+                                                TemplateURL::Type type);
 
-  // Sets the default search provider to:
-  // (1) BestDefaultExtensionControlledTURL(), if any; or,
-  // (2) LoadDefaultSearchProviderFromPrefs(), if we have a TURL with that ID;
-  // or,
-  // (3) FindNewDefaultSearchProvider().
-  void SetDefaultSearchProviderAfterRemovingDefaultExtension();
+  // Finds the most recently-installed NORMAL_CONTROLLED_BY_EXTENSION engine
+  // that supports replacement and wants to be default, if any. Notifies the
+  // DefaultSearchManager, which might change the effective default search
+  // engine.
+  void UpdateExtensionDefaultSearchEngine();
 
   content::NotificationRegistrar notification_registrar_;
-  PrefChangeRegistrar pref_change_registrar_;
 
   // Mapping from keyword to the TemplateURL.
   KeywordToTemplateMap keyword_to_template_map_;
@@ -725,15 +670,15 @@ class TemplateURLService : public WebDataServiceConsumer,
   std::vector<history::URLVisitedDetails> visits_to_add_;
 
   // Once loaded, the default search provider.  This is a pointer to a
-  // TemplateURL owned by template_urls_.
+  // TemplateURL owned by |template_urls_|.
   TemplateURL* default_search_provider_;
 
-  // The initial search provider extracted from preferences. This is only valid
-  // if we haven't been loaded or loading failed.
+  // A temporary location for the DSE until Web Data has been loaded and it can
+  // be merged into |template_urls_|.
   scoped_ptr<TemplateURL> initial_default_search_provider_;
 
-  // Whether the default search is managed via policy.
-  bool is_default_search_managed_;
+  // Source of the default search provider.
+  DefaultSearchManager::Source default_search_provider_source_;
 
   // ID assigned to next TemplateURL added to this model. This is an ever
   // increasing integer that is initialized from the database.
@@ -758,30 +703,23 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Sync's error handler. We use it to create a sync error.
   scoped_ptr<syncer::SyncErrorFactory> sync_error_factory_;
 
-  // Whether or not we are waiting on the default search provider to come in
-  // from Sync. This is to facilitate the fact that changes to the value of
-  // prefs::kSyncedDefaultSearchProviderGUID do not always come before the
-  // TemplateURL entry it refers to, and to handle the case when we want to use
-  // the Synced default when the default search provider becomes unmanaged.
-  bool pending_synced_default_search_;
-
   // A set of sync GUIDs denoting TemplateURLs that have been removed from this
   // model or the underlying WebDataService prior to MergeDataAndStartSyncing.
   // This set is used to determine what entries from the server we want to
   // ignore locally and return a delete command for.
   std::set<std::string> pre_sync_deletes_;
 
-  // This is used to log the origin of changes to the default search provider.
-  // We set this value to increasingly specific values when we know what is the
-  // cause/origin of a default search change.
-  DefaultSearchChangeOrigin dsp_change_origin_;
-
   // Stores a list of callbacks to be run after TemplateURLService has loaded.
   base::CallbackList<void(void)> on_loaded_callbacks_;
 
-  // Helper class to manage the default search engine. This will be NULL when
-  // using the testing-specific constructor.
-  scoped_ptr<DefaultSearchManager> default_search_manager_;
+  // Helper class to manage the default search engine.
+  DefaultSearchManager default_search_manager_;
+
+  // Used to observe the DSE GUID in prefs.
+  PrefChangeRegistrar pref_change_registrar_;
+
+  // Used to disable the prepopulated search engines in tests.
+  static bool fallback_search_engines_disabled_;
 
   DISALLOW_COPY_AND_ASSIGN(TemplateURLService);
 };