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

chrome/browser/search_engines/util.cc

Index: chrome/browser/search_engines/util.cc
diff --git a/chrome/browser/search_engines/util.cc b/chrome/browser/search_engines/util.cc
index afb3a53e82b1fac8cbf00f4ac92b0105ca56e32b..d08eb950aeeed30b1d9382943a43b01eb5f96a38 100644
--- a/chrome/browser/search_engines/util.cc
+++ b/chrome/browser/search_engines/util.cc
@@ -99,7 +99,8 @@ void RemoveDuplicatePrepopulateIDs(
     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;
       }
@@ -191,17 +192,14 @@ void MergeEnginesFromPrepopulateData(
     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();
@@ -210,7 +208,8 @@ void MergeEnginesFromPrepopulateData(
     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());
@@ -231,8 +230,6 @@ void MergeEnginesFromPrepopulateData(
     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.
@@ -242,13 +239,6 @@ void MergeEnginesFromPrepopulateData(
        ++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(
@@ -305,7 +295,8 @@ ActionsFromPrepopulateData CreateActionsFromCurrentPrepopulateData(
   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);
   }
 
@@ -317,14 +308,12 @@ void GetSearchProvidersUsingKeywordResult(
     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);
 
@@ -347,12 +336,6 @@ void GetSearchProvidersUsingKeywordResult(
     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,
@@ -364,12 +347,11 @@ 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;
@@ -377,7 +359,7 @@ void GetSearchProvidersUsingLoadedEngines(
       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 =