Consolidate places where a TemplateURL is removed and then added to the TemplateURLModel.

Also made TemplateURLModel::Update private since there were no
external users of this method.

BUG=None
TEST=unit_tests --gtest_filter=TemplateURL*.*

Review URL: http://codereview.chromium.org/3202005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57143 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 51 insertions, 59 deletions

diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc
index 39abf87..7c46720 100644
--- a/chrome/browser/search_engines/template_url_model.cc
+++ b/chrome/browser/search_engines/template_url_model.cc
@@ -295,36 +295,6 @@ void TemplateURLModel::RemoveAutoGeneratedBetween(Time created_after,
   }
 }
 
-void TemplateURLModel::Replace(const TemplateURL* existing_turl,
-                               TemplateURL* new_turl) {
-  DCHECK(existing_turl && new_turl);
-
-  TemplateURLVector::iterator i = find(template_urls_.begin(),
-                                       template_urls_.end(),
-                                       existing_turl);
-  DCHECK(i != template_urls_.end());
-  RemoveFromMaps(existing_turl);
-  template_urls_.erase(i);
-
-  new_turl->set_id(existing_turl->id());
-
-  template_urls_.push_back(new_turl);
-  AddToMaps(new_turl);
-
-  if (service_.get())
-    service_->UpdateKeyword(*new_turl);
-
-  if (default_search_provider_ == existing_turl)
-    SetDefaultSearchProvider(new_turl);
-
-  if (loaded_) {
-    FOR_EACH_OBSERVER(TemplateURLModelObserver, model_observers_,
-                      OnTemplateURLModelChanged());
-  }
-
-  delete existing_turl;
-}
-
 void TemplateURLModel::RemoveAutoGeneratedSince(Time created_after) {
   RemoveAutoGeneratedBetween(created_after, Time());
 }
@@ -343,7 +313,7 @@ void TemplateURLModel::RegisterExtensionKeyword(Extension* extension) {
   const TemplateURL* existing_url = GetTemplateURLForExtension(extension);
   std::wstring keyword = UTF8ToWide(extension->omnibox_keyword());
 
-  TemplateURL* template_url = new TemplateURL;
+  scoped_ptr<TemplateURL> template_url(new TemplateURL);
   template_url->set_short_name(UTF8ToWide(extension->name()));
   template_url->set_keyword(keyword);
   // This URL is not actually used for navigation. It holds the extension's
@@ -356,9 +326,9 @@ void TemplateURLModel::RegisterExtensionKeyword(Extension* extension) {
   if (existing_url) {
     // TODO(mpcomplete): only replace if the user hasn't changed the keyword.
     // (We don't have UI for that yet).
-    Replace(existing_url, template_url);
+    Update(existing_url, *template_url);
   } else {
-    Add(template_url);
+    Add(template_url.release());
   }
 }
 
@@ -395,26 +365,18 @@ void TemplateURLModel::ResetTemplateURL(const TemplateURL* url,
                                         const std::wstring& title,
                                         const std::wstring& keyword,
                                         const std::string& search_url) {
-  DCHECK(url && find(template_urls_.begin(), template_urls_.end(), url) !=
-         template_urls_.end());
-  RemoveFromMaps(url);
-  TemplateURL* modifiable_url = const_cast<TemplateURL*>(url);
-  modifiable_url->set_short_name(title);
-  modifiable_url->set_keyword(keyword);
-  if ((modifiable_url->url() && search_url.empty()) ||
-      (!modifiable_url->url() && !search_url.empty()) ||
-      (modifiable_url->url() && modifiable_url->url()->url() != search_url)) {
+  TemplateURL new_url(*url);
+  new_url.set_short_name(title);
+  new_url.set_keyword(keyword);
+  if ((new_url.url() && search_url.empty()) ||
+      (!new_url.url() && !search_url.empty()) ||
+      (new_url.url() && new_url.url()->url() != search_url)) {
     // The urls have changed, reset the favicon url.
-    modifiable_url->SetFavIconURL(GURL());
-    modifiable_url->SetURL(search_url, 0, 0);
+    new_url.SetFavIconURL(GURL());
+    new_url.SetURL(search_url, 0, 0);
   }
-  modifiable_url->set_safe_for_autoreplace(false);
-  AddToMaps(url);
-  if (service_.get())
-    service_.get()->UpdateKeyword(*url);
-
-  FOR_EACH_OBSERVER(TemplateURLModelObserver, model_observers_,
-                    OnTemplateURLModelChanged());
+  new_url.set_safe_for_autoreplace(false);
+  Update(url, new_url);
 }
 
 void TemplateURLModel::SetDefaultSearchProvider(const TemplateURL* url) {
@@ -808,7 +770,7 @@ void TemplateURLModel::MergeEnginesFromPrepopulateData() {
             existing_url->autogenerate_keyword());
         t_url->set_short_name(existing_url->short_name());
       }
-      Replace(existing_url, t_url.release());
+      Update(existing_url, *t_url);
       id_to_turl.erase(existing_url_iter);
     } else {
       Add(t_url.release());
@@ -953,6 +915,38 @@ bool TemplateURLModel::CanReplace(const TemplateURL* t_url) {
           t_url->safe_for_autoreplace());
 }
 
+void TemplateURLModel::Update(const TemplateURL* existing_turl,
+                              const TemplateURL& new_values) {
+  DCHECK(existing_turl);
+  DCHECK(find(template_urls_.begin(), template_urls_.end(), existing_turl) !=
+         template_urls_.end());
+
+  RemoveFromMaps(existing_turl);
+
+  // Use the information from new_values but preserve existing_turl's id.
+  TemplateURL::IDType previous_id = existing_turl->id();
+  TemplateURL* modifiable_turl = const_cast<TemplateURL*>(existing_turl);
+  *modifiable_turl = new_values;
+  modifiable_turl->set_id(previous_id);
+
+  AddToMaps(existing_turl);
+
+  if (service_.get())
+    service_->UpdateKeyword(*existing_turl);
+
+  if (default_search_provider_ == existing_turl) {
+    // Force an update to happen to account for any changes
+    // that occurred during the update.
+    default_search_provider_ = NULL;
+    SetDefaultSearchProvider(existing_turl);
+  }
+
+  if (loaded_) {
+    FOR_EACH_OBSERVER(TemplateURLModelObserver, model_observers_,
+                      OnTemplateURLModelChanged());
+  }
+}
+
 PrefService* TemplateURLModel::GetPrefs() {
   return profile_ ? profile_->GetPrefs() : NULL;
 }
diff --git a/chrome/browser/search_engines/template_url_model.h b/chrome/browser/search_engines/template_url_model.h
index c8ad1e3..6559bb0d 100644
--- a/chrome/browser/search_engines/template_url_model.h
+++ b/chrome/browser/search_engines/template_url_model.h
@@ -131,13 +131,6 @@ class TemplateURLModel : public WebDataServiceConsumer,
   void RemoveAutoGeneratedBetween(base::Time created_after,
                                   base::Time created_before);
 
-  // Replaces existing_turl with new_turl. new_turl is given the same ID as
-  // existing_turl. If existing_turl was the default, new_turl is made the
-  // default. After this call existing_turl is deleted. As with Add,
-  // TemplateURLModel takes ownership of existing_turl.
-  void Replace(const TemplateURL* existing_turl,
-               TemplateURL* new_turl);
-
   // Removes all auto-generated keywords that were created on or after the
   // date passed in.
   void RemoveAutoGeneratedSince(base::Time created_after);
@@ -311,6 +304,11 @@ class TemplateURLModel : public WebDataServiceConsumer,
   // in the default list and is marked as safe_for_autoreplace.
   bool CanReplace(const TemplateURL* t_url);
 
+  // Updates the information in |existing_turl| using the information from
+  // |new_values|, but the ID for |existing_turl| is retained.
+  void Update(const TemplateURL* existing_turl,
+              const TemplateURL& new_values);
+
   // Returns the preferences we use.
   PrefService* GetPrefs();