Don't persist and sync omnibox extension keywords.

components/search_engines/template_url_service.cc

 // Copyright 2014 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 "components/search_engines/template_url_service.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 646 matching lines @@
   AddExtensionControlledTURL(url, info.Pass());
 }
 
 TemplateURLService::TemplateURLVector TemplateURLService::GetTemplateURLs() {
   return template_urls_;
 }
 
 void TemplateURLService::IncrementUsageCount(TemplateURL* url) {
   DCHECK(url);
   // Extension-controlled search engines are not persisted.
-  if (url->GetType() == TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)
+  if (url->GetType() != TemplateURL::NORMAL)
     return;
   if (std::find(template_urls_.begin(), template_urls_.end(), url) ==
       template_urls_.end())
     return;
   ++url->data_.usage_count;
 
   if (web_data_service_.get())
     web_data_service_->UpdateKeyword(url->data());
 }
 
@@ skipping 191 matching lines @@
         &new_resource_keyword_version,
         &pre_sync_deletes_);
   }
 
   if (client_) {
     // TODO(vadimt): Remove ScopedProfile below once crbug.com/422460 is fixed.
     tracked_objects::ScopedProfile tracking_profile3(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "422460 TemplateURLService::OnWebDataServiceRequestDone 3"));
 
+    // We used to store omnibox extensions keywords. Now they are't persisted.
+    TemplateURLVector template_urls_no_extensions;
     // Restore extension info of loaded TemplateURLs.
     for (size_t i = 0; i < template_urls.size(); ++i) {
       DCHECK(!template_urls[i]->extension_info_);
       client_->RestoreExtensionInfoIfNecessary(template_urls[i]);
+      if (template_urls[i]->GetType() == TemplateURL::NORMAL) {
+        template_urls_no_extensions.push_back(template_urls[i]);
+      } else {
+        web_data_service_->RemoveKeyword(template_urls[i]->id());
+        delete template_urls[i];
+      }
     }
+    template_urls.swap(template_urls_no_extensions);
   }
 
   KeywordWebDataService::BatchModeScoper scoper(web_data_service_.get());
 
   {
     // TODO(vadimt): Remove ScopedProfile below once crbug.com/422460 is fixed.
     tracked_objects::ScopedProfile tracking_profile4(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "422460 TemplateURLService::OnWebDataServiceRequestDone 4"));
 
@@ skipping 98 matching lines @@
     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 keywords managed by policy.
     if ((*iter)->created_by_policy())
       continue;
     // We don't sync extension-controlled search engines.
-    if ((*iter)->GetType() == TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)
+    if ((*iter)->GetType() != TemplateURL::NORMAL)
       continue;
     current_data.push_back(CreateSyncDataFromTemplateURL(**iter));
   }
 
   return current_data;
 }
 
 syncer::SyncError TemplateURLService::ProcessSyncChanges(
     const tracked_objects::Location& from_here,
     const syncer::SyncChangeList& change_list) {
@@ skipping 12 matching lines @@
   // As we move through Sync Code, we may set this to increasingly specific
   // origins so we can tell what exactly caused a DSP change.
   base::AutoReset<DefaultSearchChangeOrigin> change_origin(&dsp_change_origin_,
       DSP_CHANGE_SYNC_UNINTENTIONAL);
 
   KeywordWebDataService::BatchModeScoper scoper(web_data_service_.get());
 
   syncer::SyncChangeList new_changes;
   syncer::SyncError error;
   for (syncer::SyncChangeList::const_iterator iter = change_list.begin();
-      iter != change_list.end(); ++iter) {
+       iter != change_list.end(); ++iter) {
     DCHECK_EQ(syncer::SEARCH_ENGINES, iter->sync_data().GetDataType());
 
     std::string guid =
         iter->sync_data().GetSpecifics().search_engine().sync_guid();
     TemplateURL* existing_turl = GetTemplateURLForGUID(guid);
     scoped_ptr<TemplateURL> turl(CreateTemplateURLFromTemplateURLAndSyncData(
-        prefs_, search_terms_data(), existing_turl, iter->sync_data(),
-        &new_changes));
+        client_.get(), prefs_, search_terms_data(), existing_turl,
+        iter->sync_data(), &new_changes));
     if (!turl.get())
       continue;
 
     // Explicitly don't check for conflicts against extension keywords; in this
     // case the functions which modify the keyword map know how to handle the
     // conflicts.
     // TODO(mpcomplete): If we allow editing extension keywords, then those will
     // need to undergo conflict resolution.
     TemplateURL* existing_keyword_turl =
         FindNonExtensionTemplateURLForKeyword(turl->keyword());
@@ skipping 133 matching lines @@
   SyncDataMap sync_data_map = CreateGUIDToSyncDataMap(initial_sync_data);
 
   KeywordWebDataService::BatchModeScoper scoper(web_data_service_.get());
 
   merge_result.set_num_items_before_association(local_data_map.size());
   for (SyncDataMap::const_iterator iter = sync_data_map.begin();
       iter != sync_data_map.end(); ++iter) {
     TemplateURL* local_turl = GetTemplateURLForGUID(iter->first);
     scoped_ptr<TemplateURL> sync_turl(
         CreateTemplateURLFromTemplateURLAndSyncData(
-            prefs_, search_terms_data(), local_turl, iter->second,
-            &new_changes));
+            client_.get(), prefs_, search_terms_data(), local_turl,
+            iter->second, &new_changes));
     if (!sync_turl.get())
       continue;
 
     if (pre_sync_deletes_.find(sync_turl->sync_guid()) !=
         pre_sync_deletes_.end()) {
       // This entry was deleted before the initial sync began (possibly through
       // preprocessing in TemplateURLService's loading code). Ignore it and send
       // an ACTION_DELETE up to the server.
       new_changes.push_back(
           syncer::SyncChange(FROM_HERE,
@@ skipping 143 matching lines @@
     se_specifics->add_alternate_urls(turl.alternate_urls()[i]);
   se_specifics->set_search_terms_replacement_key(
       turl.search_terms_replacement_key());
 
   return syncer::SyncData::CreateLocalData(se_specifics->sync_guid(),
                                            se_specifics->keyword(),
                                            specifics);
 }
 
 // static
-TemplateURL* TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData(
+scoped_ptr<TemplateURL>
+TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData(
+    TemplateURLServiceClient* client,
     PrefService* prefs,
     const SearchTermsData& search_terms_data,
     TemplateURL* existing_turl,
     const syncer::SyncData& sync_data,
     syncer::SyncChangeList* change_list) {
   DCHECK(change_list);
 
   sync_pb::SearchEngineSpecifics specifics =
       sync_data.GetSpecifics().search_engine();
 
@@ skipping 44 matching lines @@
   bool deduped = DeDupeEncodings(&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();
   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(data);
+  scoped_ptr<TemplateURL> turl(new TemplateURL(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, prefs);
-  DCHECK_NE(TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION, turl->GetType());
+  UpdateTemplateURLIfPrepopulated(turl.get(), prefs);
+
+  // The omnibox keywords aren't synced anymore.

[Peter Kasting, 2014/11/04 19:11:56]

How about this:

We used to sync keywords associated with omnibox extensions, but no longer want
to.  However, if we delete these keywords from sync, we'll break any synced old
versions of Chrome which were relying on them.  Instead, for now we simply
ignore these.
TODO(vasilii): After a few Chrome versions, change this to go ahead and delete
these from sync.

[vasilii, 2014/11/05 18:04:50]

Done.

+  DCHECK(client);
+  client->RestoreExtensionInfoIfNecessary(turl.get());
+  if (turl->GetType() == TemplateURL::OMNIBOX_API_EXTENSION)
+    return NULL;
+
+  DCHECK_EQ(TemplateURL::NORMAL, turl->GetType());
   if (reset_keyword || deduped) {
     if (reset_keyword)
       turl->ResetKeywordIfNecessary(search_terms_data, 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(search_terms_data)) {
     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(search_terms_data, false);
     } else if (existing_turl->IsGoogleSearchURLWithReplaceableKeyword(
         search_terms_data)) {
       // 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;
+  return turl.Pass();
 }
 
 // static
 SyncDataMap TemplateURLService::CreateGUIDToSyncDataMap(
     const syncer::SyncDataList& sync_data) {
   SyncDataMap data_map;
   for (syncer::SyncDataList::const_iterator i(sync_data.begin());
        i != sync_data.end();
        ++i)
     data_map[i->GetSpecifics().search_engine().sync_guid()] = *i;
@@ skipping 80 matching lines @@
            ((turl->GetType() == TemplateURL::OMNIBOX_API_EXTENSION) &&
             (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 (template_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION)
+    return;
+
   if (!template_url->sync_guid().empty())
     guid_to_template_map_.erase(template_url->sync_guid());
   // |provider_map_| is only initialized after loading has completed.
   if (loaded_) {
     provider_map_->Remove(template_url);
   }
 }
 
 void TemplateURLService::AddToMaps(TemplateURL* template_url) {
   bool template_url_is_omnibox_api =
@@ skipping 10 matching lines @@
     //   Manually-modified keywords > extension keywords > replaceable keywords
     // When there are multiple extensions, the last-added wins.
     bool existing_url_is_omnibox_api =
         existing_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION;
     DCHECK(existing_url_is_omnibox_api || template_url_is_omnibox_api);
     if (existing_url_is_omnibox_api ?
         !CanReplace(template_url) : CanReplace(existing_url))
       keyword_to_template_map_[keyword] = template_url;
   }
 
+  if (template_url_is_omnibox_api)
+    return;
+
   if (!template_url->sync_guid().empty())
     guid_to_template_map_[template_url->sync_guid()] = template_url;
   // |provider_map_| is only initialized after loading has completed.
   if (loaded_)
     provider_map_->Add(template_url, search_terms_data());
 }
 
 // Helper for partition() call in next function.
 bool HasValidID(TemplateURL* t_url) {
   return t_url->id() != kInvalidTemplateURLID;
@@ skipping 80 matching lines @@
   return NULL;
 }
 
 bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
                                         const TemplateURL& new_values) {
   DCHECK(existing_turl);
   if (std::find(template_urls_.begin(), template_urls_.end(), existing_turl) ==
       template_urls_.end())
     return false;
 
+  DCHECK_NE(TemplateURL::OMNIBOX_API_EXTENSION, existing_turl->GetType());
+
   base::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_| is only initialized after loading has completed.
   if (loaded_)
     provider_map_->Remove(existing_turl);
 
   TemplateURLID previous_id = existing_turl->id();
@@ skipping 320 matching lines @@
 
   // Check whether |template_url|'s keyword conflicts with any already in the
   // model.  Note that we can reach here during the loading phase while
   // processing the template URLs from the web data service.  In this case,
   // GetTemplateURLForKeyword() will look not only at what's already in the
   // model, but at the |initial_default_search_provider_|.  Since this engine
   // will presumably also be present in the web data, we need to double-check
   // that any "pre-existing" entries we find are actually coming from
   // |template_urls_|, lest we detect a "conflict" between the
   // |initial_default_search_provider_| and the web data version of itself.
-  if (existing_keyword_turl &&
+  if (template_url->GetType() != TemplateURL::OMNIBOX_API_EXTENSION &&
+      existing_keyword_turl &&
+      existing_keyword_turl->GetType() != TemplateURL::OMNIBOX_API_EXTENSION &&
       (std::find(template_urls_.begin(), template_urls_.end(),
                  existing_keyword_turl) != template_urls_.end())) {
     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->GetType() ==
         template_url->GetType();
     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 {
       base::string16 new_keyword =
           UniquifyKeyword(*existing_keyword_turl, false);
       ResetTemplateURLNoNotify(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 &&
-      (template_url->GetType() !=
-          TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)) {
+      (template_url->GetType() == TemplateURL::NORMAL)) {
     if (web_data_service_.get())
       web_data_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) {
   DCHECK(template_url != default_search_provider_);
 
   TemplateURLVector::iterator i =
       std::find(template_urls_.begin(), template_urls_.end(), template_url);
   if (i == template_urls_.end())
     return;
 
   RemoveFromMaps(template_url);
 
   // Remove it from the vector containing all TemplateURLs.
   template_urls_.erase(i);
 
-  if (template_url->GetType() != TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION) {
+  if (template_url->GetType() == TemplateURL::NORMAL) {
     if (web_data_service_.get())
       web_data_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);
@@ skipping 137 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_NE(TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION,
-            applied_sync_turl->GetType());
+  DCHECK_EQ(TemplateURL::NORMAL, applied_sync_turl->GetType());
 
   // 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;
   base::string16 new_keyword = UniquifyKeyword(*loser, false);
@@ skipping 90 matching lines @@
 }
 
 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);
     if (template_url->sync_guid().empty() &&
-        (template_url->GetType() !=
-            TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION)) {
+        (template_url->GetType() == TemplateURL::NORMAL)) {
       template_url->data_.sync_guid = base::GenerateGUID();
       if (web_data_service_.get())
         web_data_service_->UpdateKeyword(template_url->data());
     }
   }
 }
 
 void TemplateURLService::OnSyncedDefaultSearchProviderGUIDChanged() {
   base::AutoReset<DefaultSearchChangeOrigin> change_origin(
       &dsp_change_origin_, DSP_CHANGE_SYNC_PREF);
@@ skipping 61 matching lines @@
 
   if (most_recently_intalled_default) {
     base::AutoReset<DefaultSearchChangeOrigin> change_origin(
         &dsp_change_origin_, DSP_CHANGE_OVERRIDE_SETTINGS_EXTENSION);
     default_search_manager_.SetExtensionControlledDefaultSearchEngine(
         most_recently_intalled_default->data());
   } else {
     default_search_manager_.ClearExtensionControlledDefaultSearchEngine();
   }
 }