Fix TemplateUrl::MatchesData comparison of search_terms_replacement_key (reland)

components/search_engines/template_url_service.cc

Index: components/search_engines/template_url_service.cc
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index 0e77d27379f265169564fc3bb4cf7eff964c7a4f..2a8c241361958d8248828a1f668dc00aae5cbb67 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -678,6 +678,7 @@ void TemplateURLService::RepairPrepopulatedSearchEngines() {
   base::AutoReset<DefaultSearchChangeOrigin> change_origin(
       &dsp_change_origin_, DSP_CHANGE_PROFILE_RESET);
 
+  DefaultSearchManager::Source old_source = default_search_provider_source_;
   default_search_manager_.ClearUserSelectedDefaultSearchEngine();
 
   if (!default_search_provider_) {
@@ -691,6 +692,15 @@ void TemplateURLService::RepairPrepopulatedSearchEngines() {
         default_search_manager_.GetDefaultSearchEngine(&source);
     // ApplyDefaultSearchChange will notify observers once it is done.
     ApplyDefaultSearchChange(new_dse, source);
+  } else if (old_source == DefaultSearchManager::FROM_USER) {

[vasilii, 2017/02/07 17:12:56]

Your reasoning sounds good. It seems that we should call
SetUserSelectedDefaultSearchEngine unconditionally. I don't understand why
|old_source| matters here.

+    // We expect that if old default source was FROM_USER we could only get to
+    // fallback engine source after ClearUserSelectedDefaultSearchEngine.
+    DCHECK(default_search_provider_source_ ==
+        DefaultSearchManager::FROM_FALLBACK);
+    // Set fallback engine as user selected, because repair is considered a user
+    // action and the new engine is expected to sync to other devices.
+    default_search_manager_.SetUserSelectedDefaultSearchEngine(
+        default_search_provider_->data());
   } else {
     NotifyObservers();
   }
@@ -1937,6 +1947,11 @@ bool TemplateURLService::ApplyDefaultSearchChangeNoMetrics(
         source == DefaultSearchManager::FROM_POLICY ? data : nullptr);
   }
 
+  // |default_search_provider_source_| must be set before calling
+  // UpdateNoNotify(), since that function needs to know the source of the
+  // update in question.
+  default_search_provider_source_ = source;
+
   if (!data) {
     default_search_provider_ = nullptr;
   } else if (source == DefaultSearchManager::FROM_EXTENSION) {
@@ -1987,11 +2002,8 @@ bool TemplateURLService::ApplyDefaultSearchChangeNoMetrics(
       prefs_->SetString(prefs::kSyncedDefaultSearchProviderGUID,
                         default_search_provider_->sync_guid());
     }
-
   }
 
-  default_search_provider_source_ = source;
-
   bool changed = default_search_provider_ != previous_default_search_engine;
   if (changed)
     RequestGoogleURLTrackerServerCheckIfNecessary();