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

chrome/browser/search_engines/template_url_service.h

 // 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.
 
 #ifndef CHROME_BROWSER_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_
 #define CHROME_BROWSER_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_
 
 #include <list>
 #include <map>
 #include <set>
@@ skipping 119 matching lines @@
   // Just like GenerateSearchURL except that it takes SearchTermsData to supply
   // the data for some search terms, e.g. so this can be used on threads other
   // than the UI thread.  See the various TemplateURLRef::XXXUsingTermsData()
   // functions.
   static GURL GenerateSearchURLUsingTermsData(
       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;
+  static void SaveDefaultSearchProviderToPrefs(const TemplateURL* url,

[Peter Kasting, 2014/05/08 22:46:46]

Bad rebase?  This is the same way the code looks on trunk today, I think.

Also, is this function only called by tests now?  May want to note that if so. 
Unless you're just going to delete this in a followup change soon.

[erikwright (departed), 2014/05/09 18:00:22]

In your CL you made it non-static in order to not save when in a failed state.
This CL reverts that since it is now only used externally.

I deferred moving this function to the sole client (a test) in order to
facilitate your review of this CL. You have already LGTM'd the CL that contains
the move (I'll land it ASAP after this CL).

[Peter Kasting, 2014/05/09 19:12:49]

Ah, you're right.  Sorry, at one point I locally reverted and then un-reverted
this change, and I forgot I'd unreverted it.

+                                               PrefService* prefs);
 
   // 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
   // non-NULL, template_url_to_replace is set to the keyword to replace.
   //
   // url gives the url of the search query. The url is used to avoid generating
   // a TemplateURL for an existing TemplateURL that shares the same host.
   bool CanReplaceKeyword(const base::string16& keyword,
                          const GURL& url,
@@ skipping 16 matching lines @@
   // The caller should not try to delete the returned pointer; the data store
   // retains ownership of it.
   TemplateURL* GetTemplateURLForGUID(const std::string& sync_guid);
 
   // Returns the first TemplateURL found with a URL using the specified |host|,
   // or NULL if there are no such TemplateURLs
   TemplateURL* GetTemplateURLForHost(const std::string& host);
 
   // 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);
+  // Returns true if the Add is successful.
+  bool Add(TemplateURL* template_url);
 
   // Like Add(), but overwrites the |template_url|'s values with the provided
   // ones.
   void AddAndSetProfile(TemplateURL* template_url, Profile* profile);
   void AddWithOverrides(TemplateURL* template_url,
                         const base::string16& short_name,
                         const base::string16& keyword,
                         const std::string& url);
 
   // Add the search engine of type NORMAL_CONTROLLED_BY_EXTENSION.
@@ skipping 65 matching lines @@
   // loaded, the default search provider is pulled from preferences.
   //
   // NOTE: At least in unittest mode, this may return NULL.
   TemplateURL* GetDefaultSearchProvider();
 
   // Returns true if the |url| is a search results page from the default search
   // provider.
   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();
 
   // Returns the default search specified in the prepopulated data, if it
   // exists.  If not, returns first URL in |template_urls_|, or NULL if that's
   // empty. The returned object is owned by TemplateURLService and can be
   // destroyed at any time so should be used right after the call.
   TemplateURL* FindNewDefaultSearchProvider();
 
@@ skipping 102 matching lines @@
   static TemplateURL* CreateTemplateURLFromTemplateURLAndSyncData(
       Profile* profile,
       TemplateURL* existing_turl,
       const syncer::SyncData& sync_data,
       syncer::SyncChangeList* change_list);
 
   // Returns a map mapping Sync GUIDs to pointers to syncer::SyncData.
   static SyncDataMap CreateGUIDToSyncDataMap(
       const syncer::SyncDataList& sync_data);
 
+  // Indicates whether the pre-populated TemplateURLs should be disabled. May
+  // only be true in tests.
+  static bool fallback_search_engines_disabled() {
+    return g_fallback_search_engines_disabled;
+  }
+
 #if defined(UNIT_TEST)
+  // Disables the pre-populated TemplateURLs for testing purposes.
+  static void set_fallback_search_engines_disabled() {
+    g_fallback_search_engines_disabled = true;
+  }
+
   // Set a different time provider function, such as

[Peter Kasting, 2014/05/08 22:46:46]

Nit: While here: Set -> Sets; when -> for

   // base::MockTimeProvider::StaticNow, when testing calls to base::Time::Now.
   void set_time_provider(TimeProvider* time_provider) {
     time_provider_ = time_provider;
   }
 #endif
 
  protected:
   // Cover method for the method of the same name on the HistoryService.
   // url is the one that was visited with the given search terms.
   //
@@ skipping 66 matching lines @@
   // This does NOT notify the delegate or the database.
   //
   // This transfers ownership of the elements in |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 |urls| on exit.
   void SetTemplateURLs(TemplateURLVector* urls);
 
   // Transitions to the loaded state.
   void ChangeToLoadedState();
 
-  // Clears user preferences describing the default search engine.
-  void ClearDefaultProviderFromPrefs();
+  // Called by DefaultSearchManager when the effective Default Search Engine has

[Peter Kasting, 2014/05/08 22:46:46]

Nit: I wouldn't capitalize Default Search Engine

+  // changed.
+  void OnDefaultSearchChange(const TemplateURLData* new_dse_data,
+                             DefaultSearchManager::Source source);
+
+  // Applies a DSE change. May be called at startup or after transitioning to
+  // the loaded state.
+  void ApplyDefaultSearchChange(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
   // be replaced.
   bool CanReplaceKeywordForHost(const std::string& host,
                                 TemplateURL** to_replace);
 
   // Returns true if the TemplateURL is replaceable. This doesn't look at the
   // uniqueness of the keyword or host and is intended to be called after those
   // checks have been done. This returns true if the TemplateURL doesn't appear
@@ skipping 16 matching lines @@
                       const TemplateURL& new_values,
                       const SearchTermsData& old_search_terms_data);
 
   // If the TemplateURL comes from a prepopulated URL available in the current
   // country, update all its fields save for the keyword, short name and id so
   // that they match the internal prepopulated URL. TemplateURLs not coming from
   // a prepopulated URL are not modified.
   static void UpdateTemplateURLIfPrepopulated(TemplateURL* existing_turl,
                                               Profile* profile);
 
+  // If the TemplateURL's sync GUID matches the kSyncedDefaultSearchProviderGUID
+  // preference it will be used to update the DSE in memory and as persisted in
+  // preferences.
+  void MaybeUpdateDSEAfterSync(TemplateURL* synced_turl);
+
   // Returns the preferences we use.
   PrefService* GetPrefs();
 
   // Iterates through the TemplateURLs to see if one matches the visited url.
   // For each TemplateURL whose url matches the visited url
   // SetKeywordSearchTermsForURL is invoked.
   void UpdateKeywordSearchTermsForURL(
       const history::URLVisitedDetails& details);
 
   // If necessary, generates a visit for the site http:// + t_url.keyword().
   void AddTabToSearchVisit(const TemplateURL& t_url);
 
   // Invoked when the Google base URL has changed. Updates the mapping for all
   // TemplateURLs that have a replacement term of {google:baseURL} or
   // {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
   // part of the model in the past, and therefore we don't need to do things
   // like assign it an ID or notify sync.
   // This function guarantees that on return the model will not have two
   // non-extension TemplateURLs with the same keyword.  If that means that it
   // cannot add the provided argument, it will delete it and return false.
   // Caller is responsible for notifying observers if this function returns
   // true.
   bool AddNoNotify(TemplateURL* template_url, bool newly_adding);
 
   // Removes the keyword from the model. This deletes the supplied TemplateURL.
   // This fails if the supplied template_url is the default search provider.
   // Caller is responsible for notifying observers.
   void RemoveNoNotify(TemplateURL* template_url);
 
   // Like ResetTemplateURL(), but instead of notifying observers, returns
   // whether anything has changed.
   bool ResetTemplateURLNoNotify(TemplateURL* url,
                                 const base::string16& title,
                                 const base::string16& keyword,
                                 const std::string& search_url);
 
   // Notify the observers that the model has changed.  This is done only if the
   // 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.
   void ResetTemplateURLGUID(TemplateURL* url, const std::string& guid);
 
   // Attempts to generate a unique keyword for |turl| based on its original
   // keyword. If its keyword is already unique, that is returned. Otherwise, it
   // tries to return the autogenerated keyword if that is unique to the Service,
   // and finally it repeatedly appends special characters to the keyword until
   // it is unique to the Service. If |force| is true, then this will only
@@ skipping 37 matching lines @@
   // sent up to Sync.
   // |merge_result| tracks the changes made to the local model. Added/modified/
   // deleted are updated depending on how the |sync_turl| is merged in.
   // This should only be called from MergeDataAndStartSyncing.
   void MergeInSyncTemplateURL(TemplateURL* sync_turl,
                               const SyncDataMap& sync_data,
                               syncer::SyncChangeList* change_list,
                               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);
 
+  // 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);
 
+  // Finds the extension-supplied TemplateURL that matches |data|, if any.
+  TemplateURL* FindMatchingExtensionTemplateURL(const TemplateURLData& data,
+                                                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;
-
-  // 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();
+  // 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_;
 
   // Mapping from Sync GUIDs to the TemplateURL.
   GUIDToTemplateMap guid_to_template_map_;
 
@@ skipping 26 matching lines @@
   scoped_refptr<WebDataService> service_;
 
   // All visits that occurred before we finished loading. Once loaded
   // UpdateKeywordSearchTermsForURL is invoked for each element of the vector.
   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* 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.
   TemplateURLID next_id_;
 
   // Function returning current time in base::Time units.
   TimeProvider* time_provider_;
 
   // Do we have an active association between the TemplateURLs and sync models?
   // Set in MergeDataAndStartSyncing, reset in StopSyncing. While this is not
   // set, we ignore any local search engine changes (when we start syncing we
   // will look up the most recent values anyways).
   bool models_associated_;
 
   // Whether we're currently processing changes from the syncer. While this is
   // true, we ignore any local search engine changes, since we triggered them.
   bool processing_syncer_changes_;
 
   // Sync's syncer::SyncChange handler. We push all our changes through this.
   scoped_ptr<syncer::SyncChangeProcessor> sync_processor_;
 
   // 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.
   DefaultSearchManager default_search_manager_;
 
+  // Used to disable the prepopulated search engines in tests.
+  static bool g_fallback_search_engines_disabled;
+
   DISALLOW_COPY_AND_ASSIGN(TemplateURLService);
 };
 
 #endif  // CHROME_BROWSER_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_