Move TemplateURLService related tests to components/search_engines

Put tests next to the code being tested.

Move tests to components/ without actually removing dependencies with temporary DEPS rules added.
This results in these tests still built as parts of unit_tests, not components_unittests.

Fix TemplateURLServiceTestUtil to simplify the code and reduce chrome dependencies:
- Create KeywordWebDataService instead of depending on WebDataServiceFactory.
- Remove TemplateURLService::SetKeywordSearchTermsForURL() and TestingTemplateURLService.  Instead, TestingTemplateURLServiceClient is responsible to implement SetKeywordSearchTermsForURL().
- Remove content::TestBrowserThreadBundle from test util.  Tests are responsible to set up one.  This enables TemplateURLServiceTestSyncTest to use multiple test util instances.

BUG=387985
TEST=unit_tests

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

Cr-Commit-Position: refs/heads/master@{#292597}

chrome/browser/search_engines/search_provider_install_data_unittest.cc

@@ -19,8 +19,9 @@
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_prepopulate_data.h"
 #include "components/search_engines/template_url_service.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/mock_render_process_host.h"
-#include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
@@ -122,7 +123,6 @@ class SearchProviderInstallDataTest : public testing::Test {
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
 
- protected:
   TemplateURL* AddNewTemplateURL(const std::string& url,
                                  const base::string16& keyword);
 
@@ -130,6 +130,11 @@ class SearchProviderInstallDataTest : public testing::Test {
   // |SearchProviderInstallData| to process the update.
   void SetGoogleBaseURLAndProcessOnIOThread(GURL base_url);
 
+  TemplateURLServiceTestUtil* util() { return &util_; }
+  SearchProviderInstallData* install_data() { return install_data_; }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;  // To set up BrowserThreads.
   TemplateURLServiceTestUtil util_;
 
   // Provides the search provider install state on the I/O thread. It must be
@@ -200,7 +205,7 @@ void SearchProviderInstallDataTest::SetGoogleBaseURLAndProcessOnIOThread(
 
 TEST_F(SearchProviderInstallDataTest, GetInstallState) {
   // Set up the database.
-  util_.ChangeModelToLoadState();
+  util()->ChangeModelToLoadState();
   std::string host = "www.unittest.com";
   AddNewTemplateURL("http://" + host + "/path", base::ASCIIToUTF16("unittest"));
 
@@ -208,7 +213,7 @@ TEST_F(SearchProviderInstallDataTest, GetInstallState) {
   base::RunLoop().RunUntilIdle();
 
   // Verify the search providers install state (with no default set).
-  TestGetInstallState test_get_install_state(install_data_);
+  TestGetInstallState test_get_install_state(install_data());
   test_get_install_state.RunTests(host, std::string());
 
   // Set-up a default and try it all one more time.
@@ -216,19 +221,19 @@ TEST_F(SearchProviderInstallDataTest, GetInstallState) {
   TemplateURL* default_url =
       AddNewTemplateURL("http://" + default_host + "/",
                         base::ASCIIToUTF16("mmm"));
-  util_.model()->SetUserSelectedDefaultSearchProvider(default_url);
+  util()->model()->SetUserSelectedDefaultSearchProvider(default_url);
   test_get_install_state.RunTests(host, default_host);
 }
 
 TEST_F(SearchProviderInstallDataTest, ManagedDefaultSearch) {
   // Set up the database.
-  util_.ChangeModelToLoadState();
+  util()->ChangeModelToLoadState();
   std::string host = "www.unittest.com";
   AddNewTemplateURL("http://" + host + "/path", base::ASCIIToUTF16("unittest"));
 
   // Set a managed preference that establishes a default search provider.
   std::string host2 = "www.managedtest.com";
-  util_.SetManagedDefaultSearchPreferences(
+  util()->SetManagedDefaultSearchPreferences(
       true,
       "managed",
       "managed",
@@ -239,22 +244,22 @@ TEST_F(SearchProviderInstallDataTest, ManagedDefaultSearch) {
       std::string(),
       std::string());
 
-  EXPECT_TRUE(util_.model()->is_default_search_managed());
+  EXPECT_TRUE(util()->model()->is_default_search_managed());
 
   // Wait for the changes to be saved.
   base::RunLoop().RunUntilIdle();
 
   // Verify the search providers install state.  The default search should be
   // the managed one we previously set.
-  TestGetInstallState test_get_install_state(install_data_);
+  TestGetInstallState test_get_install_state(install_data());
   test_get_install_state.RunTests(host, host2);
 }
 
 TEST_F(SearchProviderInstallDataTest, GoogleBaseUrlChange) {
-  TestGetInstallState test_get_install_state(install_data_);
+  TestGetInstallState test_get_install_state(install_data());
 
   // Set up the database.
-  util_.ChangeModelToLoadState();
+  util()->ChangeModelToLoadState();
   std::string google_host = "w.com";
   SetGoogleBaseURLAndProcessOnIOThread(GURL("http://" + google_host + "/"));
 
@@ -262,7 +267,7 @@ TEST_F(SearchProviderInstallDataTest, GoogleBaseUrlChange) {
                     base::ASCIIToUTF16("t"));
   TemplateURL* default_url =
       AddNewTemplateURL("http://d.com/", base::ASCIIToUTF16("d"));
-  util_.model()->SetUserSelectedDefaultSearchProvider(default_url);
+  util()->model()->SetUserSelectedDefaultSearchProvider(default_url);
 
   // Wait for the changes to be saved.
   base::RunLoop().RunUntilIdle();

chrome/browser/search_engines/template_url_service_test_util.cc

@@ -4,72 +4,64 @@
 
 #include "chrome/browser/search_engines/template_url_service_test_util.h"
 
-#include "base/bind.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
-#include "base/strings/string_split.h"
-#include "base/threading/thread.h"
 #include "chrome/browser/search_engines/chrome_template_url_service_client.h"
-#include "chrome/browser/webdata/web_data_service_factory.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
-#include "components/google/core/browser/google_url_tracker.h"
-#include "components/search_engines/default_search_manager.h"
 #include "components/search_engines/default_search_pref_test_util.h"
+#include "components/search_engines/keyword_table.h"
+#include "components/search_engines/keyword_web_data_service.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/search_engines/testing_search_terms_data.h"
+#include "components/webdata/common/web_database_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// Trivial subclass of TemplateURLService that records the last invocation of
-// SetKeywordSearchTermsForURL.
-class TestingTemplateURLService : public TemplateURLService {
- public:
-  TestingTemplateURLService(Profile* profile,
-                            scoped_ptr<SearchTermsData> search_terms_data)
-      : TemplateURLService(
-            profile->GetPrefs(),
-            search_terms_data.Pass(),
-            WebDataServiceFactory::GetKeywordWebDataForProfile(
-                profile, Profile::EXPLICIT_ACCESS),
-            scoped_ptr<TemplateURLServiceClient>(
-                new ChromeTemplateURLServiceClient(profile)), NULL, NULL,
-            base::Closure()) {
-  }
+namespace {
 
-  base::string16 GetAndClearSearchTerm() {
-    base::string16 search_term;
-    search_term.swap(search_term_);
-    return search_term;
-  }
+class TestingTemplateURLServiceClient : public ChromeTemplateURLServiceClient {
+ public:
+  TestingTemplateURLServiceClient(Profile* profile,
+                                  base::string16* search_term)
+      : ChromeTemplateURLServiceClient(profile),
+        search_term_(search_term) {}
 
- protected:
   virtual void SetKeywordSearchTermsForURL(
-      const TemplateURL* t_url,
       const GURL& url,
+      TemplateURLID id,
       const base::string16& term) OVERRIDE {
-    search_term_ = term;
+    *search_term_ = term;
   }
 
  private:
-  base::string16 search_term_;
+  base::string16* search_term_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestingTemplateURLService);
+  DISALLOW_COPY_AND_ASSIGN(TestingTemplateURLServiceClient);
 };
 
+}  // namespace
 
 TemplateURLServiceTestUtil::TemplateURLServiceTestUtil()
-    : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
-      changed_count_(0),
+    : changed_count_(0),
       search_terms_data_(NULL) {
   // Make unique temp directory.
   EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
   profile_.reset(new TestingProfile(temp_dir_.path()));
 
-  profile()->CreateWebDataService();
+  scoped_refptr<WebDatabaseService> web_database_service =
+      new WebDatabaseService(temp_dir_.path().AppendASCII("webdata"),
+                             base::MessageLoopProxy::current(),
+                             base::MessageLoopProxy::current());
+  web_database_service->AddTable(
+      scoped_ptr<WebDatabaseTable>(new KeywordTable()));
+  web_database_service->LoadDatabase();
 
-  search_terms_data_ = new TestingSearchTermsData("http://www.google.com/");
-  model_.reset(new TestingTemplateURLService(
-      profile(), scoped_ptr<SearchTermsData>(search_terms_data_)));
-  model_->AddObserver(this);
+  web_data_service_ =  new KeywordWebDataService(
+      web_database_service.get(), base::MessageLoopProxy::current(),
+      KeywordWebDataService::ProfileErrorCallback());
+  web_data_service_->Init();
+
+  ResetModel(false);
 }
 
 TemplateURLServiceTestUtil::~TemplateURLServiceTestUtil() {
@@ -105,9 +97,7 @@ void TemplateURLServiceTestUtil::ChangeModelToLoadState() {
   // Initialize the web data service so that the database gets updated with
   // any changes made.
 
-  model()->web_data_service_ =
-      WebDataServiceFactory::GetKeywordWebDataForProfile(
-          profile(), Profile::EXPLICIT_ACCESS);
+  model()->web_data_service_ = web_data_service_;
   base::RunLoop().RunUntilIdle();
 }
 
@@ -121,8 +111,12 @@ void TemplateURLServiceTestUtil::ResetModel(bool verify_load) {
   if (model_)
     ClearModel();
   search_terms_data_ = new TestingSearchTermsData("http://www.google.com/");
-  model_.reset(new TestingTemplateURLService(
-      profile(), scoped_ptr<SearchTermsData>(search_terms_data_)));
+  model_.reset(new TemplateURLService(
+      profile()->GetPrefs(), scoped_ptr<SearchTermsData>(search_terms_data_),
+      web_data_service_.get(),
+      scoped_ptr<TemplateURLServiceClient>(
+          new TestingTemplateURLServiceClient(profile(), &search_term_)),
+      NULL, NULL, base::Closure()));
   model()->AddObserver(this);
   changed_count_ = 0;
   if (verify_load)
@@ -130,7 +124,9 @@ void TemplateURLServiceTestUtil::ResetModel(bool verify_load) {
 }
 
 base::string16 TemplateURLServiceTestUtil::GetAndClearSearchTerm() {
-  return model_->GetAndClearSearchTerm();
+  base::string16 search_term;
+  search_term.swap(search_term_);
+  return search_term;
 }
 
 void TemplateURLServiceTestUtil::SetGoogleBaseURL(const GURL& base_url) {
@@ -159,7 +155,3 @@ void TemplateURLServiceTestUtil::RemoveManagedDefaultSearchPreferences() {
   DefaultSearchPrefTestUtil::RemoveManagedPref(
       profile()->GetTestingPrefService());
 }
-
-TemplateURLService* TemplateURLServiceTestUtil::model() {
-  return model_.get();
-}

chrome/browser/search_engines/template_url_service_test_util.h

@@ -9,16 +9,16 @@
 
 #include "base/basictypes.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "components/search_engines/template_url_service_observer.h"
-#include "content/public/test/test_browser_thread_bundle.h"
 
 class GURL;
+class KeywordWebDataService;
 class TemplateURLService;
 class TestingProfile;
 class TestingSearchTermsData;
-class TestingTemplateURLService;
 
 class TemplateURLServiceTestUtil : public TemplateURLServiceObserver {
  public:
@@ -52,12 +52,12 @@ class TemplateURLServiceTestUtil : public TemplateURLServiceObserver {
   // TemplateURLService::SetKeywordSearchTermsForURL and clears the search term.
   base::string16 GetAndClearSearchTerm();
 
-  // Set the google base url.  |base_url| must be valid.
+  // Sets the google base url.  |base_url| must be valid.
   void SetGoogleBaseURL(const GURL& base_url);
 
-  // Set the managed preferences for the default search provider and trigger
+  // Sets the managed preferences for the default search provider and trigger
   // notification. If |alternate_url| is empty, uses an empty list of alternate
-  // URLs, otherwise use a list containing a single entry.
+  // URLs, otherwise uses a list containing a single entry.
   void SetManagedDefaultSearchPreferences(
       bool enabled,
       const std::string& name,
@@ -69,23 +69,22 @@ class TemplateURLServiceTestUtil : public TemplateURLServiceObserver {
       const std::string& alternate_url,
       const std::string& search_terms_replacement_key);
 
-  // Remove all the managed preferences for the default search provider and
-  // trigger notification.
+  // Removes all the managed preferences for the default search provider and
+  // triggers notification.
   void RemoveManagedDefaultSearchPreferences();
 
-  // Returns the TemplateURLService.
-  TemplateURLService* model();
-
-  // Returns the TestingProfile.
+  KeywordWebDataService* web_data_service() { return web_data_service_.get(); }
+  TemplateURLService* model() { return model_.get(); }
   TestingProfile* profile() { return profile_.get(); }
 
  private:
-  content::TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<TestingProfile> profile_;
   base::ScopedTempDir temp_dir_;
   int changed_count_;
+  base::string16 search_term_;
+  scoped_refptr<KeywordWebDataService> web_data_service_;
   TestingSearchTermsData* search_terms_data_;
-  scoped_ptr<TestingTemplateURLService> model_;
+  scoped_ptr<TemplateURLService> model_;
 
   DISALLOW_COPY_AND_ASSIGN(TemplateURLServiceTestUtil);
 };

chrome/chrome_tests_unit.gypi

@@ -1240,10 +1240,6 @@
         'browser/search/suggestions/image_manager_impl_unittest.cc',
         'browser/search_engines/default_search_pref_migration_unittest.cc',
         'browser/search_engines/search_provider_install_data_unittest.cc',
-        'browser/search_engines/template_url_fetcher_unittest.cc',
-        'browser/search_engines/template_url_parser_unittest.cc',
-        'browser/search_engines/template_url_service_sync_unittest.cc',
-        'browser/search_engines/template_url_service_unittest.cc',
         'browser/search_engines/template_url_scraper_unittest.cc',
         'browser/services/gcm/fake_signin_manager.cc',
         'browser/services/gcm/fake_signin_manager.h',
@@ -1949,6 +1945,11 @@
         '../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
         '../components/nacl/loader/nacl_validation_query_unittest.cc',
 
+        '../components/search_engines/template_url_fetcher_unittest.cc',
+        '../components/search_engines/template_url_parser_unittest.cc',
+        '../components/search_engines/template_url_service_sync_unittest.cc',
+        '../components/search_engines/template_url_service_unittest.cc',
+
         '../components/webdata/common/web_data_service_test_util.cc',
         '../components/webdata/common/web_data_service_test_util.h',
 

components/search_engines/DEPS

@@ -17,3 +17,15 @@ include_rules = [
   "+sync",
   "+ui/gfx",
 ]
+
+specific_include_rules = {
+  ".*_unittest\.cc$": [
+    "!chrome/browser/history/history_service.h",
+    "!chrome/browser/history/history_service_factory.h",
+    "!chrome/browser/search_engines/template_url_service_test_util.h",
+    "!chrome/common/chrome_paths.h",
+    "!chrome/test/base/testing_pref_service_syncable.h",
+    "!chrome/test/base/testing_profile.h",
+    "!content/public/test/test_browser_thread_bundle.h",
+  ],
+}

components/search_engines/template_url_service.cc

@@ -1408,14 +1408,6 @@ SyncDataMap TemplateURLService::CreateGUIDToSyncDataMap(
   return data_map;
 }
 
-void TemplateURLService::SetKeywordSearchTermsForURL(
-    const TemplateURL* t_url,
-    const GURL& url,
-    const base::string16& term) {
-  if (client_)
-    client_->SetKeywordSearchTermsForURL(url, t_url->id(), term);
-}
-
 void TemplateURLService::Init(const Initializer* initializers,
                               int num_initializers) {
   if (client_)
@@ -1747,7 +1739,10 @@ void TemplateURLService::UpdateKeywordSearchTermsForURL(
         // count is boosted.
         AddTabToSearchVisit(**i);
       }
-      SetKeywordSearchTermsForURL(*i, details.url, search_terms);
+      if (client_) {
+        client_->SetKeywordSearchTermsForURL(
+            details.url, (*i)->id(), search_terms);
+      }
     }
   }
 }

components/search_engines/template_url_service.h

@@ -374,15 +374,6 @@ class TemplateURLService : public WebDataServiceConsumer,
   }
 #endif
 
- protected:
-  // Cover method for the method of the same name on the HistoryService.
-  // url is the one that was visited with the given search terms.
-  //
-  // This exists and is virtual for testing.
-  virtual void SetKeywordSearchTermsForURL(const TemplateURL* t_url,
-                                           const GURL& url,
-                                           const base::string16& term);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceTest, TestManagedDefaultSearch);
   FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceTest,

chrome/browser/search_engines/template_url_service_sync_unittest.cc → components/search_engines/template_url_service_sync_unittest.cc

@@ -4,11 +4,11 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
+#include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
-#include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/template_url_service_test_util.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
@@ -161,7 +161,7 @@ class TemplateURLServiceSyncTest : public testing::Test {
   // For readability, we redefine an accessor for Model A for use in tests that
   // involve syncing two models.
   TemplateURLService* model_a() { return test_util_a_->model(); }
-  TemplateURLService* model_b() { return model_b_.get(); }
+  TemplateURLService* model_b() { return test_util_b_->model(); }
   TestingProfile* profile_a() { return test_util_a_->profile(); }
   TestChangeProcessor* processor() { return sync_processor_.get(); }
   scoped_ptr<syncer::SyncChangeProcessor> PassProcessor();
@@ -209,10 +209,10 @@ class TemplateURLServiceSyncTest : public testing::Test {
                                const std::string& guid);
 
  protected:
+  base::MessageLoop message_loop_;
   // We keep two TemplateURLServices to test syncing between them.
   scoped_ptr<TemplateURLServiceTestUtil> test_util_a_;
-  scoped_ptr<TestingProfile> profile_b_;
-  scoped_ptr<TemplateURLService> model_b_;
+  scoped_ptr<TemplateURLServiceTestUtil> test_util_b_;
 
   // Our dummy ChangeProcessor used to inspect changes pushed to Sync.
   scoped_ptr<TestChangeProcessor> sync_processor_;
@@ -233,13 +233,9 @@ void TemplateURLServiceSyncTest::SetUp() {
   // in the prepopulate data, which the sync tests don't care about (and would
   // just foul them up).
   test_util_a_->ChangeModelToLoadState();
-  profile_b_.reset(new TestingProfile);
-  TemplateURLServiceFactory::GetInstance()->
-      RegisterUserPrefsOnBrowserContextForTest(profile_b_.get());
-  model_b_.reset(new TemplateURLService(
-      profile_b_->GetPrefs(), make_scoped_ptr(new SearchTermsData), NULL,
-      scoped_ptr<TemplateURLServiceClient>(), NULL, NULL, base::Closure()));
-  model_b_->Load();
+
+  test_util_b_.reset(new TemplateURLServiceTestUtil);
+  test_util_b_->VerifyLoad();
 }
 
 void TemplateURLServiceSyncTest::TearDown() {