Changed keywords conflicts resolution for extensions search engines.

chrome/browser/search_engines/template_url_service_unittest.cc

 // Copyright (c) 2012 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 <stddef.h>
 
 #include <memory>
 #include <utility>
@@ skipping 10 matching lines @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "base/test/simple_test_clock.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/search_engines/template_url_service_test_util.h"
 #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"
 #include "components/search_engines/template_url_prepopulate_data.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ASCIIToUTF16;
 using base::Time;
 using base::TimeDelta;
@@ skipping 1433 matching lines @@
       std::move(ext_dse), std::move(extension_info));
   EXPECT_EQ(ext_dse_ptr, model()->GetDefaultSearchProvider());
 
   test_util()->ResetModel(true);
   user_dse = model()->GetTemplateURLForKeyword(ASCIIToUTF16("user"));
   ExpectSimilar(user_dse, model()->GetDefaultSearchProvider());
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
   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();
   SetManagedDefaultSearchPreferences(*managed, true, test_util()->profile());
   test_util()->VerifyLoad();
   // Verify that the default manager we are getting is the managed one.
   auto expected_managed_default = base::MakeUnique<TemplateURL>(*managed);
   EXPECT_TRUE(model()->is_default_search_managed());
   const TemplateURL* actual_managed_default =
       model()->GetDefaultSearchProvider();
@@ skipping 48 matching lines @@
   TemplateURL* update_url =
       model()->GetTemplateURLForKeyword(ASCIIToUTF16("engine_keyword"));
   const base::Time update_last_modified = update_url->last_modified();
   model()->SetUserSelectedDefaultSearchProvider(update_url);
   TemplateURL* reloaded_url =
       model()->GetTemplateURLForKeyword(ASCIIToUTF16("engine_keyword"));
   const base::Time reloaded_last_modified = reloaded_url->last_modified();
   EXPECT_NE(original_last_modified, reloaded_last_modified);
   EXPECT_EQ(update_last_modified, reloaded_last_modified);
 }