autocomplete: Add AutocompleteProvider::Type enum.

chrome/browser/autocomplete/autocomplete_provider_unittest.cc

Index: chrome/browser/autocomplete/autocomplete_provider_unittest.cc
diff --git a/chrome/browser/autocomplete/autocomplete_provider_unittest.cc b/chrome/browser/autocomplete/autocomplete_provider_unittest.cc
index 61d9cc4fefe78de142f069e5f526ea4909de1fff..cd31693a09181c9c2c2f1756c95fc6c1247cc6b0 100644
--- a/chrome/browser/autocomplete/autocomplete_provider_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_provider_unittest.cc
@@ -45,7 +45,7 @@ class TestProvider : public AutocompleteProvider {
   TestProvider(int relevance, const string16& prefix,
                Profile* profile,
                const string16 match_keyword)
-      : AutocompleteProvider(NULL, profile, ""),
+      : AutocompleteProvider(NULL, profile, AutocompleteProvider::TYPE_SEARCH),
         relevance_(relevance),
         prefix_(prefix),
         match_keyword_(match_keyword) {
@@ -163,7 +163,11 @@ class AutocompleteProviderTest : public testing::Test,
   void RegisterTemplateURL(const string16 keyword,
                            const std::string& template_url);
 
-  void ResetControllerWithTestProviders(bool same_destinations);
+  // Resets |controller_| with two TestProviders.  |provider1_ptr| and
+  // |provider2_ptr| are updated to point to the new providers if non-NULL.
+  void ResetControllerWithTestProviders(bool same_destinations,
+                                        TestProvider** provider1_ptr,
+                                        TestProvider** provider2_ptr);
 
   // Runs a query on the input "a", and makes sure both providers' input is
   // properly collected.
@@ -177,12 +181,13 @@ class AutocompleteProviderTest : public testing::Test,
 
   void RunQuery(const string16 query);
 
-  void ResetControllerWithTestProvidersWithKeywordAndSearchProviders();
+  void ResetControllerWithKeywordAndSearchProviders();
   void ResetControllerWithKeywordProvider();
   void RunExactKeymatchTest(bool allow_exact_keyword_match);
 
-  // These providers are owned by the controller once it's created.
-  ACProviders providers_;
+  // Returns the provider of type |type| from |controller_|.  NULL is returned
+  // if no provider of this type has been created.
+  AutocompleteProvider* GetProvider(AutocompleteProvider::Type type);
 
   AutocompleteResult result_;
   scoped_ptr<AutocompleteController> controller_;
@@ -216,11 +221,9 @@ void AutocompleteProviderTest:: RegisterTemplateURL(
 }
 
 void AutocompleteProviderTest::ResetControllerWithTestProviders(
-    bool same_destinations) {
-  // Forget about any existing providers.  The controller owns them and will
-  // Release() them below, when we delete it during the call to reset().
-  providers_.clear();
-
+    bool same_destinations,
+    TestProvider** provider1_ptr,
+    TestProvider** provider2_ptr) {
   // TODO: Move it outside this method, after refactoring the existing
   // unit tests.  Specifically:
   //   (1) Make sure that AutocompleteMatch.keyword is set iff there is
@@ -234,39 +237,46 @@ void AutocompleteProviderTest::ResetControllerWithTestProviders(
   RegisterTemplateURL(ASCIIToUTF16(kTestTemplateURLKeyword),
                       "http://aqs/{searchTerms}/{google:assistedQueryStats}");
 
+  ACProviders providers;
+
   // Construct two new providers, with either the same or different prefixes.
-  TestProvider* providerA =
-      new TestProvider(kResultsPerProvider,
-                       ASCIIToUTF16("http://a"),
-                       &profile_,
-                       ASCIIToUTF16(kTestTemplateURLKeyword));
-  providerA->AddRef();
-  providers_.push_back(providerA);
-
-  TestProvider* providerB = new TestProvider(
+  TestProvider* provider1 = new TestProvider(
+      kResultsPerProvider,
+      ASCIIToUTF16("http://a"),
+      &profile_,
+      ASCIIToUTF16(kTestTemplateURLKeyword));
+  provider1->AddRef();
+  providers.push_back(provider1);
+
+  TestProvider* provider2 = new TestProvider(
       kResultsPerProvider * 2,
       same_destinations ? ASCIIToUTF16("http://a") : ASCIIToUTF16("http://b"),
       &profile_,
       string16());
-  providerB->AddRef();
-  providers_.push_back(providerB);
+  provider2->AddRef();
+  providers.push_back(provider2);
 
   // Reset the controller to contain our new providers.
   AutocompleteController* controller =
-      new AutocompleteController(providers_, &profile_);
+      new AutocompleteController(providers, &profile_);

[Peter Kasting, 2012/09/11 01:38:42]

Wait a minute.  AutocompleteController declares AutocompleteProviderTest as a
friend class (which is how you managed to access its providers directly for
GetProvider()).  So why can't we kill the UNIT_TEST constructor entirely and do
the following here:

  controller_.reset(new AutocompleteController(&profile_, NULL, 0);
  controller_->providers_.swap(providers);

We shouldn't even need to set the |search_provider_| or |keyword_provider_|
members of the controller -- the former because no one will care about it, the
latter because TestProvider is of type SEARCH and thus this was always NULL
before.

[Daniel Erat, 2012/09/11 02:09:22]

Nice!  Done.

   controller_.reset(controller);
-  providerA->set_listener(controller);
-  providerB->set_listener(controller);
+  provider1->set_listener(controller);
+  provider2->set_listener(controller);
 
   // The providers don't complete synchronously, so listen for "result updated"
   // notifications.
   registrar_.Add(this,
                  chrome::NOTIFICATION_AUTOCOMPLETE_CONTROLLER_RESULT_READY,
                  content::Source<AutocompleteController>(controller));
+
+  if (provider1_ptr)
+    *provider1_ptr = provider1;
+  if (provider2_ptr)
+    *provider2_ptr = provider2;
 }
 
 void AutocompleteProviderTest::
-    ResetControllerWithTestProvidersWithKeywordAndSearchProviders() {
+    ResetControllerWithKeywordAndSearchProviders() {
   TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
       &profile_, &TemplateURLServiceFactory::BuildInstanceFor);
 
@@ -289,20 +299,9 @@ void AutocompleteProviderTest::
   turl_model->Add(keyword_t_url);
   ASSERT_NE(0, keyword_t_url->id());
 
-  // Forget about any existing providers.  The controller owns them and will
-  // Release() them below, when we delete it during the call to reset().
-  providers_.clear();
-
-  // Create both a keyword and search provider, and add them in that order.
-  // (Order is important; see comments in RunExactKeymatchTest().)
-  AutocompleteProvider* keyword_provider = new KeywordProvider(NULL, &profile_);
-  keyword_provider->AddRef();
-  providers_.push_back(keyword_provider);
-  AutocompleteProvider* search_provider = new SearchProvider(NULL, &profile_);
-  search_provider->AddRef();
-  providers_.push_back(search_provider);
-
-  controller_.reset(new AutocompleteController(providers_, &profile_));
+  controller_.reset(new AutocompleteController(
+      &profile_, NULL,
+      AutocompleteProvider::TYPE_KEYWORD | AutocompleteProvider::TYPE_SEARCH));
 }
 
 void AutocompleteProviderTest::ResetControllerWithKeywordProvider() {
@@ -329,20 +328,8 @@ void AutocompleteProviderTest::ResetControllerWithKeywordProvider() {
   turl_model->Add(keyword_t_url);
   ASSERT_NE(0, keyword_t_url->id());
 
-  // Forget about any existing providers.  The controller owns them and will
-  // Release() them below, when we delete it during the call to reset().
-  providers_.clear();
-
-  // Create both a keyword and search provider, and add them in that order.
-  // (Order is important; see comments in RunExactKeymatchTest().)
-  KeywordProvider* keyword_provider = new KeywordProvider(NULL, &profile_);
-  keyword_provider->AddRef();
-  providers_.push_back(keyword_provider);
-
-  AutocompleteController* controller =
-      new AutocompleteController(providers_, &profile_);
-  controller->set_keyword_provider(keyword_provider);
-  controller_.reset(controller);
+  controller_.reset(new AutocompleteController(
+      &profile_, NULL, AutocompleteProvider::TYPE_KEYWORD));
 }
 
 void AutocompleteProviderTest::RunTest() {
@@ -419,13 +406,21 @@ void AutocompleteProviderTest::RunExactKeymatchTest(
                      allow_exact_keyword_match,
                      AutocompleteInput::SYNCHRONOUS_MATCHES);
   EXPECT_TRUE(controller_->done());
-  // ResetControllerWithKeywordAndSearchProviders() adds the keyword provider
-  // first, then the search provider.  So if the default match is a keyword
-  // match, it will come from provider 0, otherwise from provider 1.
-  EXPECT_EQ(providers_[allow_exact_keyword_match ? 0 : 1],
+  EXPECT_EQ(GetProvider(allow_exact_keyword_match ?

[Peter Kasting, 2012/09/11 01:38:42]

Nit: Simpler, because you can kill GetProvider() entirely:

  EXPECT_EQ(allow_exact_keyword_match ?
      AutocompleteProvider::TYPE_KEYWORD : AutocompleteProvider::TYPE_SEARCH,
      controller_->result().default_match()->provider->type());

[Daniel Erat, 2012/09/11 02:09:22]

Duh.  Done.

+      AutocompleteProvider::TYPE_KEYWORD : AutocompleteProvider::TYPE_SEARCH),
       controller_->result().default_match()->provider);
 }
 
+AutocompleteProvider* AutocompleteProviderTest::GetProvider(
+    AutocompleteProvider::Type type) {
+  for (ACProviders::const_iterator it = controller_->providers()->begin();
+       it != controller_->providers()->end(); ++it) {
+    if ((*it)->type() == type)
+      return *it;
+  }
+  return NULL;
+}
+
 void AutocompleteProviderTest::Observe(
     int type,
     const content::NotificationSource& source,
@@ -438,19 +433,21 @@ void AutocompleteProviderTest::Observe(
 
 // Tests that the default selection is set properly when updating results.
 TEST_F(AutocompleteProviderTest, Query) {
-  ResetControllerWithTestProviders(false);
+  TestProvider* provider1 = NULL;
+  TestProvider* provider2 = NULL;
+  ResetControllerWithTestProviders(false, &provider1, &provider2);
   RunTest();
 
   // Make sure the default match gets set to the highest relevance match.  The
   // highest relevance matches should come from the second provider.
   EXPECT_EQ(kResultsPerProvider * 2, result_.size());  // two providers
   ASSERT_NE(result_.end(), result_.default_match());
-  EXPECT_EQ(providers_[1], result_.default_match()->provider);
+  EXPECT_EQ(provider2, result_.default_match()->provider);
 }
 
 // Tests assisted query stats.
 TEST_F(AutocompleteProviderTest, AssistedQueryStats) {
-  ResetControllerWithTestProviders(false);
+  ResetControllerWithTestProviders(false, NULL, NULL);
   RunTest();
 
   EXPECT_EQ(kResultsPerProvider * 2, result_.size());  // two providers
@@ -469,7 +466,9 @@ TEST_F(AutocompleteProviderTest, AssistedQueryStats) {
 }
 
 TEST_F(AutocompleteProviderTest, RemoveDuplicates) {
-  ResetControllerWithTestProviders(true);
+  TestProvider* provider1 = NULL;
+  TestProvider* provider2 = NULL;
+  ResetControllerWithTestProviders(true, &provider1, &provider2);
   RunTest();
 
   // Make sure all the first provider's results were eliminated by the second
@@ -477,11 +476,11 @@ TEST_F(AutocompleteProviderTest, RemoveDuplicates) {
   EXPECT_EQ(kResultsPerProvider, result_.size());
   for (AutocompleteResult::const_iterator i(result_.begin());
        i != result_.end(); ++i)
-    EXPECT_EQ(providers_[1], i->provider);
+    EXPECT_EQ(provider2, i->provider);
 }
 
 TEST_F(AutocompleteProviderTest, AllowExactKeywordMatch) {
-  ResetControllerWithTestProvidersWithKeywordAndSearchProviders();
+  ResetControllerWithKeywordAndSearchProviders();
   RunExactKeymatchTest(true);
   RunExactKeymatchTest(false);
 }
@@ -529,7 +528,7 @@ TEST_F(AutocompleteProviderTest, RedundantKeywordsIgnoredInResult) {
 }
 
 TEST_F(AutocompleteProviderTest, UpdateAssistedQueryStats) {
-  ResetControllerWithTestProviders(false);
+  ResetControllerWithTestProviders(false, NULL, NULL);
 
   {
     AssistedQueryStatsTestData test_data[] = {
@@ -564,4 +563,3 @@ TEST_F(AutocompleteProviderTest, UpdateAssistedQueryStats) {
     RunAssistedQueryStatsTest(test_data, ARRAYSIZE_UNSAFE(test_data));
   }
 }
-