Stop determining the type of a TemplateURL by the URL scheme

Omnibox API uses URLs with chrome-extension scheme to force TemplateURL::GetType() to return the desired value.
OTOH, SettingsOverridesAPI uses AssociatedExtensionInfo to do the same thing.
Use AssociatedExtensionInfo as the only way to inject extension related information to TemplateURL to eliminate this redundancy.

TemplateURL uses AssociatedExtensionInfo for both OmniboxAPI and SettingsOverridesAPI.
Fix TemplateURLService::RegisterOmniboxKeyword() as a wrapper of AddExtensionControlledTURL().
Merge TemplateURLService::UnregisterOmniboxKeyword() to RemoveExtensionControlledTURL().
Move the code inside of TemplateURLService::CreateTemplateURLForExtension() to OmniboxAPI.
No #include to src/extensions from chrome/browser/search_engines.

BUG=388040, 388338
TEST=git cl try

Review URL: https://codereview.chromium.org/346843008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280291 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 88 insertions, 132 deletions

diff --git a/chrome/browser/search_engines/DEPS b/chrome/browser/search_engines/DEPS
index ec828c3..d9a3ea8 100644
--- a/chrome/browser/search_engines/DEPS
+++ b/chrome/browser/search_engines/DEPS
@@ -1,3 +1,7 @@
+include_rules = [
+  "-extensions",
+]
+
 specific_include_rules = {
   "search_host_to_urls_map(_unittest)?\.": [
     "-chrome",
diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc
index b0c1b09..6b8967f 100644
--- a/chrome/browser/search_engines/template_url_service.cc
+++ b/chrome/browser/search_engines/template_url_service.cc
@@ -40,7 +40,6 @@
 #include "components/search_engines/template_url_prepopulate_data.h"
 #include "components/url_fixer/url_fixer.h"
 #include "content/public/browser/notification_service.h"
-#include "extensions/common/constants.h"
 #include "net/base/net_util.h"
 #include "sync/api/sync_change.h"
 #include "sync/api/sync_error_factory.h"
@@ -188,20 +187,6 @@ void LogDuplicatesHistogram(
 }  // namespace
 
 
-// TemplateURLService::ExtensionKeyword ---------------------------------------
-
-TemplateURLService::ExtensionKeyword::ExtensionKeyword(
-    const std::string& id,
-    const std::string& name,
-    const std::string& keyword)
-    : extension_id(id),
-      extension_name(name),
-      extension_keyword(keyword) {
-}
-
-TemplateURLService::ExtensionKeyword::~ExtensionKeyword() {}
-
-
 // TemplateURLService::LessWithPrefix -----------------------------------------
 
 class TemplateURLService::LessWithPrefix {
@@ -597,17 +582,16 @@ void TemplateURLService::AddWithOverrides(TemplateURL* template_url,
 
 void TemplateURLService::AddExtensionControlledTURL(
     TemplateURL* template_url,
-    scoped_ptr<AssociatedExtensionInfo> info) {
+    scoped_ptr<TemplateURL::AssociatedExtensionInfo> info) {
   DCHECK(loaded_);
   DCHECK(template_url);
   DCHECK_EQ(kInvalidTemplateURLID, template_url->id());
   DCHECK(info);
+  DCHECK_NE(TemplateURL::NORMAL, info->type);
   DCHECK_EQ(info->wants_to_be_default_engine,
             template_url->show_in_default_list());
+  DCHECK(!FindTemplateURLForExtension(info->extension_id, info->type));
   template_url->extension_info_.swap(info);
-  DCHECK(!FindTemplateURLForExtension(
-      template_url->GetExtensionId(),
-      TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION));
 
   WebDataService::KeywordBatchModeScoper keyword_scoper(service_.get());
   if (AddNoNotify(template_url, true)) {
@@ -623,10 +607,10 @@ void TemplateURLService::Remove(TemplateURL* template_url) {
 }
 
 void TemplateURLService::RemoveExtensionControlledTURL(
-    const std::string& extension_id) {
+    const std::string& extension_id,
+    TemplateURL::Type type) {
   DCHECK(loaded_);
-  TemplateURL* url = FindTemplateURLForExtension(
-      extension_id, TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
+  TemplateURL* url = FindTemplateURLForExtension(extension_id, type);
   if (!url)
     return;
   // NULL this out so that we can call RemoveNoNotify.
@@ -673,27 +657,26 @@ void TemplateURLService::RemoveAutoGeneratedForOriginBetween(
     NotifyObservers();
 }
 
-
 void TemplateURLService::RegisterOmniboxKeyword(
-    const std::string& extension_id,
-    const std::string& extension_name,
-    const std::string& keyword) {
+     const std::string& extension_id,
+     const std::string& extension_name,
+     const std::string& keyword,
+     const std::string& template_url_string) {
   DCHECK(loaded_);
 
-  if (!FindTemplateURLForExtension(extension_id,
-                                   TemplateURL::OMNIBOX_API_EXTENSION)) {
-    ExtensionKeyword extension_url(extension_id, extension_name, keyword);
-    Add(CreateTemplateURLForExtension(extension_url));
-  }
-}
+  if (FindTemplateURLForExtension(extension_id,
+                                  TemplateURL::OMNIBOX_API_EXTENSION))
+    return;
 
-void TemplateURLService::UnregisterOmniboxKeyword(
-    const std::string& extension_id) {
-  DCHECK(loaded_);
-  TemplateURL* url = FindTemplateURLForExtension(
-      extension_id, TemplateURL::OMNIBOX_API_EXTENSION);
-  if (url)
-    Remove(url);
+  TemplateURLData data;
+  data.short_name = base::UTF8ToUTF16(extension_name);
+  data.SetKeyword(base::UTF8ToUTF16(keyword));
+  data.SetURL(template_url_string);
+  TemplateURL* url = new TemplateURL(data);
+  scoped_ptr<TemplateURL::AssociatedExtensionInfo> info(
+      new TemplateURL::AssociatedExtensionInfo(
+          TemplateURL::OMNIBOX_API_EXTENSION, extension_id));
+  AddExtensionControlledTURL(url, info.Pass());
 }
 
 TemplateURLService::TemplateURLVector TemplateURLService::GetTemplateURLs() {
@@ -2420,18 +2403,6 @@ TemplateURL* TemplateURLService::FindPrepopulatedTemplateURL(
   return NULL;
 }
 
-TemplateURL* TemplateURLService::CreateTemplateURLForExtension(
-    const ExtensionKeyword& extension_keyword) {
-  TemplateURLData data;
-  data.short_name = base::UTF8ToUTF16(extension_keyword.extension_name);
-  data.SetKeyword(base::UTF8ToUTF16(extension_keyword.extension_keyword));
-  // This URL is not actually used for navigation. It holds the extension's
-  // ID, as well as forcing the TemplateURL to be treated as a search keyword.
-  data.SetURL(std::string(extensions::kExtensionScheme) + "://" +
-      extension_keyword.extension_id + "/?q={searchTerms}");
-  return new TemplateURL(data);
-}
-
 TemplateURL* TemplateURLService::FindTemplateURLForExtension(
     const std::string& extension_id,
     TemplateURL::Type type) {
diff --git a/chrome/browser/search_engines/template_url_service.h b/chrome/browser/search_engines/template_url_service.h
index 86415fd..a6e9948 100644
--- a/chrome/browser/search_engines/template_url_service.h
+++ b/chrome/browser/search_engines/template_url_service.h
@@ -85,18 +85,6 @@ class TemplateURLService : public WebDataServiceConsumer,
     const char* const content;
   };
 
-  // Struct describes a search engine added by an extension.
-  struct ExtensionKeyword {
-    ExtensionKeyword(const std::string& id,
-                     const std::string& name,
-                     const std::string& keyword);
-    ~ExtensionKeyword();
-
-    std::string extension_id;
-    std::string extension_name;
-    std::string extension_keyword;
-  };
-
   explicit TemplateURLService(Profile* profile);
   // The following is for testing.
   TemplateURLService(const Initializer* initializers, const int count);
@@ -167,18 +155,20 @@ class TemplateURLService : public WebDataServiceConsumer,
                         const base::string16& keyword,
                         const std::string& url);
 
-  // Add the search engine of type NORMAL_CONTROLLED_BY_EXTENSION.
-  void AddExtensionControlledTURL(TemplateURL* template_url,
-                                  scoped_ptr<AssociatedExtensionInfo> info);
+  // Adds a search engine with the specified info.
+  void AddExtensionControlledTURL(
+      TemplateURL* template_url,
+      scoped_ptr<TemplateURL::AssociatedExtensionInfo> info);
 
   // Removes the keyword from the model. This deletes the supplied TemplateURL.
   // This fails if the supplied template_url is the default search provider.
   void Remove(TemplateURL* template_url);
 
-  // Removes any TemplateURL of type NORMAL_CONTROLLED_BY_EXTENSION associated
-  // with |extension_id|. Unlike with Remove(), this can be called when the
+  // Removes any TemplateURL of the specified |type| associated with
+  // |extension_id|. Unlike with Remove(), this can be called when the
   // TemplateURL in question is the current default search provider.
-  void RemoveExtensionControlledTURL(const std::string& extension_id);
+  void RemoveExtensionControlledTURL(const std::string& extension_id,
+                                     TemplateURL::Type type);
 
   // Removes all auto-generated keywords that were created on or after the
   // date passed in.
@@ -201,11 +191,8 @@ class TemplateURLService : public WebDataServiceConsumer,
   // already exists for this extension, does nothing.
   void RegisterOmniboxKeyword(const std::string& extension_id,
                               const std::string& extension_name,
-                              const std::string& keyword);
-
-  // Removes the TemplateURL containing the keyword for the extension with the
-  // given ID, if any.
-  void UnregisterOmniboxKeyword(const std::string& extension_id);
+                              const std::string& keyword,
+                              const std::string& template_url_string);
 
   // Returns the set of URLs describing the keywords. The elements are owned
   // by TemplateURLService and should not be deleted.
@@ -647,10 +634,6 @@ class TemplateURLService : public WebDataServiceConsumer,
   // |template_urls| on exit.
   void AddTemplateURLs(TemplateURLVector* template_urls);
 
-  // Returns a new TemplateURL for the given extension.
-  TemplateURL* CreateTemplateURLForExtension(
-      const ExtensionKeyword& extension_keyword);
-
   // Returns the TemplateURL corresponding to |prepopulated_id|, if any.
   TemplateURL* FindPrepopulatedTemplateURL(int prepopulated_id);
 
diff --git a/chrome/browser/search_engines/template_url_service_sync_unittest.cc b/chrome/browser/search_engines/template_url_service_sync_unittest.cc
index e24ebaa..a00ed6e 100644
--- a/chrome/browser/search_engines/template_url_service_sync_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_sync_unittest.cc
@@ -17,7 +17,6 @@
 #include "components/search_engines/search_engines_pref_names.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_prepopulate_data.h"
-#include "extensions/common/constants.h"
 #include "net/base/net_util.h"
 #include "sync/api/sync_change_processor_wrapper_for_test.h"
 #include "sync/api/sync_error_factory.h"
@@ -167,7 +166,7 @@ class TemplateURLServiceSyncTest : public testing::Test {
   scoped_ptr<syncer::SyncChangeProcessor> PassProcessor();
   scoped_ptr<syncer::SyncErrorFactory> CreateAndPassSyncErrorFactory();
 
-  // Create a TemplateURL with some test values. The caller owns the returned
+  // Creates a TemplateURL with some test values. The caller owns the returned
   // TemplateURL*.
   TemplateURL* CreateTestTemplateURL(const base::string16& keyword,
                                      const std::string& url,
@@ -394,8 +393,8 @@ TEST_F(TemplateURLServiceSyncTest, GetAllSyncDataBasic) {
 TEST_F(TemplateURLServiceSyncTest, GetAllSyncDataWithExtension) {
   model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com"));
   model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key2"), "http://key2.com"));
-  model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key3"),
-      std::string(extensions::kExtensionScheme) + "://blahblahblah"));
+  model()->RegisterOmniboxKeyword("blahblahblah", "unittest", "key3",
+                                  "http://blahblahblah");
   syncer::SyncDataList all_sync_data =
       model()->GetAllSyncData(syncer::SEARCH_ENGINES);
 
@@ -1060,13 +1059,18 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithLocalExtensions) {
                                     CreateAndPassSyncErrorFactory());
 
   // Add some extension keywords locally.
-  TemplateURL* extension1 = CreateTestTemplateURL(ASCIIToUTF16("keyword1"),
-      std::string(extensions::kExtensionScheme) + "://extension1");
-  model()->Add(extension1);
+  model()->RegisterOmniboxKeyword("extension1", "unittest", "keyword1",
+                                  "http://extension1");
+  TemplateURL* extension1 =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword1"));
+  ASSERT_TRUE(extension1);
   EXPECT_EQ(1U, processor()->change_list_size());
-  TemplateURL* extension2 = CreateTestTemplateURL(ASCIIToUTF16("keyword2"),
-      std::string(extensions::kExtensionScheme) + "://extension2");
-  model()->Add(extension2);
+
+  model()->RegisterOmniboxKeyword("extension2", "unittest", "keyword2",
+                                  "http://extension2");
+  TemplateURL* extension2 =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword2"));
+  ASSERT_TRUE(extension2);
   EXPECT_EQ(1U, processor()->change_list_size());
 
   // Create some sync changes that will conflict with the extension keywords.
diff --git a/chrome/browser/search_engines/template_url_service_unittest.cc b/chrome/browser/search_engines/template_url_service_unittest.cc
index e82fd85..a5189a9 100644
--- a/chrome/browser/search_engines/template_url_service_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_unittest.cc
@@ -27,9 +27,6 @@
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_prepopulate_data.h"
 #include "components/webdata/common/web_database.h"
-#include "extensions/common/constants.h"
-#include "extensions/common/extension.h"
-#include "extensions/common/manifest_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ASCIIToUTF16;
@@ -428,40 +425,34 @@ TEST_F(TemplateURLServiceTest, AddExtensionKeyword) {
   TemplateURL* original2 = AddKeywordWithDate(
       "nonreplaceable", "keyword2", "http://test2", std::string(),
       std::string(), std::string(), false, "UTF-8", Time(), Time());
-  TemplateURL* original3 = AddKeywordWithDate(
-      "extension", "keyword3",
-      std::string(extensions::kExtensionScheme) + "://test3", std::string(),
-      std::string(), std::string(), false, "UTF-8", Time(), Time());
+  model()->RegisterOmniboxKeyword("test3", "extension", "keyword3",
+                                  "http://test3");
+  TemplateURL* original3 =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3"));
+  ASSERT_TRUE(original3);
 
   // Add an extension keyword that conflicts with each of the above three
   // keywords.
-  TemplateURLData data;
-  data.short_name = ASCIIToUTF16("test");
-  data.SetKeyword(ASCIIToUTF16("keyword1"));
-  data.SetURL(std::string(extensions::kExtensionScheme) + "://test4");
-  data.safe_for_autoreplace = false;
-
   // Both replaceable and non-replaceable keywords should be uniquified.
-  TemplateURL* extension1 = new TemplateURL(data);
-  model()->Add(extension1);
-  ASSERT_EQ(extension1,
-            model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword1")));
+  model()->RegisterOmniboxKeyword("test4", "test", "keyword1", "http://test4");
+  TemplateURL* extension1 =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword1"));
+  ASSERT_TRUE(extension1);
   EXPECT_EQ(original1,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("test1")));
-  data.SetKeyword(ASCIIToUTF16("keyword2"));
-  TemplateURL* extension2 = new TemplateURL(data);
-  model()->Add(extension2);
-  ASSERT_EQ(extension2,
-            model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword2")));
+
+  model()->RegisterOmniboxKeyword("test5", "test", "keyword2", "http://test5");
+  TemplateURL* extension2 =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword2"));
+  ASSERT_TRUE(extension2);
   EXPECT_EQ(original2,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("test2")));
 
   // They should override extension keywords added earlier.
-  data.SetKeyword(ASCIIToUTF16("keyword3"));
-  TemplateURL* extension3 = new TemplateURL(data);
-  model()->Add(extension3);
-  ASSERT_EQ(extension3,
-            model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3")));
+  model()->RegisterOmniboxKeyword("test6", "test", "keyword3", "http://test6");
+  TemplateURL* extension3 =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3"));
+  ASSERT_TRUE(extension3);
   EXPECT_EQ(original3,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3_")));
 }
@@ -472,10 +463,11 @@ TEST_F(TemplateURLServiceTest, AddSameKeywordWithExtensionPresent) {
   // Similar to the AddSameKeyword test, but with an extension keyword masking a
   // replaceable TemplateURL.  We should still do correct conflict resolution
   // between the non-template URLs.
-  TemplateURL* extension = AddKeywordWithDate(
-      "extension", "keyword",
-      std::string(extensions::kExtensionScheme) + "://test2", std::string(),
-      std::string(), std::string(), false, "UTF-8", Time(), Time());
+  model()->RegisterOmniboxKeyword("test2", "extension", "keyword",
+                                  "http://test2");
+  TemplateURL* extension =
+      model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword"));
+  ASSERT_TRUE(extension);
   // Adding a keyword that matches the extension should cause the extension
   // to uniquify.
   AddKeywordWithDate(
@@ -816,7 +808,8 @@ TEST_F(TemplateURLServiceTest, RepairPrepopulatedSearchEngines) {
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("bing.com")));
 
   // Register an extension with bing keyword.
-  model()->RegisterOmniboxKeyword("abcdefg", "extension_name", "bing.com");
+  model()->RegisterOmniboxKeyword("abcdefg", "extension_name", "bing.com",
+                                  "http://abcdefg");
   EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("bing.com")));
 
   model()->RepairPrepopulatedSearchEngines();
@@ -1414,14 +1407,15 @@ TEST_F(TemplateURLServiceTest, DefaultExtensionEngine) {
       model(), "ext", "ext", "http://www.search.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(),
       true, true, "UTF-8", Time(), Time());
-  scoped_ptr<AssociatedExtensionInfo> extension_info(
-      new AssociatedExtensionInfo);
+  scoped_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(
+      new TemplateURL::AssociatedExtensionInfo(
+          TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION, "ext"));
   extension_info->wants_to_be_default_engine = true;
-  extension_info->extension_id = "ext";
   model()->AddExtensionControlledTURL(ext_dse, extension_info.Pass());
   EXPECT_EQ(ext_dse, model()->GetDefaultSearchProvider());
 
-  model()->RemoveExtensionControlledTURL("ext");
+  model()->RemoveExtensionControlledTURL(
+      "ext", TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
   ExpectSimilar(user_dse, model()->GetDefaultSearchProvider());
 }
 
@@ -1439,10 +1433,10 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
       model(), "ext1", "ext1", "http://www.ext1.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(),
       true, false, "UTF-8", Time(), Time());
-  scoped_ptr<AssociatedExtensionInfo> extension_info(
-      new AssociatedExtensionInfo);
+  scoped_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(
+      new TemplateURL::AssociatedExtensionInfo(
+          TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION, "ext1"));
   extension_info->wants_to_be_default_engine = false;
-  extension_info->extension_id = "ext1";
   model()->AddExtensionControlledTURL(ext_dse, extension_info.Pass());
   EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());
 
@@ -1450,9 +1444,9 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
       model(), "ext2", "ext2", "http://www.ext2.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(),
       true, true, "UTF-8", Time(), Time());
-  extension_info.reset(new AssociatedExtensionInfo);
+  extension_info.reset(new TemplateURL::AssociatedExtensionInfo(
+      TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION, "ext2"));
   extension_info->wants_to_be_default_engine = true;
-  extension_info->extension_id = "ext2";
   model()->AddExtensionControlledTURL(ext_dse, extension_info.Pass());
   EXPECT_EQ(ext_dse, model()->GetDefaultSearchProvider());
 
@@ -1496,10 +1490,10 @@ TEST_F(TemplateURLServiceTest, ExtensionEngineVsPolicy) {
       model(), "ext1", "ext1", "http://www.ext1.com/s?q={searchTerms}",
       std::string(), std::string(), std::string(),
       true, true, "UTF-8", Time(), Time());
-  scoped_ptr<AssociatedExtensionInfo> extension_info(
-      new AssociatedExtensionInfo);
+  scoped_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(
+      new TemplateURL::AssociatedExtensionInfo(
+          TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION, "ext1"));
   extension_info->wants_to_be_default_engine = true;
-  extension_info->extension_id = "ext1";
   model()->AddExtensionControlledTURL(ext_dse, extension_info.Pass());
   EXPECT_EQ(ext_dse, model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
   EXPECT_TRUE(model()->is_default_search_managed());