Implemented 'Reset Search engines' feature.

chrome/browser/search_engines/template_url_service.cc

Index: chrome/browser/search_engines/template_url_service.cc
diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc
index 63cf81f88fab78243cfafb62aa7726cdc7867772..8a28c32d637ccf23ed70b75af0993c0ddd1a89ef 100644
--- a/chrome/browser/search_engines/template_url_service.cc
+++ b/chrome/browser/search_engines/template_url_service.cc
@@ -714,7 +714,6 @@ void TemplateURLService::OnWebDataServiceRequestDone(
   // initial_default_search_provider_ is only needed before we've finished
   // loading. Now that we've loaded we can nuke it.
   initial_default_search_provider_.reset();
-  is_default_search_managed_ = false;
 
   TemplateURLVector template_urls;
   TemplateURL* default_search_provider = NULL;
@@ -723,6 +722,31 @@ void TemplateURLService::OnWebDataServiceRequestDone(
       &template_urls, &default_search_provider, &new_resource_keyword_version,
       &pre_sync_deletes_);
 
+  AddTemplateURLsAndSetupDefaultEngine(&template_urls, default_search_provider);
+
+  // This initializes provider_map_ which should be done before
+  // calling UpdateKeywordSearchTermsForURL.
+  ChangeToLoadedState();
+
+  // Index any visits that occurred before we finished loading.
+  for (size_t i = 0; i < visits_to_add_.size(); ++i)
+    UpdateKeywordSearchTermsForURL(visits_to_add_[i]);
+  visits_to_add_.clear();
+
+  if (new_resource_keyword_version)
+    service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
+
+  EnsureDefaultSearchProviderExists();
+
+  NotifyObservers();
+  NotifyLoaded();
+}
+
+void TemplateURLService::AddTemplateURLsAndSetupDefaultEngine(
+    TemplateURLVector* template_urls,
+    TemplateURL* default_search_provider) {
+  DCHECK(template_urls);
+  is_default_search_managed_ = false;
   bool database_specified_a_default = (default_search_provider != NULL);
 
   // Check if default search provider is now managed.
@@ -732,14 +756,14 @@ void TemplateURLService::OnWebDataServiceRequestDone(
 
   // Remove entries that were created because of policy as they may have
   // changed since the database was saved.
-  RemoveProvidersCreatedByPolicy(&template_urls,
+  RemoveProvidersCreatedByPolicy(template_urls,
                                  &default_search_provider,
                                  default_from_prefs.get());
 
-  PatchMissingSyncGUIDs(&template_urls);
+  PatchMissingSyncGUIDs(template_urls);
 
   if (is_default_search_managed_) {
-    SetTemplateURLs(template_urls);
+    SetTemplateURLs(*template_urls);
 
     if (TemplateURLsHaveSamePrefs(default_search_provider,
                                   default_from_prefs.get())) {
@@ -760,7 +784,7 @@ void TemplateURLService::OnWebDataServiceRequestDone(
     // Note that this saves the default search provider to prefs.
     if (!default_search_provider ||
         (!default_search_provider->IsExtensionKeyword() &&
-         default_search_provider->SupportsReplacement())) {
+            default_search_provider->SupportsReplacement())) {
       bool success = SetDefaultSearchProviderNoNotify(default_search_provider);
       DCHECK(success);
     }
@@ -772,10 +796,11 @@ void TemplateURLService::OnWebDataServiceRequestDone(
       default_search_provider = synced_default;
       pending_synced_default_search_ = false;
     } else if (database_specified_a_default &&
-               default_search_provider == NULL) {
+        default_search_provider == NULL) {
       UMA_HISTOGRAM_ENUMERATION(kFirstPotentialEngineHistogramName,
-          FIRST_POTENTIAL_CALLSITE_ON_LOAD, FIRST_POTENTIAL_CALLSITE_MAX);
-      default_search_provider = FirstPotentialDefaultEngine(template_urls);
+                                FIRST_POTENTIAL_CALLSITE_ON_LOAD,
+                                FIRST_POTENTIAL_CALLSITE_MAX);
+      default_search_provider = FirstPotentialDefaultEngine(*template_urls);
     }
 
     // If the default search provider existed previously, then just
@@ -786,7 +811,7 @@ void TemplateURLService::OnWebDataServiceRequestDone(
       default_search_provider_ = default_search_provider;
       default_search_provider = NULL;
     }
-    SetTemplateURLs(template_urls);
+    SetTemplateURLs(*template_urls);
 
     if (default_search_provider) {
       // Note that this saves the default search provider to prefs.
@@ -798,21 +823,11 @@ void TemplateURLService::OnWebDataServiceRequestDone(
         SaveDefaultSearchProviderToPrefs(default_search_provider_);
     }
   }
+}
 
-  // This initializes provider_map_ which should be done before
-  // calling UpdateKeywordSearchTermsForURL.
-  ChangeToLoadedState();
-
-  // Index any visits that occurred before we finished loading.
-  for (size_t i = 0; i < visits_to_add_.size(); ++i)
-    UpdateKeywordSearchTermsForURL(visits_to_add_[i]);
-  visits_to_add_.clear();
-
-  if (new_resource_keyword_version)
-    service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
-
+void TemplateURLService::EnsureDefaultSearchProviderExists() {
   if (!is_default_search_managed_) {
-    bool has_default_search_provider = default_search_provider_ != NULL &&
+    bool has_default_search_provider = default_search_provider_ &&
         default_search_provider_->SupportsReplacement();
     UMA_HISTOGRAM_BOOLEAN(kHasDSPHistogramName,
                           has_default_search_provider);
@@ -831,9 +846,6 @@ void TemplateURLService::OnWebDataServiceRequestDone(
           TemplateURLPrepopulateData::kMaxPrepopulatedEngineID);
     }
   }
-
-  NotifyObservers();
-  NotifyLoaded();
 }
 
 string16 TemplateURLService::GetKeywordShortName(const string16& keyword,
@@ -1586,6 +1598,15 @@ void TemplateURLService::NotifyLoaded() {
       content::NotificationService::NoDetails());
 }
 
+void TemplateURLService::ClearDefaultProviderFromPrefs() {

[Peter Kasting, 2013/05/22 21:22:57]

Function definition order must match declaration order (several places)

[vasilii, 2013/05/23 17:03:42]

Done.

+  // We overwrite user preferences. If the default search engine is managed,
+  // there is no effect.
+  SaveDefaultSearchProviderToPrefs(NULL);
+  // Default value for kDefaultSearchProviderEnabled is true.
+  if (PrefService* prefs = GetPrefs())

[Peter Kasting, 2013/05/22 21:22:57]

Nit: Do not use side-effect statements as conditions.  Do the assignment and
then the conditional.

[vasilii, 2013/05/23 17:03:42]

Done.

+    prefs->SetBoolean(prefs::kDefaultSearchProviderEnabled, true);
+}
+
 void TemplateURLService::SaveDefaultSearchProviderToPrefs(
     const TemplateURL* t_url) {
   PrefService* prefs = GetPrefs();
@@ -2497,3 +2518,45 @@ void TemplateURLService::PatchMissingSyncGUIDs(
     }
   }
 }
+
+bool TemplateURLService::ResetNonExtensionURLs() {

[Peter Kasting, 2013/05/22 21:22:57]

This function never returns false.  Why does it return a bool?

[vasilii, 2013/05/23 17:03:42]

Done.

+  // Can't clean DB if it hasn't been loaded.
+  DCHECK(loaded());
+  TemplateURLVector entries_to_process;

[Peter Kasting, 2013/05/22 21:22:57]

Nit: In general, this function could do with separating distinct functional
blocks with blank lines.

[vasilii, 2013/05/23 17:03:42]

Done.

+  for (TemplateURLVector::const_iterator i = template_urls_.begin();
+       i != template_urls_.end(); ++i) {
+    if (!(**i).IsExtensionKeyword())

[Peter Kasting, 2013/05/22 21:22:57]

Nit: (*i)->

[vasilii, 2013/05/23 17:03:42]

Done.

+      entries_to_process.push_back(*i);
+  }
+  // Clear default provider to be able to delete it.
+  default_search_provider_ = NULL;

[Peter Kasting, 2013/05/22 21:22:57]

If the DSP was managed, don't we need to preserve the TURL it pointed to, in
case that one isn't in the prepopulated list?  Or are we guaranteed the data on
that engine is recoverable from somewhere?

[vasilii, 2013/05/23 17:03:42]

Yes. It's stored in managed prefs. AddTemplateURLsAndSetupDefaultEngine loads,
adds and set it as default. See lines 2471, 2497, 2505. This case is covered by
TemplateURLServiceTest.ResetURLsWithManagedDefault.

[Peter Kasting, 2013/05/29 04:04:41]

OK, good.  Consider adding a comment about this here for the benefit of future
readers of the code.

+  // Remove non-extension keywords

[Peter Kasting, 2013/05/22 21:22:57]

Nit: Trailing period

[vasilii, 2013/05/23 17:03:42]

Done.

+  for (TemplateURLVector::const_iterator i = entries_to_process.begin();
+       i != entries_to_process.end(); ++i) {

[Peter Kasting, 2013/05/22 21:22:57]

Nit: No {}

[vasilii, 2013/05/23 17:03:42]

Done.

+    RemoveNoNotify(*i);
+  }
+  // Store the remaining engines in entries_to_process and merge them with
+  // prepopulated ones.
+  entries_to_process.clear();
+  entries_to_process.swap(template_urls_);
+  provider_map_.reset(new SearchHostToURLsMap);
+  UIThreadSearchTermsData search_terms_data(profile_);
+  provider_map_->Init(TemplateURLVector(), search_terms_data);
+  TemplateURL* default_search_provider = NULL;
+  // This will force to merge prepopulated engines.

[Peter Kasting, 2013/05/22 21:22:57]

Nit: Grammar (I don't actually know what this sentence is supopsed to mean so I
can't suggest a better one)

[vasilii, 2013/05/23 17:03:42]

Done.

+  int new_resource_keyword_version = 0;
+  GetSearchProvidersUsingLoadedEngines(service_.get(), profile_,
+                                       &entries_to_process,
+                                       &default_search_provider,
+                                       &new_resource_keyword_version,
+                                       &pre_sync_deletes_);
+  // Setup search engines and a default one.
+  AddTemplateURLsAndSetupDefaultEngine(&entries_to_process,
+                                       default_search_provider);
+  if (new_resource_keyword_version)
+    service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
+  // Make sure the default search engine != NULL;

[Peter Kasting, 2013/05/22 21:22:57]

Nit: This comment adds nothing to the code.

[vasilii, 2013/05/23 17:03:42]

Done.

+  EnsureDefaultSearchProviderExists();
+  NotifyObservers();
+  return true;
+}