Enabled pressing TAB to cycle through the Omnibox results.

chrome/browser/autocomplete/autocomplete_unittest.cc

 // Copyright (c) 2011 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 "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ skipping 14 matching lines @@
 
 static std::ostream& operator<<(std::ostream& os,
                                 const AutocompleteResult::const_iterator& it) {
   return os << static_cast<const AutocompleteMatch*>(&(*it));
 }
 
 namespace {
 
 const size_t num_results_per_provider = 3;
 
+}  // namespace

[Peter Kasting, 2011/12/15 22:56:04]

Nit: For short namespaces like this let's just kill the newlines inside and skip
the closing comment.

+
 // Autocomplete provider that provides known results. Note that this is
 // refcounted so that it can also be a task on the message loop.
 class TestProvider : public AutocompleteProvider {
  public:
   TestProvider(int relevance, const string16& prefix)
       : AutocompleteProvider(NULL, NULL, ""),
         relevance_(relevance),
         prefix_(prefix) {
   }
 
@@ skipping 61 matching lines @@
 
 class AutocompleteProviderTest : public testing::Test,
                                  public content::NotificationObserver {
  protected:
   void ResetControllerWithTestProviders(bool same_destinations);
 
   // Runs a query on the input "a", and makes sure both providers' input is
   // properly collected.
   void RunTest();
 
+  void RunQuery(string16 query);

[Peter Kasting, 2011/12/15 22:56:04]

Nit: const string16&

+
   void ResetControllerWithTestProvidersWithKeywordAndSearchProviders();
+  void ResetControllerWithKeywordProvider();
   void RunExactKeymatchTest(bool allow_exact_keyword_match);
 
   // These providers are owned by the controller once it's created.
   ACProviders providers_;
 
   AutocompleteResult result_;
+  scoped_ptr<AutocompleteController> controller_;
 
  private:
   // content::NotificationObserver
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details);
 
   MessageLoopForUI message_loop_;
-  scoped_ptr<AutocompleteController> controller_;
   content::NotificationRegistrar registrar_;
   TestingProfile profile_;
 };
 
 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();
 
@@ skipping 53 matching lines @@
   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);
 
   AutocompleteController* controller =
       new AutocompleteController(providers_, &profile_);
   controller_.reset(controller);

[Peter Kasting, 2011/12/15 22:56:04]

Nit: Just combine these two lines

 }
 
+void AutocompleteProviderTest::
+    ResetControllerWithKeywordProvider() {
+  profile_.CreateTemplateURLService();
+
+  TemplateURLService* turl_model =
+      TemplateURLServiceFactory::GetForProfile(&profile_);
+
+  // Create a TemplateURL for KeywordProvider.
+  TemplateURL* keyword_t_url = new TemplateURL();
+  keyword_t_url->set_short_name(ASCIIToUTF16("foo.com"));
+  keyword_t_url->set_keyword(ASCIIToUTF16("foo.com"));
+  keyword_t_url->SetURL("http://foo.com/{searchTerms}", 0, 0);
+  turl_model->Add(keyword_t_url);
+  ASSERT_NE(0, keyword_t_url->id());
+
+  // Create another TemplateURL for KeywordProvider.
+  keyword_t_url = new TemplateURL();
+  keyword_t_url->set_short_name(ASCIIToUTF16("bar.com"));
+  keyword_t_url->set_keyword(ASCIIToUTF16("bar.com"));
+  keyword_t_url->SetURL("http://bar.com/{searchTerms}", 0, 0);
+  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);
+}
+
 void AutocompleteProviderTest::RunTest() {
+  RunQuery(ASCIIToUTF16("a"));
+}
+
+void AutocompleteProviderTest::RunQuery(string16 query) {
   result_.Reset();
-  controller_->Start(ASCIIToUTF16("a"), string16(), true, false, true,
-                     AutocompleteInput::ALL_MATCHES);
+  controller_->Start(query, string16(), true, false, true,
+      AutocompleteInput::ALL_MATCHES);
 
-  // The message loop will terminate when all autocomplete input has been
-  // collected.
-  MessageLoop::current()->Run();
+  if (!controller_->done())
+    // The message loop will terminate when all autocomplete input has been
+    // collected.
+    MessageLoop::current()->Run();
 }
 
 void AutocompleteProviderTest::RunExactKeymatchTest(
     bool allow_exact_keyword_match) {
   // Send the controller input which exactly matches the keyword provider we
   // created in ResetControllerWithKeywordAndSearchProviders().  The default
   // match should thus be a keyword match iff |allow_exact_keyword_match| is
   // true.
   controller_->Start(ASCIIToUTF16("k test"), string16(), true, false,
                      allow_exact_keyword_match,
@@ skipping 39 matching lines @@
        i != result_.end(); ++i)
     EXPECT_EQ(providers_[1], i->provider);
 }
 
 TEST_F(AutocompleteProviderTest, AllowExactKeywordMatch) {
   ResetControllerWithTestProvidersWithKeywordAndSearchProviders();
   RunExactKeymatchTest(true);
   RunExactKeymatchTest(false);
 }
 
+// Test that redundant associated keywords are removed.
+TEST_F(AutocompleteProviderTest, RedundantKeywordsIgnoredInResult) {
+  ResetControllerWithKeywordProvider();
+
+  // Get the controller's internal members in the correct state.
+  RunQuery(ASCIIToUTF16("fo"));
+
+  struct test_data {

[Peter Kasting, 2011/12/15 22:56:04]

Nit: Struct names use CamelCase.

+    const string16 fill_into_edit;
+    const string16 keyword;
+    const bool expected_keyword_result;
+  } result_data[] = {
+    { ASCIIToUTF16("fo"), ASCIIToUTF16(""), false },

[Peter Kasting, 2011/12/15 22:56:04]

Nit: ASCIIToUTF16("") -> string16() (many places)

+    { ASCIIToUTF16("foo.com"), ASCIIToUTF16(""), true },
+    { ASCIIToUTF16("foo.com"), ASCIIToUTF16(""), false },
+    { string16(), string16(), false },

[Peter Kasting, 2011/12/15 22:56:04]

The usage of "blank rows" in here to trigger the UpdateAssociatedKeywords call
is a bit too clever.

Instead it seems like you could just declare three instances of the test data
struct, one with each set of cases; then you could repeatedly call a helper
function that takes such a struct, populates the result set accordingly, and
then runs your checks at the end.

You can even use SCOPED_TRACE when doing this so you don't need to manually
print the set number in the helper function.

+    { ASCIIToUTF16("foo.com"), ASCIIToUTF16("foo.com"), false },
+    { ASCIIToUTF16("foo.com"), ASCIIToUTF16(""), false },
+    { string16(), string16(), false },
+    { ASCIIToUTF16("fo"), ASCIIToUTF16(""), false },
+    { ASCIIToUTF16("foo.com"), ASCIIToUTF16(""), true },
+    { ASCIIToUTF16("foo.com"), ASCIIToUTF16(""), false },
+    { ASCIIToUTF16("bar.com"), ASCIIToUTF16(""), true },
+    { string16(), string16(), false },
+  };
+
+  ACMatches matches;
+  size_t set = 1;
+  size_t set_begin = 0;
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(result_data); ++i) {
+    if (result_data[i].fill_into_edit.length() > 0) {
+      AutocompleteMatch match;
+      match.fill_into_edit = result_data[i].fill_into_edit;
+      match.keyword = result_data[i].keyword;
+      matches.push_back(match);
+    } else {
+      AutocompleteResult result;
+      result.AppendMatches(matches);
+
+      controller_->UpdateAssociatedKeywords(&result);
+      for (size_t j = 0; j < result.size(); ++j)
+        EXPECT_EQ(result_data[set_begin + j].expected_keyword_result,
+            result.match_at(j).associated_keyword.get() != NULL) <<
+            "Set: " << set << " Result: " << (j + 1);
+
+      matches.clear();
+      set++;
+      set_begin = i + 1;
+    }
+  }
+}
+
 typedef testing::Test AutocompleteTest;
 
 TEST_F(AutocompleteTest, InputType) {
   struct test_data {
     const string16 input;
     const AutocompleteInput::Type type;
   } input_cases[] = {
     { ASCIIToUTF16(""), AutocompleteInput::INVALID },
     { ASCIIToUTF16("?"), AutocompleteInput::FORCED_QUERY },
     { ASCIIToUTF16("?foo"), AutocompleteInput::FORCED_QUERY },
@@ skipping 195 matching lines @@
                                                    &scheme,
                                                    &host);
     AutocompleteInput input(input_cases[i].input, string16(), true, false,
                             true, AutocompleteInput::ALL_MATCHES);
     EXPECT_EQ(input_cases[i].scheme.begin, scheme.begin);
     EXPECT_EQ(input_cases[i].scheme.len, scheme.len);
     EXPECT_EQ(input_cases[i].host.begin, host.begin);
     EXPECT_EQ(input_cases[i].host.len, host.len);
   }
 }
-
-}  // namespace