When the Google base URL changes and causes keyword updates, save those to the database.

chrome/browser/search_engines/template_url_service.cc

 // Copyright (c) 2012 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_service.h"
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/i18n/case_conversion.h"
@@ skipping 126 matching lines @@
 void PruneSyncChanges(const SyncDataMap* sync_data,
                       SyncChangeList* change_list) {
   for (size_t i = 0; i < change_list->size(); ) {
     if (ShouldRemoveSyncChange(i, change_list, sync_data))
       change_list->erase(change_list->begin() + i);
     else
       ++i;
   }
 }
 
+// Helper class that reports a specific string as the Google base URL.  We use
+// this so that code can calculate a TemplateURL's previous search URL after a
+// change to the global base URL.
+class OldBaseURLSearchTermsData : public UIThreadSearchTermsData {
+ public:
+  OldBaseURLSearchTermsData(Profile* profile, const std::string& old_base_url);
+  virtual ~OldBaseURLSearchTermsData();
+
+  virtual std::string GoogleBaseURLValue() const OVERRIDE;
+
+ private:
+  std::string old_base_url;
+};
+
+OldBaseURLSearchTermsData::OldBaseURLSearchTermsData(
+    Profile* profile,
+    const std::string& old_base_url)
+    : UIThreadSearchTermsData(profile),
+      old_base_url(old_base_url) {
+}
+
+OldBaseURLSearchTermsData::~OldBaseURLSearchTermsData() {
+}
+
+std::string OldBaseURLSearchTermsData::GoogleBaseURLValue() const {
+  return old_base_url;
+}
+
 }  // namespace
 
 
 class TemplateURLService::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
@@ skipping 358 matching lines @@
   data.short_name = title;
   data.SetKeyword(keyword);
   if (search_url != data.url()) {
     data.SetURL(search_url);
     // The urls have changed, reset the favicon url.
     data.favicon_url = GURL();
   }
   data.safe_for_autoreplace = false;
   data.last_modified = time_provider_();
   TemplateURL new_url(url->profile(), data);
-  if (UpdateNoNotify(url, new_url))
+  UIThreadSearchTermsData search_terms_data(url->profile());
+  if (UpdateNoNotify(url, new_url, search_terms_data))
     NotifyObservers();
 }
 
 bool TemplateURLService::CanMakeDefault(const TemplateURL* url) {
   return url != GetDefaultSearchProvider() &&
       url->url_ref().SupportsReplacement() && !is_default_search_managed();
 }
 
 void TemplateURLService::SetDefaultSearchProvider(TemplateURL* url) {
   DCHECK(!is_default_search_managed_);
@@ skipping 252 matching lines @@
       visits_to_add_.push_back(*visit_details.ptr());
     else
       UpdateKeywordSearchTermsForURL(*visit_details.ptr());
   } else if (type == chrome::NOTIFICATION_DEFAULT_SEARCH_POLICY_CHANGED) {
     // Policy has been updated, so the default search prefs may be different.
     // Reload the default search provider from them.
     // TODO(pkasting): Rather than communicating via prefs, we should eventually
     // observe policy changes directly.
     UpdateDefaultSearch();
   } else if (type == chrome::NOTIFICATION_GOOGLE_URL_UPDATED) {
-    if (loaded_)
-      GoogleBaseURLChanged();
+    if (loaded_) {
+      GoogleBaseURLChanged(
+          content::Details<GoogleURLTracker::UpdatedDetails>(details)->first);
+    }
   } else if (type == chrome::NOTIFICATION_PREF_CHANGED) {
     // Listen for changes to the default search from Sync.
     DCHECK_EQ(std::string(prefs::kSyncedDefaultSearchProviderGUID),
               *content::Details<std::string>(details).ptr());
     PrefService* prefs = GetPrefs();
     TemplateURL* new_default_search = GetTemplateURLForGUID(
         prefs->GetString(prefs::kSyncedDefaultSearchProviderGUID));
     if (new_default_search && !is_default_search_managed_) {
       if (new_default_search != GetDefaultSearchProvider()) {
         SetDefaultSearchProvider(new_default_search);
@@ skipping 113 matching lines @@
             FROM_HERE,
             "ProcessSyncChanges failed on ChangeType ACTION_UPDATE");
         continue;
       }
       // Possibly resolve a keyword conflict if they have the same keywords but
       // are not the same entry.
       if (existing_keyword_turl && existing_keyword_turl != existing_turl) {
         ResolveSyncKeywordConflict(turl.get(), existing_keyword_turl,
                                    &new_changes);
       }
-      if (UpdateNoNotify(existing_turl, *turl))
+      UIThreadSearchTermsData search_terms_data(existing_turl->profile());
+      if (UpdateNoNotify(existing_turl, *turl, search_terms_data))
         NotifyObservers();
     } else {
       // We've unexpectedly received an ACTION_INVALID.
       NOTREACHED() << "Unexpected sync change state.";
       error = sync_error_factory_->CreateAndUploadError(
           FROM_HERE,
           "ProcessSyncChanges received an ACTION_INVALID");
     }
   }
 
@@ skipping 65 matching lines @@
     if (local_turl) {
       // This local search engine is already synced. If the timestamp differs
       // from Sync, we need to update locally or to the cloud. Note that if the
       // timestamps are equal, we touch neither.
       if (sync_turl->last_modified() > local_turl->last_modified()) {
         // We've received an update from Sync. We should replace all synced
         // fields in the local TemplateURL. Note that this includes the
         // TemplateURLID and the TemplateURL may have to be reparsed. This
         // also makes the local data's last_modified timestamp equal to Sync's,
         // avoiding an Update on the next MergeData call.
-        if (UpdateNoNotify(local_turl, *sync_turl))
+        UIThreadSearchTermsData search_terms_data(local_turl->profile());
+        if (UpdateNoNotify(local_turl, *sync_turl, search_terms_data))
           NotifyObservers();
       } else if (sync_turl->last_modified() < local_turl->last_modified()) {
         // Otherwise, we know we have newer data, so update Sync with our
         // data fields.
         new_changes.push_back(SyncChange(SyncChange::ACTION_UPDATE,
             local_data_map[local_turl->sync_guid()]));
       }
       local_data_map.erase(iter->first);
     } else {
       // The search engine from the cloud has not been synced locally, but there
@@ skipping 126 matching lines @@
   sync_pb::SearchEngineSpecifics specifics =
       sync_data.GetSpecifics().search_engine();
 
   // Past bugs might have caused either of these fields to be empty.  Just
   // delete this data off the server.
   if (specifics.url().empty() || specifics.sync_guid().empty()) {
     change_list->push_back(SyncChange(SyncChange::ACTION_DELETE, sync_data));
     return NULL;
   }
 
-  TemplateURLData data;
+  TemplateURLData data(existing_turl ?
+      existing_turl->data() : TemplateURLData());
   data.short_name = UTF8ToUTF16(specifics.short_name());
   data.originating_url = GURL(specifics.originating_url());
   string16 keyword(UTF8ToUTF16(specifics.keyword()));
   // NOTE: Once this code has shipped in a couple of stable releases, we can
   // probably remove the migration portion, comment out the
   // "autogenerate_keyword" field entirely in the .proto file, and fold the
   // empty keyword case into the "delete data" block above.
   bool reset_keyword =
       specifics.autogenerate_keyword() || specifics.keyword().empty();
   if (reset_keyword)
     keyword = ASCIIToUTF16("dummy");  // Will be replaced below.
   DCHECK(!keyword.empty());
   data.SetKeyword(keyword);
   data.SetURL(specifics.url());
   data.suggestions_url = specifics.suggestions_url();
   data.instant_url = specifics.instant_url();
   data.favicon_url = GURL(specifics.favicon_url());
   data.show_in_default_list = specifics.show_in_default_list();
   data.safe_for_autoreplace = specifics.safe_for_autoreplace();
   base::SplitString(specifics.input_encodings(), ';', &data.input_encodings);
   data.date_created = base::Time::FromInternalValue(specifics.date_created());
   data.last_modified = base::Time::FromInternalValue(specifics.last_modified());
   data.prepopulate_id = specifics.prepopulate_id();
   data.sync_guid = specifics.sync_guid();
-  if (existing_turl) {
-    data.id = existing_turl->id();
-    data.created_by_policy = existing_turl->created_by_policy();
-    data.usage_count = existing_turl->usage_count();
-  }
 
   TemplateURL* turl = new TemplateURL(profile, data);
   DCHECK(!turl->IsExtensionKeyword());
   if (reset_keyword) {
     turl->ResetKeywordIfNecessary(true);
     SyncData sync_data = CreateSyncDataFromTemplateURL(*turl);
     change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
+  } else if (turl->IsGoogleSearchURLWithReplaceableKeyword()) {
+    if (!existing_turl) {
+      // We're adding a new TemplateURL that uses the Google base URL, so set
+      // its keyword appropriately for the local environment.
+      turl->ResetKeywordIfNecessary(false);
+    } else if (existing_turl->IsGoogleSearchURLWithReplaceableKeyword()) {
+      // Ignore keyword changes triggered by the Google base URL changing on
+      // another client.  If the base URL changes in this client as well, we'll
+      // pick that up separately at the appropriate time.  Otherwise, changing
+      // the keyword here could result in having the wrong keyword for the local
+      // environment.
+      turl->data_.SetKeyword(existing_turl->keyword());
+    }
   }
+
   return turl;
 }
 
 // static
 SyncDataMap TemplateURLService::CreateGUIDToSyncDataMap(
     const SyncDataList& sync_data) {
   SyncDataMap data_map;
   for (SyncDataList::const_iterator i(sync_data.begin()); i != sync_data.end();
        ++i)
     data_map[i->GetSpecifics().search_engine().sync_guid()] = *i;
@@ skipping 101 matching lines @@
 
   // If the keyword we're removing is from an extension, we're now done, since
   // it won't be synced or stored in the provider map.
   // TODO(mpcomplete): If we allow editing extension keywords, then those should
   // be synced.
   if (template_url->IsExtensionKeyword())
     return;
 
   if (!template_url->sync_guid().empty())
     guid_to_template_map_.erase(template_url->sync_guid());
-  if (loaded_)
-    provider_map_->Remove(template_url);
+  if (loaded_) {
+    UIThreadSearchTermsData search_terms_data(template_url->profile());
+    provider_map_->Remove(template_url, search_terms_data);
+  }
 }
 
 void TemplateURLService::RemoveFromKeywordMapByPointer(
     TemplateURL* template_url) {
   DCHECK(template_url);
   for (KeywordToTemplateMap::iterator i = keyword_to_template_map_.begin();
        i != keyword_to_template_map_.end(); ++i) {
     if (i->second == template_url) {
       keyword_to_template_map_.erase(i);
       // A given TemplateURL only occurs once in the map. As soon as we find the
@@ skipping 230 matching lines @@
   // The extension keyword in the model may be hiding a replaceable
   // non-extension keyword.  Look for it.
   for (TemplateURLVector::const_iterator i(template_urls_.begin());
        i != template_urls_.end(); ++i) {
     if (!(*i)->IsExtensionKeyword() && ((*i)->keyword() == keyword))
       return *i;
   }
   return NULL;
 }
 
-bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
-                                        const TemplateURL& new_values) {
+bool TemplateURLService::UpdateNoNotify(
+    TemplateURL* existing_turl,
+    const TemplateURL& new_values,
+    const SearchTermsData& old_search_terms_data) {
   DCHECK(loaded_);
   DCHECK(existing_turl);
   if (std::find(template_urls_.begin(), template_urls_.end(), existing_turl) ==
       template_urls_.end())
     return false;
 
   // TODO(mpcomplete): If we allow editing extension keywords, then those should
   // be persisted to disk and synced.  In this case this DCHECK should be
   // removed.
   DCHECK(!existing_turl->IsExtensionKeyword());
 
   string16 old_keyword(existing_turl->keyword());
   keyword_to_template_map_.erase(old_keyword);
   if (!existing_turl->sync_guid().empty())
     guid_to_template_map_.erase(existing_turl->sync_guid());
 
-  provider_map_->Remove(existing_turl);
+  provider_map_->Remove(existing_turl, old_search_terms_data);
   TemplateURLID previous_id = existing_turl->id();
   existing_turl->CopyFrom(new_values);
   existing_turl->data_.id = previous_id;
-  UIThreadSearchTermsData search_terms_data(profile_);
-  provider_map_->Add(existing_turl, search_terms_data);
+  UIThreadSearchTermsData new_search_terms_data(profile_);
+  provider_map_->Add(existing_turl, new_search_terms_data);
 
   const string16& keyword = existing_turl->keyword();
   KeywordToTemplateMap::const_iterator i =
       keyword_to_template_map_.find(keyword);
   if (i == keyword_to_template_map_.end()) {
     keyword_to_template_map_[keyword] = existing_turl;
   } else {
     // We can theoretically reach here in two cases:
     //   * There is an existing extension keyword and sync brings in a rename of
     //     a non-extension keyword to match.  In this case we just need to pick
@@ skipping 146 matching lines @@
         }
       } else {
         valid_term_count++;
         (*query_terms)[key_string] = value_string;
       }
     }
   }
   return (valid_term_count > 0);
 }
 
-void TemplateURLService::GoogleBaseURLChanged() {
+void TemplateURLService::GoogleBaseURLChanged(const GURL& old_base_url) {
   bool something_changed = false;
   for (TemplateURLVector::iterator i(template_urls_.begin());
        i != template_urls_.end(); ++i) {
     TemplateURL* t_url = *i;
     if (t_url->url_ref().HasGoogleBaseURLs() ||
         t_url->suggestions_url_ref().HasGoogleBaseURLs()) {
-      something_changed = true;
-      string16 original_keyword(t_url->keyword());
-      t_url->InvalidateCachedValues();
-      const string16& new_keyword(t_url->keyword());
+      TemplateURL updated_turl(t_url->profile(), t_url->data());
+      updated_turl.ResetKeywordIfNecessary(false);
       KeywordToTemplateMap::const_iterator i =

[SteveT, 2012/06/05 15:11:42]

nit: I always found it confusing and bad that this original code used the name
|i| twice in this block. Mind changing one  or both to better names?

[Peter Kasting, 2012/06/05 20:49:03]

Done.

-          keyword_to_template_map_.find(new_keyword);
+          keyword_to_template_map_.find(updated_turl.keyword());
       if ((i != keyword_to_template_map_.end()) && (i->second != t_url)) {
         // The new autogenerated keyword conflicts with another TemplateURL.
-        // Overwrite it if it's replaceable; otherwise just reset |t_url|'s
-        // keyword.  (This will not prevent |t_url| from auto-updating the
-        // keyword in the future if the conflicting TemplateURL disappears.)
-        if (!CanReplace(i->second)) {
-          t_url->data_.SetKeyword(original_keyword);
-          continue;
-        }
-        RemoveNoNotify(i->second);
+        // Overwrite it if it's replaceable; otherwise, leave |t_url| using its
+        // current keyword.  (This will not prevent |t_url| from auto-updating
+        // the keyword in the future if the conflicting TemplateURL disappears.)
+        // Note that we must still update |t_url| in this case, or the
+        // |provider_map_| will not be updated correctly.
+        if (CanReplace(i->second))
+          RemoveNoNotify(i->second);
+        else
+          updated_turl.data_.SetKeyword(t_url->keyword());
       }
-      RemoveFromKeywordMapByPointer(t_url);
-      keyword_to_template_map_[new_keyword] = t_url;
+      something_changed = true;
+      // This will send the keyword change to sync.  Note that other clients
+      // need to reset the keyword to an appropriate local value when this
+      // change arrives; see CreateTemplateURLFromTemplateURLAndSyncData().
+      UpdateNoNotify(t_url, updated_turl,
+          OldBaseURLSearchTermsData(t_url->profile(), old_base_url.spec()));
     }
   }
-
-  if (something_changed && loaded_) {
-    UIThreadSearchTermsData search_terms_data(profile_);
-    provider_map_->UpdateGoogleBaseURLs(search_terms_data);
+  if (something_changed)
     NotifyObservers();
-  }
 }
 
 void TemplateURLService::UpdateDefaultSearch() {
   if (!loaded_) {
     // Set |initial_default_search_provider_| from the preferences.  We use this
     // value for default search provider until the database has been loaded.
     if (!LoadDefaultSearchProviderFromPrefs(&initial_default_search_provider_,
                                             &is_default_search_managed_)) {
       // Prefs does not specify, so rely on the prepopulated engines.  This
       // should happen only the first time Chrome is started.
@@ skipping 29 matching lines @@
       // default_search_provider_ can't be NULL otherwise
       // TemplateURLsHaveSamePrefs would have returned true.  Remove this now
       // invalid value.
       TemplateURL* old_default = default_search_provider_;
       bool success = SetDefaultSearchProviderNoNotify(NULL);
       DCHECK(success);
       RemoveNoNotify(old_default);
     } else if (default_search_provider_) {
       TemplateURLData data(new_default_from_prefs->data());
       data.created_by_policy = true;
-      TemplateURL new_values(profile_, data);
-      UpdateNoNotify(default_search_provider_, new_values);
+      TemplateURL new_values(new_default_from_prefs->profile(), data);
+      UIThreadSearchTermsData search_terms_data(
+          default_search_provider_->profile());
+      UpdateNoNotify(default_search_provider_, new_values, search_terms_data);
     } else {
       TemplateURL* new_template = NULL;
       if (new_default_from_prefs.get()) {
         TemplateURLData data(new_default_from_prefs->data());
         data.created_by_policy = true;
         new_template = new TemplateURL(profile_, data);
         if (!AddNoNotify(new_template, true))
           return;
       }
       bool success = SetDefaultSearchProviderNoNotify(new_template);
@@ skipping 237 matching lines @@
 }
 
 void TemplateURLService::ResetTemplateURLGUID(TemplateURL* url,
                                               const std::string& guid) {
   DCHECK(loaded_);
   DCHECK(!guid.empty());
 
   TemplateURLData data(url->data());
   data.sync_guid = guid;
   TemplateURL new_url(url->profile(), data);
-  UpdateNoNotify(url, new_url);
+  UIThreadSearchTermsData search_terms_data(url->profile());
+  UpdateNoNotify(url, new_url, search_terms_data);
 }
 
 string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl) {
   // Already unique.
   if (!GetTemplateURLForKeyword(turl.keyword()))
     return turl.keyword();
 
   // First, try to return the generated keyword for the TemplateURL.
   GURL gurl(turl.url());
   if (gurl.is_valid()) {
@@ skipping 32 matching lines @@
     sync_turl->data_.SetKeyword(new_keyword);
     // If we update the cloud TURL, we need to push an update back to sync
     // informing it that something has changed.
     SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl);
     change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
   } else {
     string16 new_keyword = UniquifyKeyword(*local_turl);
     TemplateURLData data(local_turl->data());
     data.SetKeyword(new_keyword);
     TemplateURL new_turl(local_turl->profile(), data);
-    if (UpdateNoNotify(local_turl, new_turl))
+    UIThreadSearchTermsData search_terms_data(local_turl->profile());
+    if (UpdateNoNotify(local_turl, new_turl, search_terms_data))
       NotifyObservers();
     if (!models_associated_) {
       // We're doing our initial sync, so UpdateNoNotify() won't have generated
       // an ACTION_UPDATE.  If this local URL is one that was just newly brought
       // down from the sync server, we need to go ahead and generate an update
       // for it.  If it was pre-existing, then this is unnecessary (and in fact
       // wrong) because MergeDataAndStartSyncing() will later add an ACTION_ADD
       // for this URL; but in this case, PruneSyncChanges() will prune out the
       // ACTION_UPDATE we create here.
       SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
@@ skipping 97 matching lines @@
     // TODO(mpcomplete): If we allow editing extension keywords, then those
     // should be persisted to disk and synced.
     if (template_url->sync_guid().empty() &&
         !template_url->IsExtensionKeyword()) {
       template_url->data_.sync_guid = guid::GenerateGUID();
       if (service_.get())
         service_->UpdateKeyword(template_url->data());
     }
   }
 }