Omnibox enters keyword search mode incorrectly

chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc

Index: chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc b/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1055b319ff960cb8d4d9cfc678e779ae02a3e7be
--- /dev/null
+++ b/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc
@@ -0,0 +1,137 @@
+// 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 "chrome/browser/autocomplete/autocomplete_popup_model.h"
+
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/autocomplete/autocomplete_match.h"
+#include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/test/base/testing_profile.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class AutoCompletePopupModelTest : public testing::Test {
+ public:
+  AutoCompletePopupModelTest() {
+  }
+
+  // See description above class for what this registers.

[Peter Kasting, 2011/09/14 17:49:37]

Nit: This comment makes no sense, remove.

[keishi, 2011/09/15 02:23:13]

Done.

+  virtual void SetUp();
+

[Peter Kasting, 2011/09/14 17:49:37]

Nit: Unnecessary blank line

[keishi, 2011/09/15 02:23:13]

Done.

+  virtual void TearDown();
+
+ protected:
+  AutocompleteMatch CreateMatch(const string16& keyword,
+                                const string16& query_string,
+                                AutocompleteMatch::Type type);
+
+  scoped_ptr<TestingProfile> profile_;
+  scoped_ptr<AutocompletePopupModel> model_;
+  scoped_ptr<AutocompleteEditModel> edit_model_;
+};
+
+void AutoCompletePopupModelTest::SetUp() {
+  profile_.reset(new TestingProfile());
+  profile_->CreateTemplateURLService();
+  edit_model_.reset(new AutocompleteEditModel(NULL, NULL, profile_.get()));
+  model_.reset(new AutocompletePopupModel(NULL, edit_model_.get()));
+
+  TemplateURLService* turl_model =
+      TemplateURLServiceFactory::GetForProfile(profile_.get());
+
+  turl_model->Load();
+
+  // Reset the default TemplateURL.
+  TemplateURL* default_t_url = new TemplateURL();
+  default_t_url->set_keyword(ASCIIToUTF16("t"));
+  default_t_url->SetURL("http://defaultturl/{searchTerms}", 0, 0);
+  turl_model->Add(default_t_url);
+  turl_model->SetDefaultSearchProvider(default_t_url);
+  ASSERT_NE(0, default_t_url->id());
+
+  // Create another TemplateURL for KeywordProvider.
+  TemplateURL* keyword_t_url = new TemplateURL();
+  keyword_t_url->set_short_name(ASCIIToUTF16("k"));
+  keyword_t_url->set_keyword(ASCIIToUTF16("k"));
+  keyword_t_url->SetURL("http://keyword/{searchTerms}", 0, 0);
+  turl_model->Add(keyword_t_url);
+  ASSERT_NE(0, keyword_t_url->id());
+}
+
+void AutoCompletePopupModelTest::TearDown() {
+  profile_.reset(NULL);

[Peter Kasting, 2011/09/14 17:49:37]

Nit: Don't pass an argument in any of these.

[keishi, 2011/09/15 02:23:13]

Done.

+  model_.reset(NULL);
+  edit_model_.reset(NULL);
+}
+
+AutocompleteMatch AutoCompletePopupModelTest::CreateMatch(
+    const string16& keyword,
+    const string16& query_string,
+    AutocompleteMatch::Type type) {
+  AutocompleteMatch match(NULL, 0, false, type);
+  match.contents.assign(query_string);

[Peter Kasting, 2011/09/14 17:49:37]

Nit: Use = instead of assign()

[keishi, 2011/09/15 02:23:13]

Done.

+  TemplateURLService* template_url_service =
+      TemplateURLServiceFactory::GetForProfile(profile_.get());
+  if (!keyword.empty()) {
+    const TemplateURL* template_url =
+        template_url_service->GetTemplateURLForKeyword(keyword);
+    match.template_url =
+        TemplateURL::SupportsReplacement(template_url) ? template_url : NULL;
+  }
+  if (match.template_url) {

[Peter Kasting, 2011/09/14 17:49:37]

Nit: No {}

[keishi, 2011/09/15 02:23:13]

Done.

+    match.fill_into_edit.append(match.template_url->keyword() + char16(' '));

[Peter Kasting, 2011/09/14 17:49:37]

Nit: Use = instead of append()

[keishi, 2011/09/15 02:23:13]

Done.

+  } else {
+    match.template_url = template_url_service->GetDefaultSearchProvider();
+  }
+  match.fill_into_edit.append(query_string);
+  match.transition = keyword.empty() ? PageTransition::GENERATED :

[Peter Kasting, 2011/09/14 17:49:37]

Nit: Wrap after ? instead of :

[keishi, 2011/09/15 02:23:13]

Done.

+                                       PageTransition::KEYWORD;
+  return match;
+}
+
+TEST_F(AutoCompletePopupModelTest, GetKeywordForMatch) {
+  string16 keyword;
+
+  // Possible matches when the input is "tfoo"
+  EXPECT_FALSE(model_->GetKeywordForMatch(

[Peter Kasting, 2011/09/14 17:49:37]

Nit: Write a helper function:

void AutoCompletePopupModelTest::RunTest(char* input,
                                         AutocompleteMatch::Type type,
                                         char* keyword) {
  string16 keyword16(ASCIIToUTF16(keyword));
  string16 detected_keyword;
  EXPECT_FALSE(model_->GetKeywordForMatch(
      CreateMatch(keyword16, ASCIIToUTF16(input), type), &detected_keyword));
  EXPECT_EQ(keyword16, detected_keyword);
}

Then use that in all these cases.

[keishi, 2011/09/15 02:23:13]

Done.

+               CreateMatch(string16(), ASCIIToUTF16("tfoo"),
+                           AutocompleteMatch::SEARCH_WHAT_YOU_TYPED),
+               &keyword));
+  EXPECT_EQ(string16(), keyword);
+  EXPECT_FALSE(model_->GetKeywordForMatch(
+               CreateMatch(string16(), ASCIIToUTF16("tfoo"),
+                           AutocompleteMatch::SEARCH_HISTORY),
+               &keyword));
+  EXPECT_EQ(string16(), keyword);
+  EXPECT_FALSE(model_->GetKeywordForMatch(
+               CreateMatch(string16(), ASCIIToUTF16("tfoo"),
+                           AutocompleteMatch::SEARCH_SUGGEST),
+               &keyword));
+  EXPECT_EQ(string16(), keyword);
+
+  // Possible matches when the input is "t foo"
+  EXPECT_FALSE(model_->GetKeywordForMatch(
+               CreateMatch(ASCIIToUTF16("t"), ASCIIToUTF16("foo"),
+                           AutocompleteMatch::SEARCH_HISTORY),
+               &keyword));
+  EXPECT_EQ(ASCIIToUTF16("t"), keyword);
+  EXPECT_FALSE(model_->GetKeywordForMatch(
+               CreateMatch(ASCIIToUTF16("t"), ASCIIToUTF16("foo"),
+                           AutocompleteMatch::SEARCH_OTHER_ENGINE),
+               &keyword));
+  EXPECT_EQ(ASCIIToUTF16("t"), keyword);
+
+  // Possible matches when the input is "k foo"
+  EXPECT_FALSE(model_->GetKeywordForMatch(
+               CreateMatch(ASCIIToUTF16("k"), ASCIIToUTF16("foo"),
+                           AutocompleteMatch::SEARCH_HISTORY),
+               &keyword));
+  EXPECT_EQ(ASCIIToUTF16("k"), keyword);
+  EXPECT_FALSE(model_->GetKeywordForMatch(
+               CreateMatch(ASCIIToUTF16("k"), ASCIIToUTF16("foo"),
+                           AutocompleteMatch::SEARCH_OTHER_ENGINE),
+               &keyword));
+  EXPECT_EQ(ASCIIToUTF16("k"), keyword);
+}