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 0e77d27379f265169564fc3bb4cf7eff964c7a4f..6d19120cd5b746ed0116fcc0831d43be94129530 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -2041,11 +2041,39 @@ TemplateURL* TemplateURLService::AddNoNotify(
     } else if (CanReplace(template_url.get()) && are_same_type) {
       return nullptr;
     } else {
+      // Resolve keyword conflicts for engines that can not be replaced.
+      // Use following priority while resolving keyword conflicts:
+      // extension default search -> extension search -> normal engines.
+      // By extension we mean NORMAL_CONTROLLED_BY_EXTENSION type of engine, not
+      // the OMNIBOX_API_EXTENSION.

[Peter Kasting, 2017/02/07 00:42:45]

Nit: Might want to say "for engines of the same priority, the newly-added one
wins."

[Alexander Yashkin, 2017/02/07 08:09:20]

Done

+      bool new_engine_is_default_extension =
+          IsCreatedByExtension(template_url.get()) &&
+          template_url->extension_info_->wants_to_be_default_engine;
+
+      bool old_engine_is_default_extension =
+          IsCreatedByExtension(existing_keyword_turl) &&
+          existing_keyword_turl->extension_info_->wants_to_be_default_engine;
+
+      // The only way we allow old engine to hold its keyword, is if it was
+      // extension installed default engine and new engine is not. And the
+      // second case is if new added engine is normal and old was from
+      // extension.
+      bool new_engine_lose = (old_engine_is_default_extension &&
+                              !new_engine_is_default_extension) ||
+                             (IsCreatedByExtension(existing_keyword_turl) &&
+                              !IsCreatedByExtension(template_url.get()));
+
       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 (new_engine_lose) {

[Peter Kasting, 2017/02/07 00:42:45]

Nit: If we just do this here:

      if (IsCreatedByExtension(existing_turl) &&
          (!IsCreatedByExtension(template_url.get()) ||
           (existing_turl->extension_info_->wants_to_be_default_engine &&
            !template_url->extension_info_->wants_to_be_default_engine))) {

...then we can eliminate basically all the new code above.  (But keep the
comment at the top).

Note that for better line wrapping, I renamed |existing_keyword_turl| to
|existing_turl|, which doesn't seem less clear in context.

[Alexander Yashkin, 2017/02/07 08:09:20]

Done, hope it would be clear to next generations :).

+        // Replace keyword for added search engine.
+        template_url->data_.SetKeyword(new_keyword);
+      } else {
+        // Replace keyword for existing engine.
+        ResetTemplateURLNoNotify(existing_keyword_turl,
+                                 existing_keyword_turl->short_name(),
+                                 new_keyword, existing_keyword_turl->url());
+      }
     }
   }
   TemplateURL* template_url_ptr = template_url.get();