Changed keywords conflicts resolution for extensions search engines.

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 170 matching lines @@
   // The meaningful keyword length is the length of any portion before the
   // registry ("co.uk") and its preceding dot.
   return keyword.length() - (registry_length ? (registry_length + 1) : 0);
 }
 
 bool Contains(TemplateURLService::OwnedTemplateURLVector* template_urls,
               TemplateURL* turl) {
   return FindTemplateURL(template_urls, turl) != template_urls->end();
 }
 
-bool IsCreatedByExtension(TemplateURL* template_url) {
+bool IsCreatedByExtension(const TemplateURL* template_url) {
   return template_url->type() ==
              TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION ||
          template_url->type() == TemplateURL::OMNIBOX_API_EXTENSION;
 }
 
 }  // namespace
 
 // TemplateURLService::LessWithPrefix -----------------------------------------
 
 class TemplateURLService::LessWithPrefix {
@@ skipping 321 matching lines @@
       should_notify = true;
     } else {
       ++i;
     }
   }
   if (should_notify)
     NotifyObservers();
 }
 
 void TemplateURLService::RegisterOmniboxKeyword(
-     const std::string& extension_id,
-     const std::string& extension_name,
-     const std::string& keyword,
-     const std::string& template_url_string) {
+    const std::string& extension_id,
+    const std::string& extension_name,
+    const std::string& keyword,
+    const std::string& template_url_string,
+    const base::Time& extension_install_time) {
   DCHECK(loaded_);
 
   if (FindTemplateURLForExtension(extension_id,
                                   TemplateURL::OMNIBOX_API_EXTENSION))
     return;
 
   TemplateURLData data;
   data.SetShortName(base::UTF8ToUTF16(extension_name));
   data.SetKeyword(base::UTF8ToUTF16(keyword));
   data.SetURL(template_url_string);
   std::unique_ptr<TemplateURL::AssociatedExtensionInfo> info(
       new TemplateURL::AssociatedExtensionInfo(extension_id));
+  info->install_time = extension_install_time;
   AddExtensionControlledTURL(
       base::MakeUnique<TemplateURL>(data, TemplateURL::OMNIBOX_API_EXTENSION),
       std::move(info));
 }
 
 TemplateURLService::TemplateURLVector TemplateURLService::GetTemplateURLs() {
   TemplateURLVector result;
   for (const auto& turl : template_urls_)
     result.push_back(turl.get());
   return result;
@@ skipping 877 matching lines @@
       if (i == 0)
         default_search_manager_.SetUserSelectedDefaultSearchEngine(data);
     }
   }
 
   // Request a server check for the correct Google URL if Google is the
   // default search engine.
   RequestGoogleURLTrackerServerCheckIfNecessary();
 }
 
+TemplateURL* TemplateURLService::BestEngineForKeyword(
+    TemplateURL* existing_engine,
+    TemplateURL* new_engine) {
+  DCHECK(new_engine);
+  if (!existing_engine)
+    return new_engine;  // Better than nothing.
+
+  DCHECK_EQ(new_engine->keyword(), existing_engine->keyword());
+  // We should only have overlapping keywords when at least one comes from
+  // an extension.
+  DCHECK(IsCreatedByExtension(existing_engine) ||
+         IsCreatedByExtension(new_engine));
+
+  // Following ranking is implemented:
+  // omnibox api keyword > extension search engine > normal engine.
+  // If both extensions are of same type, latest installed wins.
+  if (new_engine->type() == existing_engine->type()) {
+    return existing_engine->extension_info_->install_time >
+                   new_engine->extension_info_->install_time
+               ? existing_engine
+               : new_engine;
+  } else if (new_engine->type() ==

[vasilii, 2017/02/20 15:30:08]

You don't need else as the previous block always returns something.

[Alexander Yashkin, 2017/02/20 19:52:47]

Done.

+             TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION) {
+    return existing_engine->type() == TemplateURL::OMNIBOX_API_EXTENSION
+               ? existing_engine
+               : new_engine;
+  } else {
+    // Omnibox api keyword always wins. Normal engine loses to any extensions
+    // keywords.
+    return new_engine->type() == TemplateURL::OMNIBOX_API_EXTENSION
+               ? new_engine
+               : existing_engine;
+  }
+}
+
 void TemplateURLService::RemoveFromMaps(TemplateURL* template_url) {
   const base::string16& keyword = template_url->keyword();
   DCHECK_NE(0U, keyword_to_turl_and_length_.count(keyword));
   if (keyword_to_turl_and_length_[keyword].first == template_url) {
     // We need to check whether the keyword can now be provided by another
     // TemplateURL.  See the comments in AddToMaps() for more information on
     // extension keywords and how they can coexist with non-extension keywords.
     // In the case of more than one extension, we use the most recently
     // installed (which will be the most recently added, which will have the
     // highest ID).
     TemplateURL* best_fallback = nullptr;
     for (const auto& turl : template_urls_) {
-      // This next statement relies on the fact that there can only be one
-      // non-Omnibox API TemplateURL with a given keyword.
-      if ((turl.get() != template_url) && (turl->keyword() == keyword) &&
-          (!best_fallback ||
-           (best_fallback->type() != TemplateURL::OMNIBOX_API_EXTENSION) ||
-           ((turl->type() == TemplateURL::OMNIBOX_API_EXTENSION) &&
-            (turl->id() > best_fallback->id()))))
-        best_fallback = turl.get();
+      if ((turl.get() != template_url) && (turl->keyword() == keyword))
+        best_fallback = BestEngineForKeyword(best_fallback, turl.get());
     }
     RemoveFromDomainMap(template_url);
     if (best_fallback) {
       AddToMap(best_fallback);
       AddToDomainMap(best_fallback);
     } else {
       keyword_to_turl_and_length_.erase(keyword);
     }
   }
 
@@ skipping 11 matching lines @@
 void TemplateURLService::AddToMaps(TemplateURL* template_url) {
   bool template_url_is_omnibox_api =
       template_url->type() == TemplateURL::OMNIBOX_API_EXTENSION;
   const base::string16& keyword = template_url->keyword();
   KeywordToTURLAndMeaningfulLength::const_iterator i =
       keyword_to_turl_and_length_.find(keyword);
   if (i == keyword_to_turl_and_length_.end()) {
     AddToMap(template_url);
     AddToDomainMap(template_url);
   } else {
-    const TemplateURL* existing_url = i->second.first;
-    // We should only have overlapping keywords when at least one comes from
-    // 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_url_is_omnibox_api =
-        existing_url->type() == 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)) {
+    TemplateURL* existing_url = i->second.first;
+    if (BestEngineForKeyword(existing_url, template_url) != existing_url) {
       RemoveFromDomainMap(existing_url);
       AddToMap(template_url);
       AddToDomainMap(template_url);
     }
   }
 
   if (template_url_is_omnibox_api)
     return;
 
   if (!template_url->sync_guid().empty())
@@ skipping 127 matching lines @@
   }
   return nullptr;
 }
 
 bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
                                         const TemplateURL& new_values) {
   DCHECK(existing_turl);
   if (!Contains(&template_urls_, existing_turl))
     return false;
 
-  DCHECK_NE(TemplateURL::OMNIBOX_API_EXTENSION, existing_turl->type());
+  DCHECK(!IsCreatedByExtension(existing_turl));
 
   base::string16 old_keyword(existing_turl->keyword());
   keyword_to_turl_and_length_.erase(old_keyword);
   RemoveFromDomainMap(existing_turl);
   if (!existing_turl->sync_guid().empty())
     guid_to_turl_.erase(existing_turl->sync_guid());
 
   // |provider_map_| is only initialized after loading has completed.
   if (loaded_)
     provider_map_->Remove(existing_turl);
@@ skipping 290 matching lines @@
   }
 
   // |default_search_provider_source_| must be set before calling
   // UpdateNoNotify(), since that function needs to know the source of the
   // update in question.
   default_search_provider_source_ = source;
 
   if (!data) {
     default_search_provider_ = nullptr;
   } else if (source == DefaultSearchManager::FROM_EXTENSION) {
-    default_search_provider_ = FindMatchingExtensionTemplateURL(
-        *data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+    default_search_provider_ = FindMatchingDefaultExtensionTemplateURL(*data);
   } else if (source == DefaultSearchManager::FROM_FALLBACK) {
     default_search_provider_ =
         FindPrepopulatedTemplateURL(data->prepopulate_id);
     if (default_search_provider_) {
       TemplateURLData update_data(*data);
       update_data.sync_guid = default_search_provider_->sync_guid();
       if (!default_search_provider_->safe_for_autoreplace()) {
         update_data.safe_for_autoreplace = false;
         update_data.SetKeyword(default_search_provider_->keyword());
         update_data.SetShortName(default_search_provider_->short_name());
@@ skipping 48 matching lines @@
     bool newly_adding) {
   DCHECK(template_url);
 
   if (newly_adding) {
     DCHECK_EQ(kInvalidTemplateURLID, template_url->id());
     DCHECK(!Contains(&template_urls_, template_url.get()));
     template_url->data_.id = ++next_id_;
   }
 
   template_url->ResetKeywordIfNecessary(search_terms_data(), false);
-  // Check whether |template_url|'s keyword conflicts with any already in the
-  // model.
-  TemplateURL* existing_keyword_turl =
-      GetTemplateURLForKeyword(template_url->keyword());
 
-  // 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 (template_url->type() != TemplateURL::OMNIBOX_API_EXTENSION &&
-      existing_keyword_turl &&
-      existing_keyword_turl->type() != TemplateURL::OMNIBOX_API_EXTENSION &&
-      Contains(&template_urls_, existing_keyword_turl)) {
-    DCHECK_NE(existing_keyword_turl, template_url.get());
-    // Only replace one of the TemplateURLs if they are either both extensions,
-    // or both not extensions.
-    bool are_same_type = IsCreatedByExtension(existing_keyword_turl) ==
-                         IsCreatedByExtension(template_url.get());
-    if (CanReplace(existing_keyword_turl) && are_same_type) {
-      RemoveNoNotify(existing_keyword_turl);
-    } else if (CanReplace(template_url.get()) && are_same_type) {
-      return nullptr;
-    } 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());
+  // Check for conflicts between non extension engines keywords.
+  if (!IsCreatedByExtension(template_url.get())) {
+    // Check whether |template_url|'s keyword conflicts with any already in the
+    // model.
+    TemplateURL* existing_turl =
+        FindNonExtensionTemplateURLForKeyword(template_url->keyword());
+
+    // 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_turl && Contains(&template_urls_, existing_turl)) {
+      DCHECK_NE(existing_turl, template_url.get());
+      if (CanReplace(existing_turl))

[vasilii, 2017/02/20 15:30:08]

{} here and below.

[Alexander Yashkin, 2017/02/20 19:52:47]

Done.

+        RemoveNoNotify(existing_turl);
+      else if (CanReplace(template_url.get()))
+        return nullptr;
+      else {
+        base::string16 new_keyword = UniquifyKeyword(*existing_turl, false);
+        // Resolve keyword conflicts for engines that can not be replaced.
+        // Replace keyword for existing engine.
+        ResetTemplateURLNoNotify(existing_turl, existing_turl->short_name(),
+                                 new_keyword, existing_turl->url());
+      }
     }
   }
   TemplateURL* template_url_ptr = template_url.get();
   template_urls_.push_back(std::move(template_url));
   AddToMaps(template_url_ptr);
 
   if (newly_adding && (template_url_ptr->type() == TemplateURL::NORMAL)) {
     if (web_data_service_.get())
       web_data_service_->AddKeyword(template_url_ptr->data());
 
@@ skipping 123 matching lines @@
   DCHECK(loaded_);
   DCHECK(!guid.empty());
 
   TemplateURLData data(url->data());
   data.sync_guid = guid;
   UpdateNoNotify(url, TemplateURL(data));
 }
 
 base::string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl,
                                                    bool force) {
+  DCHECK(!IsCreatedByExtension(&turl));
   if (!force) {
     // Already unique.
     if (!GetTemplateURLForKeyword(turl.keyword()))
       return turl.keyword();
 
     // 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() &&
-        (turl.type() != TemplateURL::OMNIBOX_API_EXTENSION)) {
+    if (gurl.is_valid()) {
       base::string16 keyword_candidate = TemplateURL::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.
   base::string16 keyword_candidate(turl.keyword());
@@ skipping 232 matching lines @@
     const std::string& extension_id,
     TemplateURL::Type type) {
   DCHECK_NE(TemplateURL::NORMAL, type);
   for (const auto& turl : template_urls_) {
     if (turl->type() == type && turl->GetExtensionId() == extension_id)
       return turl.get();
   }
   return nullptr;
 }
 
-TemplateURL* TemplateURLService::FindMatchingExtensionTemplateURL(
-    const TemplateURLData& data,
-    TemplateURL::Type type) {
-  DCHECK_NE(TemplateURL::NORMAL, type);
+TemplateURL* TemplateURLService::FindMatchingDefaultExtensionTemplateURL(
+    const TemplateURLData& data) {
   for (const auto& turl : template_urls_) {
-    if (turl->type() == type &&
+    if (turl->type() == TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION &&
+        turl->extension_info_->wants_to_be_default_engine &&
         TemplateURL::MatchesData(turl.get(), &data, search_terms_data()))
       return turl.get();
   }
   return nullptr;
 }
 
 void TemplateURLService::UpdateExtensionDefaultSearchEngine() {
   TemplateURL* most_recently_intalled_default = nullptr;
   for (const auto& turl : template_urls_) {
     if ((turl->type() == TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION) &&
         turl->extension_info_->wants_to_be_default_engine &&
         turl->SupportsReplacement(search_terms_data()) &&
         (!most_recently_intalled_default ||
          (most_recently_intalled_default->extension_info_->install_time <
           turl->extension_info_->install_time)))
       most_recently_intalled_default = turl.get();
   }
 
   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();
   }
 }