Changed keywords conflicts resolution for extensions search engines.

components/search_engines/template_url_service.cc

Index: components/search_engines/template_url_service.cc
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index be5527fc553acb051431dd0a742b3f7a3cf3827c..8500480c9d9e7f746d56fc9fe1dc3940b8b97021 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -188,7 +188,7 @@ bool Contains(TemplateURLService::OwnedTemplateURLVector* template_urls,
   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;
@@ -530,10 +530,11 @@ void TemplateURLService::RemoveAutoGeneratedForUrlsBetween(
 }
 
 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,
@@ -546,6 +547,7 @@ void TemplateURLService::RegisterOmniboxKeyword(
   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));
@@ -1443,6 +1445,29 @@ void TemplateURLService::Init(const Initializer* initializers,
   RequestGoogleURLTrackerServerCheckIfNecessary();
 }
 
+TemplateURL* TemplateURLService::BestEngineForKeyword(TemplateURL* engine1,
+                                                      TemplateURL* engine2) {
+  DCHECK(engine1);
+  DCHECK(engine2);
+  DCHECK_EQ(engine1->keyword(), engine2->keyword());
+  // We should only have overlapping keywords when at least one comes from
+  // an extension.
+  DCHECK(IsCreatedByExtension(engine1) || IsCreatedByExtension(engine2));
+
+  if (engine2->type() == engine1->type()) {
+    return engine1->extension_info_->install_time >
+                   engine2->extension_info_->install_time
+               ? engine1
+               : engine2;
+  }
+  if (engine2->type() == TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION) {
+    return engine1->type() == TemplateURL::OMNIBOX_API_EXTENSION ? engine1
+                                                                 : engine2;
+  }
+  return engine2->type() == TemplateURL::OMNIBOX_API_EXTENSION ? engine2
+                                                               : engine1;
+}
+
 void TemplateURLService::RemoveFromMaps(TemplateURL* template_url) {
   const base::string16& keyword = template_url->keyword();
   DCHECK_NE(0U, keyword_to_turl_and_length_.count(keyword));
@@ -1455,14 +1480,12 @@ void TemplateURLService::RemoveFromMaps(TemplateURL* template_url) {
     // 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)) {
+        if (best_fallback)
+          best_fallback = BestEngineForKeyword(best_fallback, turl.get());
+        else
+          best_fallback = turl.get();
+      }
     }
     RemoveFromDomainMap(template_url);
     if (best_fallback) {
@@ -1494,16 +1517,8 @@ void TemplateURLService::AddToMaps(TemplateURL* template_url) {
     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);
@@ -1651,7 +1666,7 @@ bool TemplateURLService::UpdateNoNotify(TemplateURL* 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);
@@ -1962,8 +1977,7 @@ bool TemplateURLService::ApplyDefaultSearchChangeNoMetrics(
   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);
@@ -2032,39 +2046,32 @@ TemplateURL* TemplateURLService::AddNoNotify(
   }
 
   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());
+
+  // If |template_url| is not created by an extension, its keyword must not
+  // conflict with any already in the model.
+  if (!IsCreatedByExtension(template_url.get())) {
+    // 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.
+    TemplateURL* existing_turl =
+        FindNonExtensionTemplateURLForKeyword(template_url->keyword());
+    if (existing_turl && Contains(&template_urls_, existing_turl)) {
+      DCHECK_NE(existing_turl, template_url.get());
+      if (CanReplace(existing_turl)) {
+        RemoveNoNotify(existing_turl);
+      } else if (CanReplace(template_url.get())) {
+        return nullptr;
+      } else {
+        // Neither engine can be replaced. Uniquify the existing keyword.
+        base::string16 new_keyword = UniquifyKeyword(*existing_turl, false);
+        ResetTemplateURLNoNotify(existing_turl, existing_turl->short_name(),
+                                 new_keyword, existing_turl->url());
+      }
     }
   }
   TemplateURL* template_url_ptr = template_url.get();
@@ -2208,6 +2215,7 @@ void TemplateURLService::ResetTemplateURLGUID(TemplateURL* url,
 
 base::string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl,
                                                    bool force) {
+  DCHECK(!IsCreatedByExtension(&turl));
   if (!force) {
     // Already unique.
     if (!GetTemplateURLForKeyword(turl.keyword()))
@@ -2216,8 +2224,7 @@ base::string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl,
     // 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;
@@ -2470,12 +2477,11 @@ TemplateURL* TemplateURLService::FindTemplateURLForExtension(
   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();
   }