Modify extension omnibox keywords to be user-configurable

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/compiler_specific.h"
 #include "base/environment.h"
@@ skipping 351 matching lines @@
   DCHECK(t_url);
   UIThreadSearchTermsData search_terms_data(t_url->profile());
   return GenerateSearchURLUsingTermsData(t_url, search_terms_data);
 }
 
 // static
 GURL TemplateURLService::GenerateSearchURLUsingTermsData(
     const TemplateURL* t_url,
     const SearchTermsData& search_terms_data) {
   DCHECK(t_url);
-  DCHECK(!t_url->IsExtensionKeyword());
 
   const TemplateURLRef& search_ref = t_url->url_ref();
   if (!search_ref.IsValidUsingTermsData(search_terms_data))
     return GURL();
 
   if (!search_ref.SupportsReplacementUsingTermsData(search_terms_data))
     return GURL(t_url->url());
 
   return GURL(search_ref.ReplaceSearchTermsUsingTermsData(
       TemplateURLRef::SearchTermsArgs(ASCIIToUTF16(kReplacementTerm)),
@@ skipping 194 matching lines @@
 TemplateURLService::TemplateURLVector TemplateURLService::GetTemplateURLs() {
   return template_urls_;
 }
 
 void TemplateURLService::IncrementUsageCount(TemplateURL* url) {
   DCHECK(url);
   if (std::find(template_urls_.begin(), template_urls_.end(), url) ==
       template_urls_.end())
     return;
   ++url->data_.usage_count;
-  // 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())
+  if (service_.get())
     service_.get()->UpdateKeyword(url->data());
 }
 
 void TemplateURLService::ResetTemplateURL(TemplateURL* url,
                                           const string16& title,
                                           const string16& keyword,
                                           const std::string& search_url) {
   if (!loaded_)
     return;
   DCHECK(!keyword.empty());
@@ skipping 99 matching lines @@
   TemplateURL* default_search_provider = NULL;
   // Force GetSearchProvidersUsingLoadedEngines() to include the prepopulated
   // engines in the list by claiming we are currently on version 0, ensuring
   // that the prepopulate data version will be newer.
   int new_resource_keyword_version = 0;
   GetSearchProvidersUsingLoadedEngines(service_.get(), profile_,
                                        &entries_to_process,
                                        &default_search_provider,
                                        &new_resource_keyword_version,
                                        &pre_sync_deletes_);
+  for (TemplateURLVector::iterator i = entries_to_process.begin();
+       i != entries_to_process.end();) {
+    if ((*i)->IsExtensionKeyword())
+      i = entries_to_process.erase(i);

[Dan Beam, 2013/06/05 03:55:53]

you may need to also explicitly delete the entries you're erasing from the
std::vector here if this class owns this data.  heapcheck is whining.

+    else
+      ++i;
+  }
   // Setup search engines and a default one.
   AddTemplateURLsAndSetupDefaultEngine(&entries_to_process,
                                        default_search_provider);
 
   if (new_resource_keyword_version)
     service_->SetBuiltinKeywordVersion(new_resource_keyword_version);
 
   EnsureDefaultSearchProviderExists();
   NotifyObservers();
 }
@@ skipping 140 matching lines @@
   UpdateDefaultSearch();
 }
 
 syncer::SyncDataList TemplateURLService::GetAllSyncData(
     syncer::ModelType type) const {
   DCHECK_EQ(syncer::SEARCH_ENGINES, type);
 
   syncer::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;
 }
 
 syncer::SyncError TemplateURLService::ProcessSyncChanges(
@@ skipping 290 matching lines @@
     syncer::SyncChange::SyncChangeType type) {
   DCHECK_NE(type, syncer::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;
 
   syncer::SyncChangeList changes;
 
   syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*turl);
   changes.push_back(syncer::SyncChange(from_here,
                                        type,
                                        sync_data));
@@ skipping 88 matching lines @@
   data.alternate_urls.clear();
   for (int i = 0; i < specifics.alternate_urls_size(); ++i)
     data.alternate_urls.push_back(specifics.alternate_urls(i));
   data.search_terms_replacement_key = specifics.search_terms_replacement_key();
 
   TemplateURL* turl = new TemplateURL(profile, data);
   // If this TemplateURL matches a built-in prepopulated template URL, it's
   // possible that sync is trying to modify fields that should not be touched.
   // Revert these fields to the built-in values.
   UpdateTemplateURLIfPrepopulated(turl, profile);
-  DCHECK(!turl->IsExtensionKeyword());
   if (reset_keyword || deduped) {
     if (reset_keyword)
       turl->ResetKeywordIfNecessary(true);
     syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*turl);
     change_list->push_back(syncer::SyncChange(FROM_HERE,
                                               syncer::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
@@ skipping 121 matching lines @@
           (!best_fallback || !best_fallback->IsExtensionKeyword() ||
            (turl->IsExtensionKeyword() && (turl->id() > best_fallback->id()))))
         best_fallback = turl;
     }
     if (best_fallback)
       keyword_to_template_map_[keyword] = best_fallback;
     else
       keyword_to_template_map_.erase(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_) {
     UIThreadSearchTermsData search_terms_data(template_url->profile());
     provider_map_->Remove(template_url, search_terms_data);
   }
 }
 
 void TemplateURLService::RemoveFromKeywordMapByPointer(
     TemplateURL* template_url) {
@@ skipping 22 matching lines @@
     // an extension.  In that case, the ranking order is:
     //   Manually-modified keywords > extension keywords > replaceable keywords
     // When there are multiple extensions, the last-added wins.
     bool existing_extension = existing_url->IsExtensionKeyword();
     DCHECK(existing_extension || template_extension);
     if (existing_extension ?
         !CanReplace(template_url) : CanReplace(existing_url))
       keyword_to_template_map_[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_extension)
-    return;
-
   if (!template_url->sync_guid().empty())
     guid_to_template_map_[template_url->sync_guid()] = template_url;
   if (loaded_) {
     UIThreadSearchTermsData search_terms_data(profile_);
     provider_map_->Add(template_url, search_terms_data);
   }
 }
 
 void TemplateURLService::SetTemplateURLs(const TemplateURLVector& urls) {
   // Add mappings for the new items.
@@ skipping 216 matching lines @@
 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, old_search_terms_data);
   TemplateURLID previous_id = existing_turl->id();
   existing_turl->CopyFrom(new_values);
   existing_turl->data_.id = previous_id;
   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
     //     which keyword has priority to update the keyword map.
     //   * Autogeneration of the keyword for a Google default search provider
     //     at load time causes it to conflict with an existing keyword.  In this
     //     case we delete the existing keyword if it's replaceable, or else undo
     //     the change in keyword for |existing_turl|.
-    DCHECK(!existing_turl->IsExtensionKeyword());
     TemplateURL* existing_keyword_turl = i->second;
     if (existing_keyword_turl->IsExtensionKeyword()) {
       if (!CanReplace(existing_turl))
         keyword_to_template_map_[keyword] = existing_turl;
     } else {
       if (CanReplace(existing_keyword_turl)) {
         RemoveNoNotify(existing_keyword_turl);
       } else {
         existing_turl->data_.SetKeyword(old_keyword);
         keyword_to_template_map_[old_keyword] = existing_turl;
@@ skipping 315 matching lines @@
   DCHECK(template_url);
 
   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_url->ResetKeywordIfNecessary(false);
-  if (!template_url->IsExtensionKeyword()) {
-    // Check whether |template_url|'s keyword conflicts with any already in the
-    // model.
-    TemplateURL* existing_keyword_turl =
-        FindNonExtensionTemplateURLForKeyword(template_url->keyword());
-    if (existing_keyword_turl != NULL) {
-      DCHECK_NE(existing_keyword_turl, template_url);
-      if (CanReplace(existing_keyword_turl)) {
-        RemoveNoNotify(existing_keyword_turl);
-      } else if (CanReplace(template_url)) {
-        delete template_url;
-        return false;
-      } else {
-        string16 new_keyword = UniquifyKeyword(*existing_keyword_turl, false);
-        ResetTemplateURL(existing_keyword_turl,
-                         existing_keyword_turl->short_name(), new_keyword,
-                         existing_keyword_turl->url());
-      }
+  // Check whether |template_url|'s keyword conflicts with any already in the
+  // model.
+  TemplateURL* existing_keyword_turl =
+      GetTemplateURLForKeyword(template_url->keyword());
+  if (existing_keyword_turl != NULL) {
+    DCHECK_NE(existing_keyword_turl, template_url);
+    // Only replace one of the TemplateURLs if they are either both extensions,
+    // or both not extensions.
+    bool are_same_type = existing_keyword_turl->IsExtensionKeyword() ==
+        template_url->IsExtensionKeyword();
+    if (CanReplace(existing_keyword_turl) && are_same_type) {
+      RemoveNoNotify(existing_keyword_turl);
+    } else if (CanReplace(template_url) && are_same_type) {
+      delete template_url;
+      return false;
+    } else {
+      string16 new_keyword = UniquifyKeyword(*existing_keyword_turl, false);
+      ResetTemplateURL(existing_keyword_turl,
+                       existing_keyword_turl->short_name(), new_keyword,
+                       existing_keyword_turl->url());
     }
   }
   template_urls_.push_back(template_url);
   AddToMaps(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())
+    if (service_.get())
       service_->AddKeyword(template_url->data());
 
     // 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(FROM_HERE,
                              template_url,
                              syncer::SyncChange::ACTION_ADD);
   }
 
   return true;
 }
 
 void TemplateURLService::RemoveNoNotify(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);
 
-  // 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())
+  if (service_.get())
     service_->RemoveKeyword(template_url->id());
 
   // Inform sync of the deletion.
   ProcessTemplateURLChange(FROM_HERE,
                            template_url,
                            syncer::SyncChange::ACTION_DELETE);
   UMA_HISTOGRAM_ENUMERATION(kDeleteSyncedEngineHistogramName,
       DELETE_ENGINE_USER_ACTION, DELETE_ENGINE_MAX);
 
   if (profile_) {
@@ skipping 48 matching lines @@
       }
 
       // 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);
-      // 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())
+      if (service_.get())
         service_->RemoveKeyword(template_url->id());
       delete template_url;
     } else {
       ++i;
     }
   }
 }
 
 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);
   UIThreadSearchTermsData search_terms_data(url->profile());
   UpdateNoNotify(url, new_url, search_terms_data);
 }
 
 string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl,
                                              bool force) {
   if (!force) {
     // Already unique.
     if (!GetTemplateURLForKeyword(turl.keyword()))
       return turl.keyword();
 
-    // First, try to return the generated keyword for the TemplateURL.
+    // First, try to return the generated keyword for the TemplateURL (except
+    // for extensions, as their keywords are not associated with their URLs).
     GURL gurl(turl.url());
-    if (gurl.is_valid()) {
+    if (gurl.is_valid() && !turl.IsExtensionKeyword()) {
       string16 keyword_candidate = GenerateKeyword(gurl);
       if (!GetTemplateURLForKeyword(keyword_candidate))
         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.
   string16 keyword_candidate(turl.keyword());
@@ skipping 15 matching lines @@
 
 void TemplateURLService::ResolveSyncKeywordConflict(
     TemplateURL* unapplied_sync_turl,
     TemplateURL* applied_sync_turl,
     syncer::SyncChangeList* change_list) {
   DCHECK(loaded_);
   DCHECK(unapplied_sync_turl);
   DCHECK(applied_sync_turl);
   DCHECK(change_list);
   DCHECK_EQ(applied_sync_turl->keyword(), unapplied_sync_turl->keyword());
-  DCHECK(!applied_sync_turl->IsExtensionKeyword());
 
   // Both |unapplied_sync_turl| and |applied_sync_turl| are known to Sync, so
   // don't delete either of them. Instead, determine which is "better" and
   // uniquify the other one, sending an update to the server for the updated
   // entry.
   const bool applied_turl_is_better =
       IsLocalTemplateURLBetter(applied_sync_turl, unapplied_sync_turl);
   TemplateURL* loser = applied_turl_is_better ?
       unapplied_sync_turl : applied_sync_turl;
   string16 new_keyword = UniquifyKeyword(*loser, false);
@@ skipping 131 matching lines @@
       prefs->GetString(prefs::kSyncedDefaultSearchProviderGUID));
 }
 
 void TemplateURLService::PatchMissingSyncGUIDs(
     TemplateURLVector* template_urls) {
   DCHECK(template_urls);
   for (TemplateURLVector::iterator i = template_urls->begin();
        i != template_urls->end(); ++i) {
     TemplateURL* template_url = *i;
     DCHECK(template_url);
-    // 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()) {
+    if (template_url->sync_guid().empty()) {
       template_url->data_.sync_guid = base::GenerateGUID();
       if (service_.get())
         service_->UpdateKeyword(template_url->data());
     }
   }
 }
 
 void TemplateURLService::AddTemplateURLsAndSetupDefaultEngine(
     TemplateURLVector* template_urls,
     TemplateURL* default_search_provider) {
@@ skipping 92 matching lines @@
     // Don't log anything if the user has a NULL default search provider. A
     // logged value of 0 indicates a custom default search provider.
     if (default_search_provider_) {
       UMA_HISTOGRAM_ENUMERATION(
           kDSPHistogramName,
           default_search_provider_->prepopulate_id(),
           TemplateURLPrepopulateData::kMaxPrepopulatedEngineID);
     }
   }
 }