Make extensions DSE persistent in browser prefs (Reland)

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 72 matching lines @@
   TemplateURL* t_url = model->Add(CreateKeywordWithDate(
       model, short_name, keyword, url, suggest_url, alternate_url, favicon_url,
       safe_for_autoreplace, 0, encodings, date_created, last_modified,
       last_visited));
   EXPECT_NE(0, t_url->id());
   return t_url;
 }
 
 // Checks that the two TemplateURLs are similar. It does not check the id or
 // any time-related fields. Neither pointer should be NULL.
-void ExpectSimilar(const TemplateURL* expected, const TemplateURL* actual) {
+void ExpectSimilar(const TemplateURL* expected,
+                   const TemplateURL* actual,
+                   bool ignore_keyword = false) {
   ASSERT_TRUE(expected != NULL);
   ASSERT_TRUE(actual != NULL);
   EXPECT_EQ(expected->short_name(), actual->short_name());
-  EXPECT_EQ(expected->keyword(), actual->keyword());
+  if (!ignore_keyword) {

[Peter Kasting, 2017/01/25 22:31:23]

Nit: No {}

[Alexander Yashkin, 2017/01/26 07:51:01]

Done

+    EXPECT_EQ(expected->keyword(), actual->keyword());
+  }
   EXPECT_EQ(expected->url(), actual->url());
   EXPECT_EQ(expected->suggestions_url(), actual->suggestions_url());
   EXPECT_EQ(expected->favicon_url(), actual->favicon_url());
   EXPECT_EQ(expected->alternate_urls(), actual->alternate_urls());
   EXPECT_EQ(expected->safe_for_autoreplace(), actual->safe_for_autoreplace());
   EXPECT_EQ(expected->input_encodings(), actual->input_encodings());
   EXPECT_EQ(expected->search_terms_replacement_key(),
             actual->search_terms_replacement_key());
 }
 
@@ skipping 1289 matching lines @@
   EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());
 
   std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(
       model(), "ext", "ext", "http://www.search.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(), true, kPrepopulatedId,
       "UTF-8", Time(), Time(), Time(),
       TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
   std::unique_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(
       new TemplateURL::AssociatedExtensionInfo("ext"));
   extension_info->wants_to_be_default_engine = true;
-  TemplateURL* ext_dse_ptr = model()->AddExtensionControlledTURL(
+  TemplateURL* ext_dse_ptr = test_util()->AddExtensionControlledTURL(
       std::move(ext_dse), std::move(extension_info));
   EXPECT_EQ(ext_dse_ptr, model()->GetDefaultSearchProvider());
 
-  model()->RemoveExtensionControlledTURL(
-      "ext", TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  test_util()->RemoveExtensionControlledTURL("ext");
   ExpectSimilar(user_dse, model()->GetDefaultSearchProvider());
 }
 
-TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
+TEST_F(TemplateURLServiceTest, DefaultExtensionEnginePersist) {
   test_util()->VerifyLoad();
   // Add third-party default search engine.
   TemplateURL* user_dse = AddKeywordWithDate(
       "user", "user", "http://www.goo.com/s?q={searchTerms}", std::string(),
       std::string(), std::string(), true, "UTF-8", Time(), Time(), Time());
   model()->SetUserSelectedDefaultSearchProvider(user_dse);
   EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());
 
+  // Create non-default extension search engine.
   std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(
       model(), "ext1", "ext1", "http://www.ext1.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(), true, 0, "UTF-8", Time(),
       Time(), Time(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
   std::unique_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(
       new TemplateURL::AssociatedExtensionInfo("ext1"));
   extension_info->wants_to_be_default_engine = false;
-  model()->AddExtensionControlledTURL(std::move(ext_dse),
-                                      std::move(extension_info));
+
+  test_util()->AddExtensionControlledTURL(std::move(ext_dse),
+                                          std::move(extension_info));
   EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());
 
+  // Create default extension search engine.
   ext_dse = CreateKeywordWithDate(
       model(), "ext2", "ext2", "http://www.ext2.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(), true, kPrepopulatedId,
       "UTF-8", Time(), Time(), Time(),
       TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
   extension_info.reset(new TemplateURL::AssociatedExtensionInfo("ext2"));
   extension_info->wants_to_be_default_engine = true;
-  TemplateURL* ext_dse_ptr = model()->AddExtensionControlledTURL(
+
+  auto cloned_ext_dse = base::MakeUnique<TemplateURL>(ext_dse->data());
+  TemplateURL* ext_dse_ptr = test_util()->AddExtensionControlledTURL(
       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());
+  // A default search engine set by an extension must be persisted across
+  // browser restarts, until the extension is unloaded/disabled.
+  test_util()->ResetModel(false);
+  EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider());
+
+  // Non-default extension engines are not persisted across restarts.
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
-  EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+}
+
+TEST_F(TemplateURLServiceTest, DefaultExtensionEnginePersistsBeforeLoad) {
+  // Chrome will load the extension system before the TemplateURLService, so
+  // extensions controlling the default search engine may be registered before
+  // the service has loaded.
+  std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(
+      model(), "ext2", "ext2", "http://www.ext2.com/s?q={searchTerms}",
+      std::string(), std::string(), std::string(), true, kPrepopulatedId,
+      "UTF-8", Time(), Time(), Time(),
+      TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  auto extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("ext2");
+  extension_info->wants_to_be_default_engine = true;
+
+  auto cloned_ext_dse = base::MakeUnique<TemplateURL>(ext_dse->data());
+  test_util()->AddExtensionControlledTURL(std::move(ext_dse),
+                                          std::move(extension_info));
+
+  // Default search engine from extension must be persisted between browser
+  // restarts, and should be available before the TemplateURLService is loaded.
+  EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider());
+
+  // Check extension DSE is the same after service load.
+  test_util()->VerifyLoad();
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider());
+}
+
+// Check that extension engine and prepopulated engine with same keywords are
+// handled corrected.
+TEST_F(TemplateURLServiceTest, DefaultExtensionEngineAndPrepopulated) {
+  // Add default extension engine.
+  auto ext_data = GenerateDummyTemplateURLData("ext2");
+  ext_data->safe_for_autoreplace = false;
+  auto ext_dse = base::MakeUnique<TemplateURL>(
+      *ext_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  auto extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("ext2");
+  extension_info->wants_to_be_default_engine = true;
+
+  auto cloned_ext_dse = base::MakeUnique<TemplateURL>(ext_dse->data());
+  test_util()->AddExtensionControlledTURL(std::move(ext_dse),
+                                          std::move(extension_info));
+
+  // Add prepopulated engine with same keyword as extension.
+  model()->Add(
+      base::MakeUnique<TemplateURL>(*GenerateDummyTemplateURLData("ext2")));
+  // Wait for any saves to finish.
+  base::RunLoop().RunUntilIdle();
+
+  test_util()->VerifyLoad();
+  // Check extension DSE is set as default. Due to keywords conflict with
+  // prepopulated engine its keyword changed.
+  EXPECT_TRUE(model()->IsExtensionControlledDefaultSearch());
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider(),
+                true);

[Peter Kasting, 2017/01/25 22:31:23]

Should we be checking as to which engine's keyword got changed?

[Alexander Yashkin, 2017/01/26 07:51:01]

Done

+}
+
+// Check that two extensions with same engine handled corrected.

[Peter Kasting, 2017/01/25 22:31:23]

What parts of the engines being "the same" are relevant here?  The keyword?  The
search URL?  The fact that literally every piece of data from the two engines is
the same?

[Alexander Yashkin, 2017/01/26 07:51:01]

The keyword conflict is relevant. Changed comments and code to highlight this.

+TEST_F(TemplateURLServiceTest, TwoExtensionsWithSameEngine) {
+  // Add default extension engine.
+  auto ext_data = GenerateDummyTemplateURLData("extension");
+  ext_data->safe_for_autoreplace = false;
+  auto ext_dse = base::MakeUnique<TemplateURL>(
+      *ext_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  auto extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("extension_id1");
+  extension_info->wants_to_be_default_engine = true;
+
+  auto cloned_ext_dse = base::MakeUnique<TemplateURL>(ext_dse->data());
+  test_util()->AddExtensionControlledTURL(std::move(ext_dse),
+                                          std::move(extension_info));
+
+  // Add non default extension with same engine. This action will change
+  // previous extension keyword due to conflict.
+  ext_data = GenerateDummyTemplateURLData("extension");
+  ext_data->safe_for_autoreplace = false;
+  ext_dse = base::MakeUnique<TemplateURL>(
+      *ext_data, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  extension_info =
+      base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("extension_id2");
+  extension_info->wants_to_be_default_engine = false;
+
+  test_util()->AddExtensionControlledTURL(std::move(ext_dse),
+                                          std::move(extension_info));
+  // Wait for any saves to finish.
+  base::RunLoop().RunUntilIdle();
+
+  test_util()->VerifyLoad();
+  // Check that extension DSE is set as default. Due to keywords conflict with
+  // prepopulated engine its keyword changed.
+  auto current_dse = model()->GetDefaultSearchProvider();
+  EXPECT_TRUE(model()->IsExtensionControlledDefaultSearch());
+  ExpectSimilar(cloned_ext_dse.get(), current_dse, true);
+  EXPECT_EQ("extension_id1",
+            current_dse->GetExtensionInfoForTesting()->extension_id);
 }
 
 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();
   ExpectSimilar(expected_managed_default.get(), actual_managed_default);
 
   std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(
       model(), "ext1", "ext1", "http://www.ext1.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(), true, kPrepopulatedId,
       "UTF-8", Time(), Time(), Time(),
       TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
   auto extension_info =
       base::MakeUnique<TemplateURL::AssociatedExtensionInfo>("ext1");
   extension_info->wants_to_be_default_engine = true;
-  TemplateURL* ext_dse_ptr = model()->AddExtensionControlledTURL(
+  TemplateURL* ext_dse_ptr = test_util()->AddExtensionControlledTURL(
       std::move(ext_dse), std::move(extension_info));
   EXPECT_EQ(ext_dse_ptr,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
   EXPECT_TRUE(model()->is_default_search_managed());
   actual_managed_default = model()->GetDefaultSearchProvider();
   ExpectSimilar(expected_managed_default.get(), actual_managed_default);
 }
 
 TEST_F(TemplateURLServiceTest, LastVisitedTimeUpdate) {
   test_util()->VerifyLoad();
@@ skipping 27 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);
 }