More misc. cleanups to minimize future refactoring diffs.

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 236 matching lines @@
     // We already have a TemplateURL for this keyword. Only allow it to be
     // replaced if the TemplateURL can be replaced.
     return CanReplace(existing_url);
   }
 
   // We don't have a TemplateURL with keyword. Only allow a new one if there
   // isn't a TemplateURL for the specified host, or there is one but it can
   // be replaced. We do this to ensure that if the user assigns a different
   // keyword to a generated TemplateURL, we won't regenerate another keyword for
   // the same host.
-  if (url.is_valid() && !url.host().empty())
-    return CanReplaceKeywordForHost(url.host(), template_url_to_replace);
-  return true;
+  return !url.is_valid() || url.host().empty() ||
+      CanReplaceKeywordForHost(url.host(), template_url_to_replace);
 }
 
 void TemplateURLService::FindMatchingKeywords(
     const string16& prefix,
     bool support_replacement_only,
     std::vector<string16>* matches) const {
   // Sanity check args.
   if (prefix.empty())
     return;
   DCHECK(matches != NULL);
@@ skipping 18 matching lines @@
     DCHECK(!i->second->url().empty());
     if (!support_replacement_only || i->second->url_ref().SupportsReplacement())
       matches->push_back(i->first);
   }
 }
 
 const TemplateURL* TemplateURLService::GetTemplateURLForKeyword(
     const string16& keyword) const {
   KeywordToTemplateMap::const_iterator elem(
       keyword_to_template_map_.find(keyword));
-  return (elem == keyword_to_template_map_.end()) ? NULL : elem->second;
+  if (elem != keyword_to_template_map_.end())
+    return elem->second;
+  return (initial_default_search_provider_.get() &&
+      (initial_default_search_provider_->keyword() == keyword)) ?
+      initial_default_search_provider_.get() : NULL;
 }
 
 const TemplateURL* TemplateURLService::GetTemplateURLForGUID(
     const std::string& sync_guid) const {
   GUIDToTemplateMap::const_iterator elem(
       guid_to_template_map_.find(sync_guid));
-  return (elem == guid_to_template_map_.end()) ? NULL : elem->second;
+  if (elem != guid_to_template_map_.end())
+    return elem->second;
+  return (initial_default_search_provider_.get() &&
+      (initial_default_search_provider_->sync_guid() == sync_guid)) ?
+      initial_default_search_provider_.get() : NULL;
 }
 
 const TemplateURL* TemplateURLService::GetTemplateURLForHost(
     const std::string& host) const {
-  return provider_map_.GetTemplateURLForHost(host);
+  const TemplateURL* t_url = provider_map_.GetTemplateURLForHost(host);
+  if (t_url)
+    return t_url;
+  return (initial_default_search_provider_.get() &&
+      (GenerateSearchURL(initial_default_search_provider_.get()).host() ==
+          host)) ? initial_default_search_provider_.get() : NULL;
 }
 
 void TemplateURLService::Add(TemplateURL* template_url) {
-  AddNoNotify(template_url);
+  AddNoNotify(template_url, true);
   NotifyObservers();
 }
 
 void TemplateURLService::AddWithOverrides(const TemplateURL* template_url,
                                           const string16& short_name,
                                           const string16& keyword,
                                           const std::string& url) {
   TemplateURL* modifiable_url = const_cast<TemplateURL*>(template_url);
   modifiable_url->data_.short_name = short_name;
   modifiable_url->data_.SetKeyword(keyword);
@@ skipping 43 matching lines @@
   // TODO(mpcomplete): disable the keyword when the extension is disabled.
   if (extension->omnibox_keyword().empty())
     return;
 
   Load();
   if (!loaded_) {
     pending_extension_ids_.push_back(extension->id());
     return;
   }
 
-  const TemplateURL* existing_url = GetTemplateURLForExtension(extension);
-  string16 keyword = UTF8ToUTF16(extension->omnibox_keyword());
-
-  TemplateURLData data;
-  data.short_name = UTF8ToUTF16(extension->name());
-  data.SetKeyword(UTF8ToUTF16(extension->omnibox_keyword()));
-  // This URL is not actually used for navigation. It holds the extension's
-  // ID, as well as forcing the TemplateURL to be treated as a search keyword.
-  data.SetURL(std::string(chrome::kExtensionScheme) + "://" + extension->id() +
-      "/?q={searchTerms}");
-  scoped_ptr<TemplateURL> template_url(new TemplateURL(data));
-
-  if (existing_url) {
-    // TODO(mpcomplete): only replace if the user hasn't changed the keyword.
-    // (We don't have UI for that yet).
-    UpdateNoNotify(existing_url, *template_url);
-  } else {
-    AddNoNotify(template_url.release());
+  if (!GetTemplateURLForExtension(extension)) {
+    TemplateURLData data;
+    data.short_name = UTF8ToUTF16(extension->name());
+    data.SetKeyword(UTF8ToUTF16(extension->omnibox_keyword()));
+    // This URL is not actually used for navigation. It holds the extension's
+    // ID, as well as forcing the TemplateURL to be treated as a search keyword.
+    data.SetURL(std::string(chrome::kExtensionScheme) + "://" +
+        extension->id() + "/?q={searchTerms}");
+    Add(new TemplateURL(data));
   }
-  NotifyObservers();
 }
 
 void TemplateURLService::UnregisterExtensionKeyword(
     const Extension* extension) {
-  const TemplateURL* url = GetTemplateURLForExtension(extension);
-  if (url)
-    Remove(url);
+  if (loaded_) {
+    const TemplateURL* url = GetTemplateURLForExtension(extension);
+    if (url)
+      Remove(url);
+  } else {
+    PendingExtensionIDs::iterator i = std::find(pending_extension_ids_.begin(),
+        pending_extension_ids_.end(), extension->id());
+    if (i != pending_extension_ids_.end())
+      pending_extension_ids_.erase(i);
+  }
 }
 
 const TemplateURL* TemplateURLService::GetTemplateURLForExtension(
     const Extension* extension) const {
   for (TemplateURLVector::const_iterator i = template_urls_.begin();
        i != template_urls_.end(); ++i) {
     if ((*i)->IsExtensionKeyword() &&
         ((*i)->url_ref().GetHost() == extension->id()))
       return *i;
   }
 
   return NULL;
 }
 
-std::vector<const TemplateURL*> TemplateURLService::GetTemplateURLs() const {
+TemplateURLService::TemplateURLVector
+    TemplateURLService::GetTemplateURLs() const {
   return template_urls_;
 }
 
 void TemplateURLService::IncrementUsageCount(const TemplateURL* url) {
   DCHECK(url && std::find(template_urls_.begin(), template_urls_.end(), url) !=
       template_urls_.end());
   ++const_cast<TemplateURL*>(url)->data_.usage_count;
-  if (service_.get())
-    service_->UpdateKeyword(*url);
+  // Extension keywords are not persisted.
+  // TODO(mpcomplete): If we allow editing extension keywords, then those should
+  // be persisted to disk and synced.
+  if (service_.get() && !url->IsExtensionKeyword())
+    service_.get()->UpdateKeyword(*url);
 }
 
 void TemplateURLService::ResetTemplateURL(const TemplateURL* url,
                                           const string16& title,
                                           const string16& keyword,
                                           const std::string& search_url) {
   TemplateURLData data(url->data());
   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(data);
   UpdateNoNotify(url, new_url);
   NotifyObservers();
 }
 
 bool TemplateURLService::CanMakeDefault(const TemplateURL* url) {
   return url != GetDefaultSearchProvider() &&
       url->url_ref().SupportsReplacement() && !is_default_search_managed();
 }
 
 void TemplateURLService::SetDefaultSearchProvider(const TemplateURL* url) {
-  if (is_default_search_managed_) {
-    NOTREACHED();
-    return;
-  }
+  DCHECK(!is_default_search_managed_);
+  // Extension keywords cannot be made default, as they are inherently async.
+  DCHECK(!url || !url->IsExtensionKeyword());
+
   // Always persist the setting in the database, that way if the backup
   // signature has changed out from under us it gets reset correctly.
   SetDefaultSearchProviderNoNotify(url);
   NotifyObservers();
 }
 
 const TemplateURL* TemplateURLService::GetDefaultSearchProvider() {
   if (loaded_ && !load_failed_)
     return default_search_provider_;
 
   // We're not loaded, rely on the default search provider stored in prefs.
   return initial_default_search_provider_.get();
 }
 
 const TemplateURL* TemplateURLService::FindNewDefaultSearchProvider() {
-  // See if the prepoluated default still exists.
+  // See if the prepopulated default still exists.
   scoped_ptr<TemplateURL> prepopulated_default(
       TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(GetPrefs()));
   for (TemplateURLVector::iterator i = template_urls_.begin();
        i != template_urls_.end(); ++i) {
     if ((*i)->prepopulate_id() == prepopulated_default->prepopulate_id())
       return *i;
   }
-  // If not, use the first of the templates.
-  return template_urls_.empty() ? NULL : template_urls_[0];
+  // If not, use the first non-extension keyword of the templates.
+  for (TemplateURLVector::const_iterator i(template_urls_.begin());
+       i != template_urls_.end(); ++i) {
+    if (!(*i)->IsExtensionKeyword())
+      return *i;
+  }
+  return NULL;
 }
 
 void TemplateURLService::AddObserver(TemplateURLServiceObserver* observer) {
   model_observers_.AddObserver(observer);
 }
 
 void TemplateURLService::RemoveObserver(TemplateURLServiceObserver* observer) {
   model_observers_.RemoveObserver(observer);
 }
 
@@ skipping 29 matching lines @@
   }
 
   // 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;
 
   std::vector<TemplateURL*> template_urls;
   const TemplateURL* default_search_provider = NULL;
   int new_resource_keyword_version = 0;
-  GetSearchProvidersUsingKeywordResult(*result,
-                                       service_.get(),
-                                       GetPrefs(),
-                                       &template_urls,
-                                       &default_search_provider,
-                                       &new_resource_keyword_version);
+  GetSearchProvidersUsingKeywordResult(*result, service_.get(), GetPrefs(),
+      &template_urls, &default_search_provider, &new_resource_keyword_version);
 
   bool database_specified_a_default = (default_search_provider != NULL);
 
   // Check if default search provider is now managed.
   scoped_ptr<TemplateURL> default_from_prefs;
   LoadDefaultSearchProviderFromPrefs(&default_from_prefs,
                                      &is_default_search_managed_);
 
   // Check if the default search provider has been changed in Web Data by
   // another program. No immediate action is performed because the default
@@ skipping 27 matching lines @@
       // The value from the preferences was previously stored in the database.
       // Reuse it.
     } else {
       // The value from the preferences takes over.
       default_search_provider = NULL;
       if (default_from_prefs.get()) {
         TemplateURLData data(default_from_prefs->data());
         data.created_by_policy = true;
         data.id = kInvalidTemplateURLID;
         TemplateURL* managed_default = new TemplateURL(data);
-        AddNoNotify(managed_default);
+        AddNoNotify(managed_default, true);
         default_search_provider = managed_default;
       }
     }
     // Note that this saves the default search provider to prefs.
-    SetDefaultSearchProviderNoNotify(default_search_provider);
+    if (!default_search_provider ||
+        !default_search_provider->IsExtensionKeyword())
+      SetDefaultSearchProviderNoNotify(default_search_provider);
   } else {
     // If we had a managed default, replace it with the synced default if
     // applicable, or the first provider of the list.
     const TemplateURL* synced_default = GetPendingSyncedDefaultSearchProvider();
     if (synced_default) {
       default_search_provider = synced_default;
       pending_synced_default_search_ = false;
     } else if (database_specified_a_default &&
-               default_search_provider == NULL &&
-               !template_urls.empty()) {
-      default_search_provider = template_urls[0];
+               default_search_provider == NULL) {
+      for (std::vector<TemplateURL*>::const_iterator i = template_urls.begin();
+           i != template_urls.end(); ++i) {
+        if (!(*i)->IsExtensionKeyword()) {
+          default_search_provider = *i;
+          break;
+        }
+      }
     }
 
     // If the default search provider existed previously, then just
     // set the member variable. Otherwise, we'll set it using the method
     // to ensure that it is saved properly after its id is set.
     if (default_search_provider &&
         (default_search_provider->id() != kInvalidTemplateURLID)) {
       default_search_provider_ = default_search_provider;
       default_search_provider = NULL;
     }
@@ skipping 119 matching lines @@
 }
 
 SyncDataList TemplateURLService::GetAllSyncData(
     syncable::ModelType type) const {
   DCHECK_EQ(syncable::SEARCH_ENGINES, type);
 
   SyncDataList current_data;
   for (TemplateURLVector::const_iterator iter = template_urls_.begin();
       iter != template_urls_.end(); ++iter) {
     // We don't sync extension keywords.
+    // TODO(mpcomplete): If we allow editing extension keywords, then those
+    // should be persisted to disk and synced.
     if ((*iter)->IsExtensionKeyword())
       continue;
     // We don't sync keywords managed by policy.
     if ((*iter)->created_by_policy())
       continue;
     current_data.push_back(CreateSyncDataFromTemplateURL(**iter));
   }
 
   return current_data;
 }
@@ skipping 144 matching lines @@
             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
       // might be a local search engine that is a duplicate that needs to be
       // merged.
       const TemplateURL* dupe_turl = FindDuplicateOfSyncTemplateURL(*sync_turl);
       if (dupe_turl) {
         // Merge duplicates and remove the processed local TURL from the map.
-        TemplateURL* modifiable_dupe_turl =
-            const_cast<TemplateURL*>(dupe_turl);
         std::string old_guid = dupe_turl->sync_guid();
         MergeSyncAndLocalURLDuplicates(sync_turl.release(),
-                                       modifiable_dupe_turl,
+                                       const_cast<TemplateURL*>(dupe_turl),
                                        &new_changes);
         local_data_map.erase(old_guid);
       } else {
         std::string guid = sync_turl->sync_guid();
         // Keyword conflict is possible in this case. Resolve it first before
         // adding the new TemplateURL. Note that we don't remove the local TURL
         // from local_data_map in this case as it may still need to be pushed to
         // the cloud.
         ResolveSyncKeywordConflict(sync_turl.get(), &new_changes);
         // Force the local ID to kInvalidTemplateURLID so we can add it.
@@ skipping 34 matching lines @@
   DCHECK_NE(type, SyncChange::ACTION_INVALID);
   DCHECK(turl);
 
   if (!models_associated_)
     return;  // Not syncing.
 
   if (processing_syncer_changes_)
     return;  // These are changes originating from us. Ignore.
 
   // Avoid syncing Extension keywords.
+  // TODO(mpcomplete): If we allow editing extension keywords, then those should
+  // be persisted to disk and synced.
   if (turl->IsExtensionKeyword())
     return;
 
   // Avoid syncing keywords managed by policy.
   if (turl->created_by_policy())
     return;
 
   SyncChangeList changes;
 
   SyncData sync_data = CreateSyncDataFromTemplateURL(*turl);
@@ skipping 49 matching lines @@
   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();
   }
-  return new TemplateURL(data);
+
+  TemplateURL* turl = new TemplateURL(data);
+  DCHECK(!turl->IsExtensionKeyword());
+  return turl;
 }
 
 // static
 SyncDataMap TemplateURLService::CreateGUIDToSyncDataMap(
     const SyncDataList& sync_data) {
   SyncDataMap data_map;
-  SyncDataList::const_iterator iter;
-  for (iter = sync_data.begin(); iter != sync_data.end(); ++iter) {
-    data_map[iter->GetSpecifics().search_engine().sync_guid()] = *iter;
-  }
+  for (SyncDataList::const_iterator i(sync_data.begin()); i != sync_data.end();
+       ++i)
+    data_map[i->GetSpecifics().search_engine().sync_guid()] = *i;
   return data_map;
 }
 
 void TemplateURLService::SetKeywordSearchTermsForURL(const TemplateURL* t_url,
                                                      const GURL& url,
                                                      const string16& term) {
   HistoryService* history = profile_  ?
       profile_->GetHistoryService(Profile::EXPLICIT_ACCESS) : NULL;
   if (!history)
     return;
@@ skipping 34 matching lines @@
       std::string osd_url(initializers[i].url);
       osd_url.replace(template_position, arraysize(kTemplateParameter) - 1,
                       kSearchTermParameter);
 
       // TemplateURLService ends up owning the TemplateURL, don't try and free
       // it.
       TemplateURLData data;
       data.short_name = UTF8ToUTF16(initializers[i].content);
       data.SetKeyword(UTF8ToUTF16(initializers[i].keyword));
       data.SetURL(osd_url);
-      AddNoNotify(new TemplateURL(data));
+      AddNoNotify(new TemplateURL(data), true);
     }
   }
 
   // Initialize default search.
   UpdateDefaultSearch();
 
   // Request a server check for the correct Google URL if Google is the
   // default search engine, not in headless mode and not in Chrome Frame.
   if (initial_default_search_provider_.get() &&
-    initial_default_search_provider_->url_ref().HasGoogleBaseURLs()) {
+      initial_default_search_provider_->url_ref().HasGoogleBaseURLs()) {
     scoped_ptr<base::Environment> env(base::Environment::Create());
     if (!env->HasVar(env_vars::kHeadless) &&
         !CommandLine::ForCurrentProcess()->HasSwitch(switches::kChromeFrame))
       GoogleURLTracker::RequestServerCheck();
   }
 }
 
 void TemplateURLService::RemoveFromMaps(const TemplateURL* template_url) {
   if (!template_url->keyword().empty())
     keyword_to_template_map_.erase(template_url->keyword());
+
+  // 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);
 }
 
 void TemplateURLService::RemoveFromKeywordMapByPointer(
     const 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
       // entry, stop.
       break;
     }
   }
 }
 
 void TemplateURLService::AddToMaps(const TemplateURL* template_url) {
   if (!template_url->keyword().empty())
     keyword_to_template_map_[template_url->keyword()] = template_url;
+
+  // Extension keywords are not synced, so they don't go into the GUID map,
+  // and do not use host-based search URLs, so they don't go into the provider
+  // map, so at this point we're done.
+  // TODO(mpcomplete): If we allow editing extension keywords, then those should
+  // be persisted to disk and synced.
+  if (template_url->IsExtensionKeyword())
+    return;
+
   if (!template_url->sync_guid().empty())
     guid_to_template_map_[template_url->sync_guid()] = template_url;
   if (loaded_) {
     UIThreadSearchTermsData search_terms_data;
     provider_map_.Add(template_url, search_terms_data);
   }
 }
 
 void TemplateURLService::SetTemplateURLs(
     const std::vector<TemplateURL*>& urls) {
   // Add mappings for the new items.
 
   // First, add the items that already have id's, so that the next_id_
   // gets properly set.
   for (std::vector<TemplateURL*>::const_iterator i = urls.begin();
-       i != urls.end();
-       ++i) {
+       i != urls.end(); ++i) {
     if ((*i)->id() != kInvalidTemplateURLID) {
       next_id_ = std::max(next_id_, (*i)->id());
-      AddToMaps(*i);
-      template_urls_.push_back(*i);
+      AddNoNotify(*i, false);

[Peter Kasting, 2012/04/10 01:24:33]

I changed this call because in the future I need to add some code to handle
keyword conflicts, and I wanted to add it to one function (AddNoNotify())
instead of insert calls to it both there and here.

     }
   }
 
   // Next add the new items that don't have id's.
   for (std::vector<TemplateURL*>::const_iterator i = urls.begin();
-       i != urls.end();
-       ++i) {
+       i != urls.end(); ++i) {
     if ((*i)->id() == kInvalidTemplateURLID)
-      AddNoNotify(*i);
+      AddNoNotify(*i, true);
   }
 }
 
 void TemplateURLService::ChangeToLoadedState() {
   DCHECK(!loaded_);
 
   UIThreadSearchTermsData search_terms_data;
   provider_map_.Init(template_urls_, search_terms_data);
   loaded_ = true;
 }
@@ skipping 24 matching lines @@
   std::string search_url;
   std::string suggest_url;
   std::string instant_url;
   std::string icon_url;
   std::string encodings;
   std::string short_name;
   std::string keyword;
   std::string id_string;
   std::string prepopulate_id;
   if (t_url) {
+    DCHECK(!t_url->IsExtensionKeyword());
     enabled = true;
     search_url = t_url->url();
     suggest_url = t_url->suggestions_url();
     instant_url = t_url->instant_url();
     GURL icon_gurl = t_url->favicon_url();
     if (!icon_gurl.is_empty())
       icon_url = icon_gurl.spec();
     encodings = JoinString(t_url->input_encodings(), ';');
     short_name = UTF16ToUTF8(t_url->short_name());
     keyword = UTF16ToUTF8(t_url->keyword());
@@ skipping 63 matching lines @@
     int64 value;
     base::StringToInt64(id_string, &value);
     data.id = value;
   }
   if (!prepopulate_id.empty() && !*is_managed) {
     int value;
     base::StringToInt(prepopulate_id, &value);
     data.prepopulate_id = value;
   }
   default_provider->reset(new TemplateURL(data));
+  DCHECK(!(*default_provider)->IsExtensionKeyword());
   return true;
 }
 
 bool TemplateURLService::CanReplaceKeywordForHost(
     const std::string& host,
     const TemplateURL** to_replace) {
+  DCHECK(to_replace);
+  DCHECK(!*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)
-          *to_replace = url;
-        return true;
-      }
+  if (!urls)
+    return true;
+  for (TemplateURLSet::const_iterator i(urls->begin()); i != urls->end(); ++i) {
+    if (CanReplace(*i)) {
+      if (to_replace)
+        *to_replace = *i;
+      return true;
     }
   }
-
-  if (to_replace)
-    *to_replace = NULL;
-  return !urls;
+  return false;
 }
 
 bool TemplateURLService::CanReplace(const TemplateURL* t_url) {
   return (t_url != default_search_provider_ && !t_url->show_in_default_list() &&
           t_url->safe_for_autoreplace());
 }
 
 void TemplateURLService::UpdateNoNotify(const TemplateURL* existing_turl,
                                         const TemplateURL& new_values) {
   DCHECK(loaded_);
   DCHECK(existing_turl);
   DCHECK(std::find(template_urls_.begin(), template_urls_.end(),
                    existing_turl) != template_urls_.end());
 
-  if (!existing_turl->keyword().empty())
-    keyword_to_template_map_.erase(existing_turl->keyword());
+  // 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());
+  if (!old_keyword.empty())
+    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);
   TemplateURLID previous_id = existing_turl->id();
   TemplateURL* modifiable_turl = const_cast<TemplateURL*>(existing_turl);
   *modifiable_turl = new_values;
   modifiable_turl->data_.id = previous_id;
   UIThreadSearchTermsData search_terms_data;
   provider_map_.Add(existing_turl, search_terms_data);
 
-  if (!existing_turl->keyword().empty())
-    keyword_to_template_map_[existing_turl->keyword()] = existing_turl;
+  const string16& keyword = existing_turl->keyword();
+  if (!keyword.empty())
+    keyword_to_template_map_[keyword] = existing_turl;
   if (!existing_turl->sync_guid().empty())
     guid_to_template_map_[existing_turl->sync_guid()] = existing_turl;
 
   if (service_.get())
     service_->UpdateKeyword(*existing_turl);
 
   // Inform sync of the update.
   ProcessTemplateURLChange(existing_turl, SyncChange::ACTION_UPDATE);
 
   if (default_search_provider_ == existing_turl)
@@ skipping 45 matching lines @@
       built_terms = true;
 
       if (content::PageTransitionStripQualifier(details.transition) ==
           content::PAGE_TRANSITION_KEYWORD) {
         // The visit is the result of the user entering a keyword, generate a
         // KEYWORD_GENERATED visit for the KEYWORD so that the keyword typed
         // count is boosted.
         AddTabToSearchVisit(**i);
       }
 
-      QueryTerms::iterator terms_iterator =
-          query_terms.find(search_ref.GetSearchTermKey());
-      if (terms_iterator != query_terms.end() &&
-          !terms_iterator->second.empty()) {
+      QueryTerms::iterator j(query_terms.find(search_ref.GetSearchTermKey()));
+      if (j != query_terms.end() && !j->second.empty()) {
         SetKeywordSearchTermsForURL(*i, row.url(),
-            search_ref.SearchTermToString16(terms_iterator->second));
+                                    search_ref.SearchTermToString16(j->second));
       }
     }
   }
 }
 
 void TemplateURLService::AddTabToSearchVisit(const TemplateURL& t_url) {
   // Only add visits for entries the user hasn't modified. If the user modified
   // the entry the keyword may no longer correspond to the host name. It may be
   // possible to do something more sophisticated here, but it's so rare as to
   // not be worth it.
@@ skipping 47 matching lines @@
         valid_term_count++;
         (*query_terms)[key_string] = value_string;
       }
     }
   }
   return (valid_term_count > 0);
 }
 
 void TemplateURLService::GoogleBaseURLChanged() {
   bool something_changed = false;
-  for (size_t i = 0; i < template_urls_.size(); ++i) {
-    const TemplateURL* t_url = template_urls_[i];
+  for (TemplateURLVector::iterator i(template_urls_.begin());
+       i != template_urls_.end(); ++i) {
+    TemplateURL* t_url = const_cast<TemplateURL*>(*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());
       RemoveFromKeywordMapByPointer(t_url);
-      t_url->InvalidateCachedValues();
-      if (!t_url->keyword().empty())
-        keyword_to_template_map_[t_url->keyword()] = t_url;
-      something_changed = true;
+      if (!new_keyword.empty())
+        keyword_to_template_map_[new_keyword] = t_url;
     }
   }
 
   if (something_changed && loaded_) {
     UIThreadSearchTermsData search_terms_data;
     provider_map_.UpdateGoogleBaseURLs(search_terms_data);
     NotifyObservers();
   }
 }
 
@@ skipping 39 matching lines @@
       // invalid value.
       const TemplateURL* old_default = default_search_provider_;
       SetDefaultSearchProviderNoNotify(NULL);
       RemoveNoNotify(old_default);
     } else if (default_search_provider_) {
       TemplateURLData data(new_default_from_prefs->data());
       data.created_by_policy = true;
       TemplateURL new_values(data);
       UpdateNoNotify(default_search_provider_, new_values);
     } else {
-      // AddNoNotify will take ownership of new_template, so it's safe to
-      // release.
       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(data);
-        AddNoNotify(new_template);
+        AddNoNotify(new_template, true);
       }
       SetDefaultSearchProviderNoNotify(new_template);
     }
   } else if (!is_default_search_managed_ && new_is_default_managed) {
     // The default used to be unmanaged and is now managed.  Add the new
     // managed default to the list of URLs and set it as default.
     is_default_search_managed_ = new_is_default_managed;
-    // AddNoNotify will take ownership of new_template, so it's safe to
-    // release.
     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(data);
-      AddNoNotify(new_template);
+      AddNoNotify(new_template, true);
     }
     SetDefaultSearchProviderNoNotify(new_template);
   } else {
     // The default was managed and is no longer.
     DCHECK(is_default_search_managed_ && !new_is_default_managed);
     is_default_search_managed_ = new_is_default_managed;
     // If we had a default, delete the previous default if created by policy
     // and set a likely default.
     if ((default_search_provider_ != NULL) &&
         default_search_provider_->created_by_policy()) {
       const TemplateURL* old_default = default_search_provider_;
       default_search_provider_ = NULL;
       RemoveNoNotify(old_default);
     }
 
     // The likely default should be from Sync if we were waiting on Sync.
     // Otherwise, it should be FindNewDefaultSearchProvider.
     const TemplateURL* synced_default = GetPendingSyncedDefaultSearchProvider();
     if (synced_default)
       pending_synced_default_search_ = false;
     SetDefaultSearchProviderNoNotify(synced_default ? synced_default :
         FindNewDefaultSearchProvider());
   }
   NotifyObservers();
 }
 
 void TemplateURLService::SetDefaultSearchProviderNoNotify(
     const TemplateURL* url) {
-  DCHECK(!url || std::find(template_urls_.begin(), template_urls_.end(), url) !=
-      template_urls_.end());
+  if (url) {
+    DCHECK(std::find(template_urls_.begin(), template_urls_.end(), url) !=
+           template_urls_.end());
+    // Extension keywords cannot be made default, as they're inherently async.
+    DCHECK(!url->IsExtensionKeyword());
+  }
+
   default_search_provider_ = url;
 
   if (url) {
     TemplateURL* modifiable_url = const_cast<TemplateURL*>(url);
     // Don't mark the url as edited, otherwise we won't be able to rev the
     // template urls we ship with.
     modifiable_url->data_.show_in_default_list = true;
     if (service_.get())
       service_->UpdateKeyword(*url);
 
@@ skipping 22 matching lines @@
   }
 
   if (service_.get())
     service_->SetDefaultSearchProvider(url);
 
   // Inform sync the change to the show_in_default_list flag.
   if (url)
     ProcessTemplateURLChange(url, SyncChange::ACTION_UPDATE);
 }
 
-void TemplateURLService::AddNoNotify(TemplateURL* template_url) {
+void TemplateURLService::AddNoNotify(TemplateURL* template_url,
+                                     bool newly_adding) {
   DCHECK(template_url);
-  DCHECK_EQ(kInvalidTemplateURLID, template_url->id());
-  DCHECK(std::find(template_urls_.begin(), template_urls_.end(),
-                   template_url) == template_urls_.end());
-  template_url->data_.id = ++next_id_;
+
+  if (newly_adding) {
+    DCHECK_EQ(kInvalidTemplateURLID, template_url->id());
+    DCHECK(std::find(template_urls_.begin(), template_urls_.end(),
+                     template_url) == template_urls_.end());
+    template_url->data_.id = ++next_id_;
+  }
+
   template_urls_.push_back(template_url);
   AddToMaps(template_url);
 
-  if (service_.get())
-    service_->AddKeyword(*template_url);
+  if (newly_adding) {
+    // Don't persist extension keywords to disk.  They'll get re-added on each
+    // launch as the extensions are loaded.
+    // TODO(mpcomplete): If we allow editing extension keywords, then those
+    // should be persisted to disk and synced.
+    if (service_.get() && !template_url->IsExtensionKeyword())
+      service_->AddKeyword(*template_url);
 
-  // Inform sync of the addition. Note that this will assign a GUID to
-  // template_url and add it to the guid_to_template_map_.
-  ProcessTemplateURLChange(template_url, SyncChange::ACTION_ADD);
+    // Inform sync of the addition. Note that this will assign a GUID to
+    // template_url and add it to the guid_to_template_map_.
+    ProcessTemplateURLChange(template_url, SyncChange::ACTION_ADD);
+  }
 }
 
 void TemplateURLService::RemoveNoNotify(const TemplateURL* template_url) {
   TemplateURLVector::iterator i =
       std::find(template_urls_.begin(), template_urls_.end(), template_url);
   if (i == template_urls_.end())
     return;
 
   if (template_url == default_search_provider_) {
     // Should never delete the default search provider.
     NOTREACHED();
     return;
   }
 
   RemoveFromMaps(template_url);
 
   // Remove it from the vector containing all TemplateURLs.
   template_urls_.erase(i);
 
-  if (service_.get())
+  // Extension keywords are not persisted.
+  // TODO(mpcomplete): If we allow editing extension keywords, then those should
+  // be persisted to disk and synced.
+  if (service_.get() && !template_url->IsExtensionKeyword())
     service_->RemoveKeyword(template_url->id());
 
   // Inform sync of the deletion.
   ProcessTemplateURLChange(template_url, SyncChange::ACTION_DELETE);
 
   if (profile_) {
     content::Source<Profile> source(profile_);
     TemplateURLID id = template_url->id();
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_TEMPLATE_URL_REMOVED,
@@ skipping 21 matching lines @@
 // 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 TemplateURLService::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(); ) {
+       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())
+      // Extension keywords are not persisted.
+      // TODO(mpcomplete): If we allow editing extension keywords, then those
+      // should be persisted to disk and synced.
+      if (service_.get() && !template_url->IsExtensionKeyword())
         service_->RemoveKeyword(template_url->id());
       delete template_url;
     } else {
       ++i;
     }
   }
 }
 
 void TemplateURLService::ResetTemplateURLGUID(const TemplateURL* url,
                                               const std::string& guid) {
   DCHECK(!guid.empty());
 
   TemplateURLData data(url->data());
   data.sync_guid = guid;
   TemplateURL new_url(data);
   UpdateNoNotify(url, new_url);
 }
 
 string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl) const {
   // Already unique.
   if (!GetTemplateURLForKeyword(turl.keyword()))
     return turl.keyword();
 
   // First, try to return the generated keyword for the TemplateURL.
   GURL gurl(turl.url());
-  string16 keyword_candidate = GenerateKeyword(gurl, true);
-  if (!GetTemplateURLForKeyword(keyword_candidate) &&
-      !keyword_candidate.empty()) {
-    return keyword_candidate;
+  if (gurl.is_valid()) {
+    string16 keyword_candidate = GenerateKeyword(gurl, true);
+    if (!GetTemplateURLForKeyword(keyword_candidate) &&
+        !keyword_candidate.empty())
+      return keyword_candidate;
   }
 
   // We try to uniquify the keyword by appending a special character to the end.
   // This is a best-effort approach where we try to preserve the original
   // keyword and let the user do what they will after our attempt.
-  keyword_candidate = turl.keyword();
+  string16 keyword_candidate(turl.keyword());
   do {
     keyword_candidate.append(ASCIIToUTF16("_"));
   } while (GetTemplateURLForKeyword(keyword_candidate));
 
   return keyword_candidate;
 }
 
 bool TemplateURLService::ResolveSyncKeywordConflict(
     TemplateURL* sync_turl,
     SyncChangeList* change_list) {
@@ skipping 23 matching lines @@
     UpdateNoNotify(existing_turl, new_turl);
     NotifyObservers();
   }
   return true;
 }
 
 const TemplateURL* TemplateURLService::FindDuplicateOfSyncTemplateURL(
     const TemplateURL& sync_turl) {
   const TemplateURL* existing_turl =
       GetTemplateURLForKeyword(sync_turl.keyword());
-  if (!existing_turl)
-    return NULL;
-
-  if (!existing_turl->url().empty() &&
-      existing_turl->url() == sync_turl.url()) {
-    return existing_turl;
-  }
-  return NULL;
+  return existing_turl && !existing_turl->url().empty() &&
+      (existing_turl->url() == sync_turl.url()) ? existing_turl : NULL;
 }
 
 void TemplateURLService::MergeSyncAndLocalURLDuplicates(
     TemplateURL* sync_turl,
     TemplateURL* local_turl,
     SyncChangeList* change_list) {
   DCHECK(sync_turl);
   DCHECK(local_turl);
   DCHECK(change_list);
   scoped_ptr<TemplateURL> scoped_sync_turl(sync_turl);
@@ skipping 53 matching lines @@
       prefs->GetString(prefs::kSyncedDefaultSearchProviderGUID));
 }
 
 void TemplateURLService::PatchMissingSyncGUIDs(
     std::vector<TemplateURL*>* template_urls) {
   DCHECK(template_urls);
   for (std::vector<TemplateURL*>::iterator i = template_urls->begin();
        i != template_urls->end(); ++i) {
     TemplateURL* template_url = *i;
     DCHECK(template_url);
-    if (template_url->sync_guid().empty()) {
+    // Extension keywords are never synced.
+    // 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);
     }
   }
 }