Changed keywords conflicts resolution for extensions search engines.

chrome/browser/search_engines/template_url_service_unittest.cc

Index: chrome/browser/search_engines/template_url_service_unittest.cc
diff --git a/chrome/browser/search_engines/template_url_service_unittest.cc b/chrome/browser/search_engines/template_url_service_unittest.cc
index ddc06380b068dc5a15b115a81b7277584f0d54a4..bc4d94400e113effc429a4f0c95c2f49df47b39d 100644
--- a/chrome/browser/search_engines/template_url_service_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_unittest.cc
@@ -28,6 +28,7 @@
 #include "chrome/test/base/testing_profile.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/search_engines/keyword_web_data_service.h"
+#include "components/search_engines/search_engines_test_util.h"
 #include "components/search_engines/search_host_to_urls_map.h"
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url.h"
@@ -1481,6 +1482,105 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
 }
 
+// Check that default extension engine and user engines with same keywords are
+// handled corrected.
+TEST_F(TemplateURLServiceTest, DefaultExtensionAndUserEnginesWithSameKeywords) {
+  test_util()->VerifyLoad();
+  // TemplateURLData used for all engines.
+  auto turl_data = GenerateDummyTemplateURLData("ext2");

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

Nit: "ext2" is pretty random, especially when the test doesn't use "ext" or
"ext1".  Just use "extension".

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

Done

+  turl_data->safe_for_autoreplace = false;
+
+  // Add non replaceable user engine.
+  const TemplateURL* user1 =
+      model()->Add(base::MakeUnique<TemplateURL>(*turl_data));
+
+  // Add default extension engine with same keyword.
+  auto ext_dse = base::MakeUnique<TemplateURL>(
+      *turl_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  auto extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("ext2");
+  extension_info->wants_to_be_default_engine = true;
+
+  model()->AddExtensionControlledTURL(std::move(ext_dse),
+                                      std::move(extension_info));
+
+  // Add another non replaceable user engine with same keyword as extension.
+  const TemplateURL* user2 =
+      model()->Add(base::MakeUnique<TemplateURL>(*turl_data));
+  // Wait for any saves to finish.
+  base::RunLoop().RunUntilIdle();
+
+  // Check extension DSE is set as default and its keyword is not changed.
+  EXPECT_TRUE(model()->IsExtensionControlledDefaultSearch());
+  auto current_dse = model()->GetDefaultSearchProvider();
+  ExpectSimilar(turl_data.get(), &current_dse->data());
+  EXPECT_EQ(current_dse,
+            model()->FindTemplateURLForExtension(
+                "ext2", TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION));
+
+  // Expect that both user engine keywords are changed because of conflict
+  // with extension engine keyword.
+  EXPECT_NE(turl_data->keyword(), user1->keyword());
+  EXPECT_NE(turl_data->keyword(), user2->keyword());
+  // Check that user engine keywords are different.
+  EXPECT_NE(user1->keyword(), user2->keyword());
+}
+
+// Check that two extensions with same engine handled corrected.

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

Nit: handled corrected -> are handled correctly

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

Done, thanks.

+TEST_F(TemplateURLServiceTest, ExtensionsWithSameKeywords) {
+  test_util()->VerifyLoad();
+
+  // TemplateURLData used for all extension.

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

Nit: extension -> extensions

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

Done.

+  auto turl_data = GenerateDummyTemplateURLData("extension");
+  turl_data->safe_for_autoreplace = false;
+
+  // Add non default extension engine.
+  auto ext_dse = base::MakeUnique<TemplateURL>(
+      *turl_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  auto extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("extension_id1");
+  extension_info->wants_to_be_default_engine = false;
+
+  auto extension1 = model()->AddExtensionControlledTURL(

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

Nit: This should not use auto; it's a raw pointer (so it'd be auto*), and the
type of the object is not object from the function call, so specify the type
explicitly. (2 places)

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

Done.

+      std::move(ext_dse), std::move(extension_info));
+
+  // Add default extension engine.
+  ext_dse = base::MakeUnique<TemplateURL>(
+      *turl_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("extension_id2");
+  extension_info->wants_to_be_default_engine = true;
+
+  model()->AddExtensionControlledTURL(std::move(ext_dse),

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

Nit: Shouldn't this store the result in a temp |extension_2|, and we check that
that's DSE below?  (Possibly this would obviate the need for some of the
existing checks below like the IsExtensionCOntrolledDefaultSearch() and
FindTemplateURLForExtension() calls)

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

Done.

+                                      std::move(extension_info));
+
+  // Add another non default extension with same keyword. This action must not
+  // change default extension keyword due to conflict.
+  ext_dse = base::MakeUnique<TemplateURL>(
+      *turl_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("extension_id3");
+  extension_info->wants_to_be_default_engine = false;
+
+  auto extension3 = model()->AddExtensionControlledTURL(
+      std::move(ext_dse), std::move(extension_info));
+  // Wait for any saves to finish.

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

Nit: Blank line above this

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

Done

+  base::RunLoop().RunUntilIdle();
+
+  // Check that extension2 is set as default. Its keyword must not be changed.
+  auto current_dse = model()->GetDefaultSearchProvider();
+  EXPECT_TRUE(model()->IsExtensionControlledDefaultSearch());
+  ExpectSimilar(turl_data.get(), &current_dse->data());
+  EXPECT_EQ(current_dse,
+            model()->FindTemplateURLForExtension(
+                "extension_id2", TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION));
+
+  // Check that non default extensions keywords are updated.
+  EXPECT_NE(extension1->keyword(), turl_data->keyword());
+  EXPECT_NE(extension3->keyword(), turl_data->keyword());
+  EXPECT_NE(extension1->keyword(), extension3->keyword());
+}
+
 TEST_F(TemplateURLServiceTest, ExtensionEngineVsPolicy) {
   // Set a managed preference that establishes a default search provider.
   std::unique_ptr<TemplateURLData> managed = CreateTestSearchEngine();