Fix duplicate insertions of default search providers into the Web Data database.

chrome/browser/search_engines/template_url_model.cc

 // Copyright (c) 2011 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/template_url_model.h"
 
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/i18n/case_conversion.h"
 #include "base/stl_util-inl.h"
@@ skipping 21 matching lines @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/common/notification_service.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using base::Time;
 typedef SearchHostToURLsMap::TemplateURLSet TemplateURLSet;
 
+namespace {
+
 // String in the URL that is replaced by the search term.
-static const char kSearchTermParameter[] = "{searchTerms}";
+const char kSearchTermParameter[] = "{searchTerms}";
 
 // String in Initializer that is replaced with kSearchTermParameter.
-static const char kTemplateParameter[] = "%s";
+const char kTemplateParameter[] = "%s";
 
 // Term used when generating a search url. Use something obscure so that on
 // the rare case the term replaces the URL it's unlikely another keyword would
 // have the same url.
-static const char kReplacementTerm[] = "blah.blah.blah.blah.blah";
+const char kReplacementTerm[] = "blah.blah.blah.blah.blah";
 
+bool TemplateURLsHaveSamePrefs(const TemplateURL* url1,
+                               const TemplateURL* url2) {
+  if (url1 == url2)
+    return true;
+  return NULL != url1 &&
+      NULL != url2 &&
+      url1->short_name() == url2->short_name() &&
+      url1->keyword() == url2->keyword() &&
+      TemplateURLRef::SameUrlRefs(url1->url(), url2->url()) &&
+      TemplateURLRef::SameUrlRefs(url1->suggestions_url(),
+                                  url2->suggestions_url()) &&
+      url1->GetFaviconURL() == url2->GetFaviconURL() &&
+      url1->safe_for_autoreplace() == url2->safe_for_autoreplace() &&
+      url1->show_in_default_list() == url2->show_in_default_list() &&
+      url1->input_encodings() == url2->input_encodings();
+}
 
-// Removes from the vector any template URL that was created because of
-// policy.  These TemplateURLs are freed.
-// Sets default_search_provider to NULL if it was one of them.
-static void RemoveProvidersCreatedByPolicy(
-    std::vector<TemplateURL*>* template_urls,
-    const TemplateURL** default_search_provider) {
-  DCHECK(template_urls);
-  DCHECK(default_search_provider);
-  for (std::vector<TemplateURL*>::iterator i = template_urls->begin();
-      i != template_urls->end(); ) {
-    TemplateURL* template_url = *i;
-    if (template_url->created_by_policy()) {
-      if (*default_search_provider &&
-          (*default_search_provider)->id() == template_url->id())
-        *default_search_provider = NULL;
-      i = template_urls->erase(i);
-      delete template_url;
-    } else {
-      ++i;
-    }
-  }
-}
+}  // namespace
+
 
 class TemplateURLModel::LessWithPrefix {
  public:
   // We want to find the set of keywords that begin with a prefix.  The STL
   // algorithms will return the set of elements that are "equal to" the
   // prefix, where "equal(x, y)" means "!(cmp(x, y) || cmp(y, x))".  When
   // cmp() is the typical std::less<>, this results in lexicographic equality;
   // we need to extend this to mark a prefix as "not less than" a keyword it
   // begins, which will cause the desired elements to be considered "equal to"
   // the prefix.  Note: this is still a strict weak ordering, as required by
@@ skipping 392 matching lines @@
   int new_resource_keyword_version = 0;
   GetSearchProvidersUsingKeywordResult(*result,
                                        service_.get(),
                                        GetPrefs(),
                                        &template_urls,
                                        &default_search_provider,
                                        &new_resource_keyword_version);
 
   bool database_specified_a_default = NULL != default_search_provider;
 
-  // Remove entries that were created because of policy as they may have
-  // changed since the database was saved.
-  RemoveProvidersCreatedByPolicy(&template_urls, &default_search_provider);
-
   // Check if default search provider is now managed.
   scoped_ptr<TemplateURL> default_from_prefs;
   LoadDefaultSearchProviderFromPrefs(&default_from_prefs,
                                      &is_default_search_managed_);
 
+  // Remove entries that were created because of policy as they may have
+  // changed since the database was saved.
+  RemoveProvidersCreatedByPolicy(&template_urls,
+                                 &default_search_provider,
+                                 default_from_prefs.get());
+
   if (is_default_search_managed_) {
     SetTemplateURLs(template_urls);
-    // Set the default.  AddNoNotify will take ownership of default_from_prefs
-    // so it is safe to release.  If it's null, there's no ownership to worry
-    // about :-)
-    TemplateURL* managed_default = default_from_prefs.release();
-    if (managed_default) {
-      managed_default->set_created_by_policy(true);
-      managed_default->set_id(0);
-      AddNoNotify(managed_default);
+
+    if (TemplateURLsHaveSamePrefs(default_search_provider,
+                                  default_from_prefs.get())) {
+      // The value from the preferences was previously stored in the database.
+      // Reuse it.
+    } else {
+      // The value from the preferences takes over.
+      //
+      // AddNoNotify will take ownership of default_from_prefs so it is safe to
+      // release. If it's null, there's no ownership to worry about :-)
+      TemplateURL* managed_default = default_from_prefs.release();
+      if (managed_default) {
+        managed_default->set_created_by_policy(true);
+        managed_default->set_id(0);
+        AddNoNotify(managed_default);
+        default_search_provider = managed_default;
+      }
     }
     // Note that this saves the default search provider to prefs.
-    SetDefaultSearchProviderNoNotify(managed_default);
+    SetDefaultSearchProviderNoNotify(default_search_provider);
   } else {
     // If we had a managed default, replace it with the first provider of
     // the list.
     if (database_specified_a_default &&
         NULL == default_search_provider &&
         !template_urls.empty())
       default_search_provider = template_urls[0];
 
     // If the default search provider existed previously, then just
     // set the member variable. Otherwise, we'll set it using the method
@@ skipping 354 matching lines @@
   }
   if (!prepopulate_id.empty() && !*is_managed) {
     int value;
     base::StringToInt(prepopulate_id, &value);
     (*default_provider)->set_prepopulate_id(value);
   }
   (*default_provider)->set_show_in_default_list(true);
   return true;
 }
 
-static bool TemplateURLsHaveSamePrefs(const TemplateURL* url1,
-                                      const TemplateURL* url2) {
-  if (url1 == url2)
-    return true;
-  return NULL != url1 &&
-      NULL != url2 &&
-      url1->short_name() == url2->short_name() &&
-      url1->keyword() == url2->keyword() &&
-      TemplateURLRef::SameUrlRefs(url1->url(), url2->url()) &&
-      TemplateURLRef::SameUrlRefs(url1->suggestions_url(),
-                                  url2->suggestions_url()) &&
-      url1->GetFaviconURL() == url2->GetFaviconURL() &&
-      url1->safe_for_autoreplace() == url2->safe_for_autoreplace() &&
-      url1->show_in_default_list() == url2->show_in_default_list() &&
-      url1->input_encodings() == url2->input_encodings();
-}
-
-
 bool TemplateURLModel::CanReplaceKeywordForHost(
     const std::string& host,
     const TemplateURL** to_replace) {
   const TemplateURLSet* urls = provider_map_.GetURLsForHost(host);
   if (urls) {
     for (TemplateURLSet::const_iterator i = urls->begin();
          i != urls->end(); ++i) {
       const TemplateURL* url = *i;
       if (CanReplace(url)) {
         if (to_replace)
@@ skipping 363 matching lines @@
   delete template_url;
 }
 
 void TemplateURLModel::NotifyObservers() {
   if (!loaded_)
     return;
 
   FOR_EACH_OBSERVER(TemplateURLModelObserver, model_observers_,
                     OnTemplateURLModelChanged());
 }
+
+// |template_urls| are the TemplateURLs loaded from the database.
+// |default_search_provider| points to one of them, if it was set in the db.
+// |default_from_prefs| is the default search provider from the preferences.
+// Check |is_default_search_managed_| to determine if it was set by policy.
+//
+// This function removes from the vector and the database all the TemplateURLs
+// that were set by policy, unless it is the current default search provider
+// and matches what is set by a managed preference.
+void TemplateURLModel::RemoveProvidersCreatedByPolicy(
+    std::vector<TemplateURL*>* template_urls,
+    const TemplateURL** default_search_provider,
+    const TemplateURL* default_from_prefs) {
+  DCHECK(template_urls);
+  DCHECK(default_search_provider);
+  for (std::vector<TemplateURL*>::iterator i = template_urls->begin();
+      i != template_urls->end(); ) {
+    TemplateURL* template_url = *i;
+    if (template_url->created_by_policy()) {
+      if (template_url == *default_search_provider &&
+          is_default_search_managed_ &&
+          TemplateURLsHaveSamePrefs(template_url,
+                                    default_from_prefs)) {
+        // If the database specified a default search provider that was set
+        // by policy, and the default search provider from the preferences
+        // is also set by policy and they are the same, keep the entry in the
+        // database and the |default_search_provider|.
+        ++i;
+        continue;
+      }
+
+      // The database loaded a managed |default_search_provider|, but it has
+      // been updated in the prefs. Remove it from the database, and update the
+      // |default_search_provider| pointer here.
+      if (*default_search_provider &&
+          (*default_search_provider)->id() == template_url->id())
+        *default_search_provider = NULL;
+
+      i = template_urls->erase(i);
+      if (service_.get())
+        service_->RemoveKeyword(*template_url);
+      delete template_url;
+    } else {
+      ++i;
+    }
+  }
+}