Move most TemplateURL data members to a new struct, TemplateURLData. This allows us to eliminate t…

chrome/browser/search_engines/util.cc

Index: chrome/browser/search_engines/util.cc
===================================================================
--- chrome/browser/search_engines/util.cc	(revision 130759)
+++ chrome/browser/search_engines/util.cc	(working copy)
@@ -53,7 +53,7 @@
     if (prepopulate_id) {
       if (ids.find(prepopulate_id) != ids.end()) {
         if (service)
-          service->RemoveKeyword(**i);
+          service->RemoveKeyword((*i)->id());
         delete *i;
         i = template_urls->erase(i);
       } else {
@@ -91,7 +91,8 @@
   DCHECK(template_urls);
   DCHECK(default_search_provider);
 
-  // Build a map from prepopulate id to TemplateURL of existing urls.
+  // Create a map to hold all provided |template_urls| that originally came from
+  // prepopulate data (i.e. have a non-zero prepopulate_id()).
   typedef std::map<int, TemplateURL*> IDMap;
   IDMap id_to_turl;
   for (std::vector<TemplateURL*>::iterator i(template_urls->begin());
@@ -101,66 +102,69 @@
       id_to_turl[prepopulate_id] = *i;
   }
 
+  // Get the current set of prepopulatd URLs.
   std::vector<TemplateURL*> prepopulated_urls;
   size_t default_search_index;
   TemplateURLPrepopulateData::GetPrepopulatedEngines(prefs,
-                                                     &prepopulated_urls,
-                                                     &default_search_index);
+      &prepopulated_urls, &default_search_index);
 
-  std::set<int> updated_ids;
+  // For each current prepopulated URL, check whether |template_urls| contained
+  // a matching prepopulated URL.  If so, update the passed-in URL to match the
+  // current data.  (If the passed-in URL was user-edited, we persist the user's
+  // name and keyword.)  If not, add the prepopulated URL to |template_urls|.
+  // Along the way, point |default_search_provider| at the default prepopulated
+  // URL, if the user hasn't already set another URL as default.
   for (size_t i = 0; i < prepopulated_urls.size(); ++i) {
     // We take ownership of |prepopulated_urls[i]|.
     scoped_ptr<TemplateURL> prepopulated_url(prepopulated_urls[i]);
     const int prepopulated_id = prepopulated_url->prepopulate_id();
-    if (!prepopulated_id || updated_ids.count(prepopulated_id)) {
-      // Prepopulate engines need a unique id.
-      NOTREACHED();
-      continue;
-    }
+    DCHECK_NE(0, prepopulated_id);
 
-    TemplateURL* existing_url = NULL;
+    TemplateURL* url_in_vector = NULL;
     IDMap::iterator existing_url_iter(id_to_turl.find(prepopulated_id));
     if (existing_url_iter != id_to_turl.end()) {
-      existing_url = existing_url_iter->second;
+      // Update the data store with the new prepopulated data.  Preserve user
+      // edits to the name and keyword.
+      TemplateURLData data(prepopulated_url->data());
+      scoped_ptr<TemplateURL> existing_url(existing_url_iter->second);
+      id_to_turl.erase(existing_url_iter);
       if (!existing_url->safe_for_autoreplace()) {
-        // User edited the entry, preserve the keyword and description.
-        prepopulated_url->set_safe_for_autoreplace(false);
-        prepopulated_url->set_keyword(existing_url->keyword());
-        prepopulated_url->set_autogenerate_keyword(
-            existing_url->autogenerate_keyword());
-        prepopulated_url->set_short_name(existing_url->short_name());
+        data.safe_for_autoreplace = false;
+        data.SetKeyword(existing_url->keyword());
+        data.SetAutogenerateKeyword(existing_url->autogenerate_keyword());
+        data.short_name = existing_url->short_name();
       }
-      prepopulated_url->set_id(existing_url->id());
+      data.id = existing_url->id();
+      url_in_vector = new TemplateURL(data);
+      if (service)
+        service->UpdateKeyword(*url_in_vector);
 
-      *existing_url = *prepopulated_url;
-      if (service) {
-        service->UpdateKeyword(*existing_url);
-      }
-      id_to_turl.erase(existing_url_iter);
+      // Replace the entry in |template_urls| with the updated one.
+      std::vector<TemplateURL*>::iterator j = std::find(template_urls->begin(),
+          template_urls->end(), existing_url.get());
+      *j = url_in_vector;
     } else {
-      existing_url = prepopulated_url.get();
       template_urls->push_back(prepopulated_url.release());
+      url_in_vector = template_urls->back();
     }
-    DCHECK(existing_url);
+    DCHECK(url_in_vector);
     if (i == default_search_index && !*default_search_provider)
-      *default_search_provider = existing_url;
-
-    updated_ids.insert(prepopulated_id);
+      *default_search_provider = url_in_vector;
   }
 
-  // Remove any prepopulated engines which are no longer in the master list, as
-  // long as the user hasn't modified them or made them the default engine.
+  // The block above removed all the URLs from the |id_to_turl| map that were
+  // found in the prepopulate data.  Any remaining URLs that haven't been
+  // user-edited or made default can be removed from the data store.
   for (IDMap::iterator i(id_to_turl.begin()); i != id_to_turl.end(); ++i) {
     const TemplateURL* template_url = i->second;
     if ((template_url->safe_for_autoreplace()) &&
         (template_url != *default_search_provider)) {
-      std::vector<TemplateURL*>::iterator i = std::find(template_urls->begin(),
-                                                        template_urls->end(),
-                                                        template_url);
-      DCHECK(i != template_urls->end());
-      template_urls->erase(i);
+      std::vector<TemplateURL*>::iterator j =
+          std::find(template_urls->begin(), template_urls->end(), template_url);
+      DCHECK(j != template_urls->end());
+      template_urls->erase(j);
        if (service)
-         service->RemoveKeyword(*template_url);
+         service->RemoveKeyword(template_url->id());
       delete template_url;
     }
   }
@@ -185,7 +189,10 @@
   WDKeywordsResult keyword_result = reinterpret_cast<
       const WDResult<WDKeywordsResult>*>(&result)->GetValue();
 
-  template_urls->swap(keyword_result.keywords);
+  for (KeywordTable::Keywords::const_iterator i(
+       keyword_result.keywords.begin()); i != keyword_result.keywords.end();
+       ++i)
+    template_urls->push_back(new TemplateURL(*i));
 
   const int resource_keyword_version =
       TemplateURLPrepopulateData::GetDataVersion(prefs);
@@ -222,8 +229,10 @@
   if (!keyword_result.did_default_search_provider_change)
     return false;
 
-  backup_default_search_provider->reset(
-      keyword_result.default_search_provider_backup);
+  if (keyword_result.backup_valid) {
+    backup_default_search_provider->reset(new TemplateURL(
+        keyword_result.default_search_provider_backup));
+  }
   return true;
 }