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 f8896d52fa419fe78b3684f6f3a667af7039ea18..e30151e7636d24aaa5d27a1f9b5518888cb50739 100644
--- a/chrome/browser/search_engines/template_url_service.h
+++ b/chrome/browser/search_engines/template_url_service.h
@@ -101,6 +101,22 @@ class TemplateURLService : public WebDataServiceConsumer,
   TemplateURLService(const Initializer* initializers, const int count);
   virtual ~TemplateURLService();
 
+  // Saves enough of url to preferences so that it can be loaded from

[Peter Kasting, 2014/05/03 00:28:18]

Nit: url -> |url|

[erikwright (departed), 2014/05/05 01:49:38]

Done.

+  // preferences on start up.
+  static void SaveDefaultSearchProviderToLegacyPrefs(const TemplateURL* url,
+                                                     PrefService* prefs);
+
+  // Creates a TemplateURLData that was previously saved to prefs via
+  // SaveDefaultSearchProviderToLegacyPrefs or set via policy.
+  // Returns true if successful, false otherwise.
+  // If the user or the policy has opted for no default search, this

[Peter Kasting, 2014/05/03 00:28:18]

Users cannot opt for no default search.  Only policy is supposed to be able to
do that.

[erikwright (departed), 2014/05/05 01:49:38]

Done.

+  // returns true but default_provider is set to NULL.
+  // |*is_managed| specifies whether the default is managed via policy.
+  static bool LoadDefaultSearchProviderFromLegacyPrefs(
+      PrefService* prefs,
+      scoped_ptr<TemplateURLData>* default_provider,
+      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.
@@ -245,7 +261,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();
@@ -410,35 +428,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;
@@ -460,22 +449,9 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Transitions to the loaded state.
   void ChangeToLoadedState();
 
-  // Saves enough of url to preferences so that it can be loaded from
-  // preferences on start up.
-  void SaveDefaultSearchProviderToPrefs(const TemplateURL* url);
-
-  // 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.
-  bool LoadDefaultSearchProviderFromPrefs(
-      scoped_ptr<TemplateURLData>* default_provider,
-      bool* is_managed);
-
-  // Clears user preferences describing the default search engine.
-  void ClearDefaultProviderFromPrefs();
+  // Receives notice that the DSE has changed.
+  void OnDefaultSearchChange(const TemplateURLData* new_dse_data,
+                             DefaultSearchManager::Source source);
 
   // 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
@@ -502,7 +478,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
@@ -529,26 +505,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
@@ -570,14 +526,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.
@@ -635,58 +589,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;

[Peter Kasting, 2014/05/03 00:28:18]

Const functions must not return non-const pointers.  At the very least, don't
add new functions that do so; if at all possible, please also fix the signatures
of these existing functions.

[erikwright (departed), 2014/05/05 01:49:38]

I removed const from each of the method signatures that returned non-const
pointers to the contents of the TemplateURLService. They were never called on a
const instance, so this had no ripple effects.

 
+  // Returns the TemplateURL corresponding to |prepopulated_id|, if any.
+  TemplateURL* FindPrepopulatedTemplateURL(int prepopulated_id) const;
+
   // Returns the TemplateURL associated with |extension_id|, if any.
   TemplateURL* FindTemplateURLForExtension(const std::string& extension_id,
                                            TemplateURL::Type type) const;
 
-  // 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) const;
 
-  // 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 +662,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,24 +695,12 @@ 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_;