Make extensions DSE persistent in browser prefs

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 ff7932f17c3295c3f4fce1b0586d1071efefccb0..79b7efec53508ede1cf991c05f177d1c9612efa7 100644
--- a/chrome/browser/search_engines/template_url_service_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_unittest.cc
@@ -1389,16 +1389,15 @@ TEST_F(TemplateURLServiceTest, DefaultExtensionEngine) {
   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(
@@ -1408,6 +1407,7 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
   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(),
@@ -1415,25 +1415,53 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
   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));
-  EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());
 
+  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(), 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(
+      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 before template url service loaded.
+  EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext2")));
+  TemplateURL* current_dse = model()->GetDefaultSearchProvider();
+  ExpectSimilar(cloned_ext_dse.get(), current_dse);
+
+  // Load template url service, but extensions are not loaded and not registered
+  // yet. Still extension default search.
+  test_util()->VerifyLoad();
+  current_dse = model()->GetDefaultSearchProvider();
+  ExpectSimilar(cloned_ext_dse.get(), current_dse);
+
+  // Emulate extension system loaded and extension registers itself to template
+  // url service, still same extension engine is default.

[vasilii, 2016/12/06 19:16:42]

Can you explain why this is true?

[Alexander Yashkin, 2016/12/09 08:19:52]

Thats how I think it should work.

How TemplateURLService initializes(Here DSE stands for default search engine):
1. After creation, but before TemplateURLService load is completed it reads
initial DSE from preferences. And after previous call to
test_util()->AddExtensionControlledTURL initial DSE is provided by extension
overriden DSE pref.
2. After TemplateURLService is loaded but before extension subsystem is loaded,
TemplateURLService creates current default TemplateUrl to use from initial DSE.
This is done by ApplyDefaultSearchChangeNoMetrics called from
ChangeToLoadedState function. Default TemplateUrl is created from initial and is
still extension controlled one.
3. After extensions are loaded, those that override default search override DSE
in prefs and registers themselves in TemplateUrlService. 
In browser its done from SettingsOverridesAPI::OnExtensionLoaded calling
RegisterSearchProvider. Here in test I just call
test_util()->AddExtensionControlledTURL manually to emulate such behaviour.

Or is the question is about something else?

[vasilii, 2016/12/13 17:16:39]

Acknowledged.

+  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(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  extension_info.reset(new TemplateURL::AssociatedExtensionInfo("ext2"));
+  extension_info->wants_to_be_default_engine = true;
+
+  test_util()->AddExtensionControlledTURL(std::move(ext_dse),
+                                          std::move(extension_info));
+  current_dse = model()->GetDefaultSearchProvider();
+  ExpectSimilar(cloned_ext_dse.get(), current_dse);
 }
 
 TEST_F(TemplateURLServiceTest, ExtensionEngineVsPolicy) {
@@ -1455,7 +1483,7 @@ TEST_F(TemplateURLServiceTest, ExtensionEngineVsPolicy) {
   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")));