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

chrome/browser/search_engines/util.cc

 // 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.
 
 #include "chrome/browser/search_engines/util.h"
 
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 81 matching lines @@
   while (!unchecked_urls.empty()) {
     // Find the best URL.
     int prepopulate_id = unchecked_urls.begin()->first;
     PrepopulatedURLMap::const_iterator prepopulated_url =
         prepopulated_url_map.find(prepopulate_id);
     UncheckedURLMap::iterator end = unchecked_urls.upper_bound(prepopulate_id);
     UncheckedURLMap::iterator best = unchecked_urls.begin();
     bool matched_keyword = false;
     for (UncheckedURLMap::iterator i = unchecked_urls.begin(); i != end; ++i) {
       // A URL is automatically the best if it's the default search engine.
-      if (i->second == default_search_provider) {
+      if (default_search_provider &&
+          default_search_provider->sync_guid() == i->second->sync_guid()) {
         best = i;
         break;
       }
 
       // Otherwise, a URL is best if it matches the prepopulated data's keyword;
       // if none match, just fall back to using the one with the lowest ID.
       if (matched_keyword)
         continue;
       if ((prepopulated_url != prepopulated_url_map.end()) &&
           i->second->HasSameKeywordAs(*prepopulated_url->second)) {
@@ skipping 71 matching lines @@
 // This is invoked when the version of the prepopulate data changes.
 // If |removed_keyword_guids| is not NULL, the Sync GUID of each item removed
 // from the DB will be added to it.  Note that this function will take
 // ownership of |prepopulated_urls| and will clear the vector.
 void MergeEnginesFromPrepopulateData(
     Profile* profile,
     WebDataService* service,
     ScopedVector<TemplateURLData>* prepopulated_urls,
     size_t default_search_index,
     TemplateURLService::TemplateURLVector* template_urls,
-    TemplateURL** default_search_provider,
+    TemplateURL* default_search_provider,
     std::set<std::string>* removed_keyword_guids) {
   DCHECK(service == NULL || BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(prepopulated_urls);
   DCHECK(template_urls);
-  DCHECK(default_search_provider);
 
-  int default_prepopulated_id =
-      (*prepopulated_urls)[default_search_index]->prepopulate_id;
   ActionsFromPrepopulateData actions(CreateActionsFromCurrentPrepopulateData(
-      prepopulated_urls, *template_urls, *default_search_provider));
+      prepopulated_urls, *template_urls, default_search_provider));
 
   // Remove items.
   for (std::vector<TemplateURL*>::iterator i = actions.removed_engines.begin();
        i < actions.removed_engines.end(); ++i) {
     scoped_ptr<TemplateURL> template_url(*i);
     TemplateURLService::TemplateURLVector::iterator j =
         std::find(template_urls->begin(), template_urls->end(), template_url);
     DCHECK(j != template_urls->end());
-    DCHECK(*j != *default_search_provider);
+    DCHECK(!default_search_provider ||
+           (*j)->sync_guid() != default_search_provider->sync_guid());

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

How does sync have anything to do with this?  It seems like if we can't do a
pointer comparison any longer, we need to compare the whole TURL.

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

I will revisit this part of the implementation now.

[erikwright (departed), 2014/05/05 06:01:57]

In the end, if the prepopulate_id matches the DSE, the current properties of the
DSE will wipe out the properties we read here, anyway.

Those properties can come from the following source (in priority order):

(1) From the search_provider_overrides source that matches the prepopulate_id in
prefs.
(2) From the built-in prepopulated engine that matches the prepopulate_id in
prefs.
(3) From the DSE properties stored in prefs.

If the prefs say "not safe for autoreplace" we will always keep the keyword and
short name.

Since, in case (3), no prepopulate ID matched, it appears that this routine
should, in pretty much all circumstances, be expected to produce the same
properties (minus keyword and short name) that were selected by
DefaultSearchManager (and ended up being passed into this method via
default_search_provider).

So if we are going to eliminate a duplicate, and one of the entries has the same
keyword as the DSE, I guess we might as well keep that, because (other than
short name) all other properties will be wiped anyway.

     template_urls->erase(j);
     if (service) {
       service->RemoveKeyword(template_url->id());
       if (removed_keyword_guids)
         removed_keyword_guids->insert(template_url->sync_guid());
     }
   }
 
   // Edit items.
   for (EditedEngines::iterator i(actions.edited_engines.begin());
        i < actions.edited_engines.end(); ++i) {
     TemplateURLData& data = i->second;
     scoped_ptr<TemplateURL> existing_url(i->first);
     if (service)
       service->UpdateKeyword(data);
 
     // Replace the entry in |template_urls| with the updated one.
     TemplateURLService::TemplateURLVector::iterator j = std::find(
         template_urls->begin(), template_urls->end(), existing_url.get());
     *j = new TemplateURL(profile, data);
-    if (*default_search_provider == existing_url.get())
-      *default_search_provider = *j;
   }
 
   // Add items.
   for (std::vector<TemplateURLData>::const_iterator it =
            actions.added_engines.begin();
        it != actions.added_engines.end();
        ++it) {
     template_urls->push_back(new TemplateURL(profile, *it));
   }
-
-  if (!*default_search_provider) {
-    // The user had no existing default search provider, so set the
-    // default to the default prepopulated engine.
-    *default_search_provider = FindURLByPrepopulateID(*template_urls,
-                                                      default_prepopulated_id);
-  }
 }
 
 ActionsFromPrepopulateData CreateActionsFromCurrentPrepopulateData(
     ScopedVector<TemplateURLData>* prepopulated_urls,
     const TemplateURLService::TemplateURLVector& existing_urls,
     const TemplateURL* default_search_provider) {
   // Create a map to hold all provided |template_urls| that originally came from
   // prepopulate data (i.e. have a non-zero prepopulate_id()).
   typedef std::map<int, TemplateURL*> IDMap;
   IDMap id_to_turl;
@@ skipping 36 matching lines @@
   // Clear the pointers.
   prepopulated_urls->weak_erase(prepopulated_urls->begin(),
                                 prepopulated_urls->end());
 
   // The block above removed all the URLs from the |id_to_turl| map that were
   // found in the prepopulate data.  Any remaining URLs that haven't been
   // user-edited or made default can be removed from the data store.
   for (IDMap::iterator i(id_to_turl.begin()); i != id_to_turl.end(); ++i) {
     TemplateURL* template_url = i->second;
     if ((template_url->safe_for_autoreplace()) &&
-        (template_url != default_search_provider))
+        (!default_search_provider ||
+         template_url->sync_guid() != default_search_provider->sync_guid()))

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

Same comment.

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

Ditto.

       actions.removed_engines.push_back(template_url);
   }
 
   return actions;
 }
 
 void GetSearchProvidersUsingKeywordResult(
     const WDTypedResult& result,
     WebDataService* service,
     Profile* profile,
     TemplateURLService::TemplateURLVector* template_urls,
-    TemplateURL** default_search_provider,
+    TemplateURL* default_search_provider,
     int* new_resource_keyword_version,
     std::set<std::string>* removed_keyword_guids) {
   DCHECK(service == NULL || BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(template_urls);
   DCHECK(template_urls->empty());
-  DCHECK(default_search_provider);
-  DCHECK(*default_search_provider == NULL);
   DCHECK_EQ(KEYWORDS_RESULT, result.GetType());
   DCHECK(new_resource_keyword_version);
 
   WDKeywordsResult keyword_result = reinterpret_cast<
       const WDResult<WDKeywordsResult>*>(&result)->GetValue();
 
   for (KeywordTable::Keywords::iterator i(keyword_result.keywords.begin());
        i != keyword_result.keywords.end(); ++i) {
     // Fix any duplicate encodings in the local database.  Note that we don't
     // adjust the last_modified time of this keyword; this way, we won't later
     // overwrite any changes on the sync server that happened to this keyword
     // since the last time we synced.  Instead, we also run a de-duping pass on
     // the server-provided data in
     // TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData() and
     // update the server with the merged, de-duped results at that time.  We
     // still fix here, though, to correct problems in clients that have disabled
     // search engine sync, since in that case that code will never be reached.
     if (DeDupeEncodings(&i->input_encodings) && service)
       service->UpdateKeyword(*i);
     template_urls->push_back(new TemplateURL(profile, *i));
   }
 
-  int64 default_search_provider_id = keyword_result.default_search_provider_id;
-  if (default_search_provider_id) {
-    *default_search_provider =
-        GetTemplateURLByID(*template_urls, default_search_provider_id);
-  }
-
   *new_resource_keyword_version = keyword_result.builtin_keyword_version;
   GetSearchProvidersUsingLoadedEngines(service, profile, template_urls,
                                        default_search_provider,
                                        new_resource_keyword_version,
                                        removed_keyword_guids);
 }
 
 void GetSearchProvidersUsingLoadedEngines(
     WebDataService* service,
     Profile* profile,
     TemplateURLService::TemplateURLVector* template_urls,
-    TemplateURL** default_search_provider,
+    TemplateURL* default_search_provider,
     int* resource_keyword_version,
     std::set<std::string>* removed_keyword_guids) {
   DCHECK(service == NULL || BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(template_urls);
-  DCHECK(default_search_provider);
   DCHECK(resource_keyword_version);
   PrefService* prefs = profile ? profile->GetPrefs() : NULL;
   size_t default_search_index;
   ScopedVector<TemplateURLData> prepopulated_urls =
       TemplateURLPrepopulateData::GetPrepopulatedEngines(prefs,
                                                          &default_search_index);
   RemoveDuplicatePrepopulateIDs(service, prepopulated_urls,
-                                *default_search_provider, template_urls,
+                                default_search_provider, template_urls,
                                 removed_keyword_guids);
 
   const int prepopulate_resource_keyword_version =
       TemplateURLPrepopulateData::GetDataVersion(prefs);
   if (*resource_keyword_version < prepopulate_resource_keyword_version) {
     MergeEnginesFromPrepopulateData(profile, service, &prepopulated_urls,
         default_search_index, template_urls, default_search_provider,
         removed_keyword_guids);
     *resource_keyword_version = prepopulate_resource_keyword_version;
   } else {
     *resource_keyword_version = 0;
   }
 }
 
 bool DeDupeEncodings(std::vector<std::string>* encodings) {
   std::vector<std::string> deduped_encodings;
   std::set<std::string> encoding_set;
   for (std::vector<std::string>::const_iterator i(encodings->begin());
        i != encodings->end(); ++i) {
     if (encoding_set.insert(*i).second)
       deduped_encodings.push_back(*i);
   }
   encodings->swap(deduped_encodings);
   return encodings->size() != deduped_encodings.size();
 }