Make extensions DSE persistent in browser prefs

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 1387 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.

[Peter Kasting, 2016/12/22 20:49:03]

Nit: Blank line above this

[Alexander Yashkin, 2016/12/23 19:44:08]

Done

   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(
+
+  std::unique_ptr<TemplateURL> cloned_ext_dse(

[Peter Kasting, 2016/12/22 20:49:03]

Nit: Prefer = to () for this "copy-like" initialization; see
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Variab...
.

You're welcome to use "auto" here to avoid restarting the type, since
MakeUnique<TemplateURL> is pretty clear.

[Alexander Yashkin, 2016/12/23 19:44:08]

Done

+      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());
+  test_util()->ResetModel(false);
+  // Non default extension engines are not persisted.
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
-  EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+
+  // Default search engine from extension must be persisted between browser
+  // restarts, until extension is unloaded/disabled. Check it persists.

[Peter Kasting, 2016/12/22 20:49:03]

Nit: Last sentence unnecessary (restates the code)

[Alexander Yashkin, 2016/12/23 19:44:08]

Done

+  EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider());
+}
+
+TEST_F(TemplateURLServiceTest, DefaultExtensionEnginePersistsBeforeLoad) {
+  // Emulate extension system loaded and extension registers itself to template
+  // url service, this is always done before TemplateURLService is loaded.

[Peter Kasting, 2016/12/22 20:49:03]

Nit: The meaning of this is unclear.  Do you mean: "Chrome will load the
extension system before the TemplateURLService, so extensions controlling the
default search engine may be registered before the service has loaded."

[Alexander Yashkin, 2016/12/23 19:44:08]

Done, thanks.

+  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>("ext1");
+  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 is available before TemplateURLService load.
+  EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider());
+
+  // Load template url service.
+  test_util()->VerifyLoad();
+  // Check extension DSE is the same after service load.
+  ExpectSimilar(cloned_ext_dse.get(), model()->GetDefaultSearchProvider());
 }
 
 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);
 }