Omnibox: Create Keyword Verbatim Result in Search Provider

It's a little weird that SearchProvider creates the default provider's
search-what-you-typed (a.k.a. verbatim) match, the default provider's
suggestion matches, and the keyword provider's suggestions, but not
the keyword provider's search-what-you-typed match.

This change adds code to create the keyword-search-what-you-typed 
suggestion (for keywords that aren't extensions) to SearchProvider.  It
also modifies the code in KeywordProvider to prevent creating the
keyword-search-what-you-typed suggestions in these cases.

This is the natural step to fix the linked bug.  It also preps the path to get suggested relevance scores for keyword providers.

It should have nearly no user-visible effect.  The reason for the nearly no
and not a "no" is because now that the SearchProvider is generating
the keyword verbatim result, we allow the SearchProvider to return
an additional result to the AutocompleteController.  (The keyword provider
would always return this result, so we leave an extra slot in SearchProvider's
result set for this result.)  However, it's vaguely conceivable if we
have suggest relevance scoring and have more than four very-highly-scoring
suggestions that those suggestions could starve out the keyword verbatim
result.  Frankly, I'm not worried about this.

Note that there is a logging change: the verbatim result for the
keyword provider's search was previously marked as Keyword
provider, SEARCH_OTHER_ENGINE result type.  Now it will be
marked as Search provider, SEARCH_OTHER_ENGINE result type.

As for how I created CalculateRelevanceForKeywordVerbatim(), it's a direct
copy of KeywordProvider::CalculateRelevance() with certain tests removed
for things that are guaranteed to be true/false in SearchProvider if we're
using the keyword fetcher.  I decided not to make this code in common
because there's no reason the Keyword scoring algorithm should
be the same as the SearchProvider's scoring algorithm.  i.e., if one
later alters one of these functions, no harm will come as a result of
the fact that the code isn't shared.

BUG=171104

Review URL: https://chromiumcodereview.appspot.com/12090006

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

7 files changed, 301 insertions, 56 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_provider_unittest.cc b/chrome/browser/autocomplete/autocomplete_provider_unittest.cc
index 292a69b..fe4be82 100644
--- a/chrome/browser/autocomplete/autocomplete_provider_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_provider_unittest.cc
@@ -397,15 +397,20 @@ 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.
+  // match should thus be a search-other-engine match iff
+  // |allow_exact_keyword_match| is true.  Regardless, the match should
+  // be from SearchProvider.  (It provides all verbatim search matches,
+  // keyword or not.)
   controller_->Start(AutocompleteInput(
       ASCIIToUTF16("k test"), string16::npos, string16(), true, false,
       allow_exact_keyword_match, AutocompleteInput::SYNCHRONOUS_MATCHES));
   EXPECT_TRUE(controller_->done());
-  EXPECT_EQ(allow_exact_keyword_match ?
-      AutocompleteProvider::TYPE_KEYWORD : AutocompleteProvider::TYPE_SEARCH,
+  EXPECT_EQ(AutocompleteProvider::TYPE_SEARCH,
       controller_->result().default_match()->provider->type());
+  EXPECT_EQ(allow_exact_keyword_match ?
+      AutocompleteMatch::SEARCH_OTHER_ENGINE :
+      AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+      controller_->result().default_match()->type);
 }
 
 void AutocompleteProviderTest::Observe(
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index 9332417..7cbc6dc 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -306,14 +306,24 @@ void KeywordProvider::Start(const AutocompleteInput& input,
   // Any exact match is going to be the highest quality match, and thus at the
   // front of our vector.
   if (keyword_matches.front() == keyword) {
-    const TemplateURL* template_url(model->GetTemplateURLForKeyword(keyword));
+    const TemplateURL* template_url = model->GetTemplateURLForKeyword(keyword);
+    const bool is_extension_keyword = template_url->IsExtensionKeyword();
+
+    // Only create an exact match if |remaining_input| is empty or if
+    // this is an extension keyword.  If |remaining_input| is a
+    // non-empty non-extension keyword (i.e., a regular keyword that
+    // supports replacement and that has extra text following it),
+    // then SearchProvider creates the exact (a.k.a. verbatim) match.
+    if (!remaining_input.empty() && !is_extension_keyword)
+      return;
+
     // TODO(pkasting): We should probably check that if the user explicitly
     // typed a scheme, that scheme matches the one in |template_url|.
     matches_.push_back(CreateAutocompleteMatch(model, keyword, input,
                                                keyword.length(),
                                                remaining_input, -1));
 
-    if (profile_ && template_url->IsExtensionKeyword()) {
+    if (profile_ && is_extension_keyword) {
       if (input.matches_requested() == AutocompleteInput::ALL_MATCHES) {
         if (template_url->GetExtensionId() != current_keyword_extension_id_)
           MaybeEndExtensionKeywordMode();
@@ -447,6 +457,15 @@ int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type,
                                         bool supports_replacement,
                                         bool prefer_keyword,
                                         bool allow_exact_keyword_match) {
+  // This function is responsible for scoring suggestions of keywords
+  // themselves and the suggestion of the verbatim query on an
+  // extension keyword.  SearchProvider::CalculateRelevanceForKeywordVerbatim()
+  // scores verbatim query suggestions for non-extension keywords.
+  // These two functions are currently in sync, but there's no reason
+  // we couldn't decide in the future to score verbatim matches
+  // differently for extension and non-extension keywords.  If you
+  // make such a change, however, you should update this comment to
+  // describe it, so it's clear why the functions diverge.
   if (!complete)
     return (type == AutocompleteInput::URL) ? 700 : 450;
   if (!supports_replacement || (allow_exact_keyword_match && prefer_keyword))
diff --git a/chrome/browser/autocomplete/keyword_provider_unittest.cc b/chrome/browser/autocomplete/keyword_provider_unittest.cc
index 07d035e..6f9ac9e 100644
--- a/chrome/browser/autocomplete/keyword_provider_unittest.cc
+++ b/chrome/browser/autocomplete/keyword_provider_unittest.cc
@@ -88,10 +88,13 @@ TEST_F(KeywordProviderTest, Edit) {
 
     // Check that tokenization only collapses whitespace between first tokens,
     // no-query-input cases have a space appended, and action is not escaped.
-    {ASCIIToUTF16("z foo"),           1, {ASCIIToUTF16("z foo")}},
     {ASCIIToUTF16("z"),               1, {ASCIIToUTF16("z ")}},
     {ASCIIToUTF16("z    \t"),         1, {ASCIIToUTF16("z ")}},
-    {ASCIIToUTF16("z   a   b   c++"), 1, {ASCIIToUTF16("z a   b   c++")}},
+
+    // Check that exact, substituting keywords with a verbatim search term
+    // don't generate a result.  (These are handled by SearchProvider.)
+    {ASCIIToUTF16("z foo"),           0, {}},
+    {ASCIIToUTF16("z   a   b   c++"), 0, {}},
 
     // Matches should be limited to three, and sorted in quality order, not
     // alphabetical.
@@ -103,9 +106,11 @@ TEST_F(KeywordProviderTest, Edit) {
     {ASCIIToUTF16("www.a"),           3, {ASCIIToUTF16("aa "),
                                           ASCIIToUTF16("ab "),
                                           ASCIIToUTF16("aaaa ")}},
-    // Exact matches should prevent returning inexact matches.
-    {ASCIIToUTF16("aaaa foo"),        1, {ASCIIToUTF16("aaaa foo")}},
-    {ASCIIToUTF16("www.aaaa foo"),    1, {ASCIIToUTF16("aaaa foo")}},
+    // Exact matches should prevent returning inexact matches.  Also, the
+    // verbatim query for this keyword match should not be returned.  (It's
+    // returned by SearchProvider.)
+    {ASCIIToUTF16("aaaa foo"),        0, {}},
+    {ASCIIToUTF16("www.aaaa foo"),    0, {}},
 
     // Clean up keyword input properly.  "http" and "https" are the only
     // allowed schemes.
@@ -133,8 +138,8 @@ TEST_F(KeywordProviderTest, URL) {
 
     // Check that tokenization only collapses whitespace between first tokens
     // and query input, but not rest of URL, is escaped.
-    {ASCIIToUTF16("z   a   b   c++"), 1, {GURL("a+++b+++c%2B%2B=z")}},
-    {ASCIIToUTF16("www.www www"),     1, {GURL(" +%2B?=wwwfoo ")}},
+    {ASCIIToUTF16("w  bar +baz"),     2, {GURL(" +%2B?=bar+%2Bbazfoo "),
+                                          GURL("bar+%2Bbaz=z")}},
 
     // Substitution should work with various locations of the "%s".
     {ASCIIToUTF16("aaa 1a2b"),        2, {GURL("http://aaaa/?aaaa=1&b=1a2b&c"),
@@ -158,21 +163,28 @@ TEST_F(KeywordProviderTest, Contents) {
     {ASCIIToUTF16("z    \t"),         1,
         {ASCIIToUTF16("Search z for <enter query>")}},
 
-    // Check that tokenization only collapses whitespace between first tokens
-    // and contents are not escaped or unescaped.
-    {ASCIIToUTF16("z   a   b   c++"), 1,
-        {ASCIIToUTF16("Search z for a   b   c++")}},
-    {ASCIIToUTF16("www.www www"),     1, {ASCIIToUTF16("Search www for www")}},
+    // Exact keyword matches with remaining text should return nothing.
+    {ASCIIToUTF16("www.www www"),     0, {}},
+    {ASCIIToUTF16("z   a   b   c++"), 0, {}},
+
+    // Exact keyword matches with remaining text when the keyword is an
+    // extension keyword should return something.  This is tested in
+    // chrome/browser/extensions/api/omnibox/omnibox_apitest.cc's
+    // in OmniboxApiTest's Basic test.
 
     // Substitution should work with various locations of the "%s".
     {ASCIIToUTF16("aaa"),             2,
         {ASCIIToUTF16("Search aaaa for <enter query>"),
          ASCIIToUTF16("Search aaaaa for <enter query>")}},
-    {ASCIIToUTF16("a 1 2 3"),         3, {ASCIIToUTF16("Search aa for 1 2 3"),
-                                          ASCIIToUTF16("Search ab for 1 2 3"),
-                                        ASCIIToUTF16("Search aaaa for 1 2 3")}},
-    {ASCIIToUTF16("www.w w"),         2, {ASCIIToUTF16("Search www for w"),
-        ASCIIToUTF16("Search weasel for w")}},
+    {ASCIIToUTF16("www.w w"),         2,
+        {ASCIIToUTF16("Search www for w"),
+         ASCIIToUTF16("Search weasel for w")}},
+    // Also, check that tokenization only collapses whitespace between first
+    // tokens and contents are not escaped or unescaped.
+    {ASCIIToUTF16("a   1 2+ 3"),      3,
+        {ASCIIToUTF16("Search aa for 1 2+ 3"),
+         ASCIIToUTF16("Search ab for 1 2+ 3"),
+         ASCIIToUTF16("Search aaaa for 1 2+ 3")}},
   };
 
   RunTest<string16>(contents_cases, arraysize(contents_cases),
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 7f72b8f..cf82bfe 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -786,7 +786,6 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
   int did_not_accept_keyword_suggestion = keyword_suggest_results_.empty() ?
       TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
       TemplateURLRef::NO_SUGGESTION_CHOSEN;
-  // Keyword what you typed results are handled by the KeywordProvider.
 
   int verbatim_relevance = GetVerbatimRelevance();
   int did_not_accept_default_suggestion = default_suggest_results_.empty() ?
@@ -797,7 +796,23 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
                   did_not_accept_default_suggestion, false, &map);
   }
-  const size_t what_you_typed_size = map.size();
+  if (!keyword_input_.text().empty()) {
+    const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
+    // We only create the verbatim search query match for a keyword
+    // if it's not an extension keyword.  Extension keywords are handled
+    // in KeywordProvider::Start().  (Extensions are complicated...)
+    // Note: in this provider, SEARCH_OTHER_ENGINE must correspond
+    // to the keyword verbatim search query.  Do not create other matches
+    // of type SEARCH_OTHER_ENGINE.
+    if (keyword_url && !keyword_url->IsExtensionKeyword()) {
+      AddMatchToMap(keyword_input_.text(), keyword_input_.text(),
+                    CalculateRelevanceForKeywordVerbatim(
+                        input_.type(), input_.prefer_keyword()),
+                    AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                    did_not_accept_keyword_suggestion, true, &map);
+    }
+  }
+  const size_t verbatim_matches_size = map.size();
   if (!default_provider_suggestion_.text.empty() &&
       default_provider_suggestion_.type == INSTANT_SUGGESTION_SEARCH &&
       !input_.prevent_inline_autocomplete())
@@ -833,8 +848,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
   AddNavigationResultsToMatches(keyword_navigation_results_, true);
   AddNavigationResultsToMatches(default_navigation_results_, false);
 
-  // Allow an additional match for "what you typed" if it's present.
-  const size_t max_total_matches = kMaxMatches + what_you_typed_size;
+  // Allow additional match(es) for verbatim results if present.
+  const size_t max_total_matches = kMaxMatches + verbatim_matches_size;
   std::partial_sort(matches_.begin(),
       matches_.begin() + std::min(max_total_matches, matches_.size()),
       matches_.end(), &AutocompleteMatch::MoreRelevant);
@@ -868,12 +883,17 @@ bool SearchProvider::IsTopMatchHighRankSearchForURL() const {
   return input_.type() == AutocompleteInput::URL &&
          matches_.front().relevance > CalculateRelevanceForVerbatim() &&
          (matches_.front().type == AutocompleteMatch::SEARCH_SUGGEST ||
-          matches_.front().type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED);
+          matches_.front().type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED ||
+          matches_.front().type == AutocompleteMatch::SEARCH_OTHER_ENGINE);
 }
 
 bool SearchProvider::IsTopMatchNotInlinable() const {
+  // Note: this test assumes the SEARCH_OTHER_ENGINE match corresponds to
+  // the verbatim search query on the keyword engine.  SearchProvider should
+  // not create any other match of type SEARCH_OTHER_ENGINE.
   return matches_.front().type != AutocompleteMatch::SEARCH_WHAT_YOU_TYPED &&
          matches_.front().type != AutocompleteMatch::URL_WHAT_YOU_TYPED &&
+         matches_.front().type != AutocompleteMatch::SEARCH_OTHER_ENGINE &&
          matches_.front().inline_autocomplete_offset == string16::npos &&
          matches_.front().fill_into_edit != input_.text();
 }
@@ -905,10 +925,11 @@ void SearchProvider::UpdateMatches() {
       ConvertResultsToAutocompleteMatches();
     }
     if (IsTopMatchNotInlinable()) {
-      // Disregard suggested relevances if the top match is not SWYT, inlinable,
-      // or URL_WHAT_YOU_TYPED (which may be top match regardless of inlining).
-      // For example, input "foo" should not invoke a search for "bar", which
-      // would happen if the "bar" search match outranked all other matches.
+      // Disregard suggested relevances if the top match is not a verbatim
+      // match, inlinable, or URL_WHAT_YOU_TYPED (which may be top match
+      // regardless of inlining).  For example, input "foo" should not
+      // invoke a search for "bar", which would happen if the "bar" search
+      // match outranked all other matches.
       ApplyCalculatedRelevance();
       ConvertResultsToAutocompleteMatches();
     }
@@ -1095,6 +1116,25 @@ int SearchProvider::CalculateRelevanceForVerbatim() const {
   }
 }
 
+// static
+int SearchProvider::CalculateRelevanceForKeywordVerbatim(
+    AutocompleteInput::Type type,
+    bool prefer_keyword) {
+  // This function is responsible for scoring verbatim query matches
+  // for non-extension keywords.  KeywordProvider::CalculateRelevance()
+  // scores verbatim query matches for extension keywords, as well as
+  // for keyword matches (i.e., suggestions of a keyword itself, not a
+  // suggestion of a query on a keyword search engine).  These two
+  // functions are currently in sync, but there's no reason we
+  // couldn't decide in the future to score verbatim matches
+  // differently for extension and non-extension keywords.  If you
+  // make such a change, however, you should update this comment to
+  // describe it, so it's clear why the functions diverge.
+  if (prefer_keyword)
+    return 1500;
+  return (type == AutocompleteInput::QUERY) ? 1450 : 1100;
+}
+
 int SearchProvider::CalculateRelevanceForHistory(
     const Time& time,
     bool is_keyword,
diff --git a/chrome/browser/autocomplete/search_provider.h b/chrome/browser/autocomplete/search_provider.h
index 0a0059a..d30e87b 100644
--- a/chrome/browser/autocomplete/search_provider.h
+++ b/chrome/browser/autocomplete/search_provider.h
@@ -3,9 +3,9 @@
 // found in the LICENSE file.
 //
 // This file contains the Search autocomplete provider.  This provider is
-// responsible for all non-keyword autocomplete entries that start with
-// "Search <engine> for ...", including searching for the current input string,
-// search history, and search suggestions.  An instance of it gets created and
+// responsible for all autocomplete entries that start with "Search <engine>
+// for ...", including searching for the current input string, search
+// history, and search suggestions.  An instance of it gets created and
 // managed by the autocomplete controller.
 
 #ifndef CHROME_BROWSER_AUTOCOMPLETE_SEARCH_PROVIDER_H_
@@ -288,11 +288,15 @@ class SearchProvider : public AutocompleteProvider,
                               bool is_keyword,
                               MatchMap* map);
 
-  // Get the relevance score for the verbatim result; this value may be provided
-  // by the suggest server; otherwise it is calculated locally.
+  // Gets the relevance score for the verbatim result; this value may be
+  // provided by the suggest server; otherwise it is calculated locally.
   int GetVerbatimRelevance() const;
-  // Calculate the relevance score for the verbatim result.
+  // Calculates the relevance score for the verbatim result.
   int CalculateRelevanceForVerbatim() const;
+  // Calculates the relevance score for the keyword verbatim result (if the
+  // input matches one of the profile's keyword).
+  static int CalculateRelevanceForKeywordVerbatim(AutocompleteInput::Type type,
+                                                  bool prefer_keyword);
   // |time| is the time at which this query was last seen.  |is_keyword|
   // indicates whether the results correspond to the keyword provider or default
   // provider. |prevent_inline_autocomplete| is true if we should not inline
@@ -300,10 +304,10 @@ class SearchProvider : public AutocompleteProvider,
   int CalculateRelevanceForHistory(const base::Time& time,
                                    bool is_keyword,
                                    bool prevent_inline_autocomplete) const;
-  // Calculate the relevance for search suggestion results. Set |for_keyword| to
-  // true for relevance values applicable to keyword provider results.
+  // Calculates the relevance for search suggestion results. Set |for_keyword|
+  // to true for relevance values applicable to keyword provider results.
   int CalculateRelevanceForSuggestion(bool for_keyword) const;
-  // Calculate the relevance for navigation results. Set |for_keyword| to true
+  // Calculates the relevance for navigation results. Set |for_keyword| to true
   // for relevance values applicable to keyword provider results.
   int CalculateRelevanceForNavigation(bool for_keyword) const;
 
diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc
index fa97663..1d17f81 100644
--- a/chrome/browser/autocomplete/search_provider_unittest.cc
+++ b/chrome/browser/autocomplete/search_provider_unittest.cc
@@ -68,6 +68,28 @@ class SearchProviderTest : public testing::Test,
 
   virtual void TearDown();
 
+  struct ResultInfo {
+    ResultInfo() : result_type(AutocompleteMatch::NUM_TYPES) {
+    }
+    ResultInfo(GURL gurl,
+               AutocompleteMatch::Type result_type,
+               string16 fill_into_edit)
+      : gurl(gurl),
+        result_type(result_type),
+        fill_into_edit(fill_into_edit) {
+    }
+    const GURL gurl;
+    const AutocompleteMatch::Type result_type;
+    const string16 fill_into_edit;
+  };
+  struct TestData {
+    const string16 input;
+    const size_t num_results;
+    const ResultInfo output[3];
+  };
+
+  void RunTest(TestData* cases, int num_cases, bool prefer_keyword);
+
  protected:
   // Adds a search for |term|, using the engine |t_url| to the history, and
   // returns the URL for that search.
@@ -270,6 +292,34 @@ void SearchProviderTest::TearDown() {
   provider_ = NULL;
 }
 
+void SearchProviderTest::RunTest(TestData* cases,
+                                 int num_cases,
+                                 bool prefer_keyword) {
+  ACMatches matches;
+  for (int i = 0; i < num_cases; ++i) {
+    AutocompleteInput input(cases[i].input, string16::npos, string16(),
+                            false, prefer_keyword, true,
+                            AutocompleteInput::ALL_MATCHES);
+    provider_->Start(input, false);
+    matches = provider_->matches();
+    string16 diagnostic_details = ASCIIToUTF16("Input was: ") + cases[i].input +
+        ASCIIToUTF16("; prefer_keyword was: ") +
+        (prefer_keyword ? ASCIIToUTF16("true") : ASCIIToUTF16("false"));
+    EXPECT_EQ(cases[i].num_results, matches.size()) << diagnostic_details;
+    if (matches.size() == cases[i].num_results) {
+      for (size_t j = 0; j < cases[i].num_results; ++j) {
+        EXPECT_EQ(cases[i].output[j].gurl, matches[j].destination_url) <<
+            diagnostic_details;
+        EXPECT_EQ(cases[i].output[j].result_type, matches[j].type) <<
+            diagnostic_details;
+        EXPECT_EQ(cases[i].output[j].fill_into_edit,
+                  matches[j].fill_into_edit) <<
+            diagnostic_details;
+      }
+    }
+  }
+}
+
 GURL SearchProviderTest::AddSearchToHistory(TemplateURL* t_url,
                                             string16 term,
                                             int visit_count) {
@@ -787,8 +837,9 @@ TEST_F(SearchProviderTest, InlineMixedCaseMatches) {
   EXPECT_EQ(ASCIIToUTF16("FOO"), term_match.fill_into_edit);
 }
 
-// Verifies AutocompleteControllers sets descriptions for results correctly.
-TEST_F(SearchProviderTest, UpdateKeywordDescriptions) {
+// Verifies AutocompleteControllers return results (including keyword
+// results) in the right order and set descriptions for them correctly.
+TEST_F(SearchProviderTest, KeywordOrderingAndDescriptions) {
   // Add an entry that corresponds to a keyword search with 'term2'.
   AddSearchToHistory(keyword_t_url_, ASCIIToUTF16("term2"), 1);
   profile_.BlockUntilHistoryProcessesPendingRequests();
@@ -800,16 +851,114 @@ TEST_F(SearchProviderTest, UpdateKeywordDescriptions) {
       AutocompleteInput::ALL_MATCHES));
   const AutocompleteResult& result = controller.result();
 
-  // There should be two matches, one for the keyword one for what you typed.
-  ASSERT_EQ(2u, result.size());
+  // There should be three matches, one for the keyword history, one for
+  // keyword provider's what-you-typed, and one for the default provider's
+  // what you typed, in that order.
+  ASSERT_EQ(3u, result.size());
+  EXPECT_EQ(AutocompleteMatch::SEARCH_HISTORY, result.match_at(0).type);
+  EXPECT_EQ(AutocompleteMatch::SEARCH_OTHER_ENGINE, result.match_at(1).type);
+  EXPECT_EQ(AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, result.match_at(2).type);
+  EXPECT_GT(result.match_at(0).relevance, result.match_at(1).relevance);
+  EXPECT_GT(result.match_at(1).relevance, result.match_at(2).relevance);
+
+  // The two keyword results should come with the keyword we expect.
+  EXPECT_EQ(ASCIIToUTF16("k"), result.match_at(0).keyword);
+  EXPECT_EQ(ASCIIToUTF16("k"), result.match_at(1).keyword);
+  // The default provider has a different keyword.  (We don't explicitly
+  // set it during this test, so all we do is assert that it's different.)
+  EXPECT_NE(result.match_at(0).keyword, result.match_at(2).keyword);
+
+  // The top result will always have a description.  The third result,
+  // coming from a different provider than the first two, should also.
+  // Whether the second result has one doesn't matter much.  (If it was
+  // missing, people would infer that it's the same search provider as
+  // the one above it.)
+  EXPECT_FALSE(result.match_at(0).description.empty());
+  EXPECT_FALSE(result.match_at(2).description.empty());
+  EXPECT_NE(result.match_at(0).description, result.match_at(2).description);
+}
 
-  EXPECT_FALSE(result.match_at(0).keyword.empty());
-  EXPECT_FALSE(result.match_at(1).keyword.empty());
-  EXPECT_NE(result.match_at(0).keyword, result.match_at(1).keyword);
+TEST_F(SearchProviderTest, KeywordVerbatim) {
+  TestData cases[] = {
+    // Test a simple keyword input.
+    { ASCIIToUTF16("k foo"), 2,
+      { ResultInfo(GURL("http://keyword/foo"),
+                   AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                   ASCIIToUTF16("k foo")),
+        ResultInfo(GURL("http://defaultturl/k%20foo"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("k foo") ) } },
+
+    // Make sure extra whitespace after the keyword doesn't change the
+    // keyword verbatim query.
+    { ASCIIToUTF16("k   foo"), 2,
+      { ResultInfo(GURL("http://keyword/foo"),
+                   AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                   ASCIIToUTF16("k foo")),
+        ResultInfo(GURL("http://defaultturl/k%20%20%20foo"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("k   foo")) } },
+    // Leading whitespace should be stripped before SearchProvider gets the
+    // input; hence there are no tests here about how it handles those inputs.
+
+    // But whitespace elsewhere in the query string should matter to both
+    // matches.
+    { ASCIIToUTF16("k  foo  bar"), 2,
+      { ResultInfo(GURL("http://keyword/foo%20%20bar"),
+                   AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                   ASCIIToUTF16("k foo  bar")),
+        ResultInfo(GURL("http://defaultturl/k%20%20foo%20%20bar"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("k  foo  bar")) } },
+    // Note in the above test case we don't test trailing whitespace because
+    // SearchProvider still doesn't handle this well.  See related bugs:
+    // 102690, 99239, 164635.
+
+    // Keywords can be prefixed by certain things that should get ignored
+    // when constructing the keyword match.
+    { ASCIIToUTF16("www.k foo"), 2,
+      { ResultInfo(GURL("http://keyword/foo"),
+                   AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                   ASCIIToUTF16("k foo")),
+        ResultInfo(GURL("http://defaultturl/www.k%20foo"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("www.k foo")) } },
+    { ASCIIToUTF16("http://k foo"), 2,
+      { ResultInfo(GURL("http://keyword/foo"),
+                   AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                   ASCIIToUTF16("k foo")),
+        ResultInfo(GURL("http://defaultturl/http%3A//k%20foo"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("http://k foo")) } },
+    { ASCIIToUTF16("http://www.k foo"), 2,
+      { ResultInfo(GURL("http://keyword/foo"),
+                   AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                   ASCIIToUTF16("k foo")),
+        ResultInfo(GURL("http://defaultturl/http%3A//www.k%20foo"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("http://www.k foo")) } },
+
+    // A keyword with no remaining input shouldn't get a keyword
+    // verbatim match.
+    { ASCIIToUTF16("k"), 1,
+      { ResultInfo(GURL("http://defaultturl/k"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("k")) } },
+    { ASCIIToUTF16("k "), 1,
+      { ResultInfo(GURL("http://defaultturl/k%20"),
+                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+                   ASCIIToUTF16("k ")) } }
+
+    // The fact that verbatim queries to keyword are handled by KeywordProvider
+    // not SearchProvider is tested in
+    // chrome/browser/extensions/api/omnibox/omnibox_apitest.cc.
+  };
 
-  EXPECT_FALSE(result.match_at(0).description.empty());
-  EXPECT_FALSE(result.match_at(1).description.empty());
-  EXPECT_NE(result.match_at(0).description, result.match_at(1).description);
+  // Test not in keyword mode.
+  RunTest(cases, arraysize(cases), false);
+
+  // Test in keyword mode.  (Both modes should give the same result.)
+  RunTest(cases, arraysize(cases), true);
 }
 
 // Verifies Navsuggest results don't set a TemplateURL, which instant relies on.
diff --git a/chrome/browser/extensions/api/omnibox/omnibox_apitest.cc b/chrome/browser/extensions/api/omnibox/omnibox_apitest.cc
index 22c2304..97a4ae8 100644
--- a/chrome/browser/extensions/api/omnibox/omnibox_apitest.cc
+++ b/chrome/browser/extensions/api/omnibox/omnibox_apitest.cc
@@ -63,8 +63,7 @@ class OmniboxApiTest : public ExtensionApiTest {
   }
 };
 
-// Flaky, http://crbug.com/167158 .
-IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DISABLED_Basic) {
+IN_PROC_BROWSER_TEST_F(OmniboxApiTest, Basic) {
   ASSERT_TRUE(RunExtensionTest("omnibox")) << message_;
 
   // The results depend on the TemplateURLService being loaded. Make sure it is
@@ -72,7 +71,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DISABLED_Basic) {
   ui_test_utils::WaitForTemplateURLServiceToLoad(
       TemplateURLServiceFactory::GetForProfile(browser()->profile()));
 
-  LocationBar* location_bar = GetLocationBar(browser());
   AutocompleteController* autocomplete_controller =
       GetAutocompleteController(browser());
 
@@ -114,15 +112,27 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DISABLED_Basic) {
     const AutocompleteResult& result = autocomplete_controller->result();
     ASSERT_EQ(5U, result.size()) << AutocompleteResultAsString(result);
 
-    ASSERT_FALSE(result.match_at(0).keyword.empty());
+    EXPECT_EQ(ASCIIToUTF16("keyword"), result.match_at(0).keyword);
     EXPECT_EQ(ASCIIToUTF16("keyword suggestio"),
               result.match_at(0).fill_into_edit);
+    EXPECT_EQ(AutocompleteMatch::SEARCH_OTHER_ENGINE, result.match_at(0).type);
+    EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
+              result.match_at(0).provider->type());
+    EXPECT_EQ(ASCIIToUTF16("keyword"), result.match_at(1).keyword);
     EXPECT_EQ(ASCIIToUTF16("keyword suggestion1"),
               result.match_at(1).fill_into_edit);
+    EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
+              result.match_at(1).provider->type());
+    EXPECT_EQ(ASCIIToUTF16("keyword"), result.match_at(2).keyword);
     EXPECT_EQ(ASCIIToUTF16("keyword suggestion2"),
               result.match_at(2).fill_into_edit);
+    EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
+              result.match_at(2).provider->type());
+    EXPECT_EQ(ASCIIToUTF16("keyword"), result.match_at(3).keyword);
     EXPECT_EQ(ASCIIToUTF16("keyword suggestion3"),
               result.match_at(3).fill_into_edit);
+    EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
+              result.match_at(3).provider->type());
 
     string16 description =
         ASCIIToUTF16("Description with style: <match>, [dim], (url till end)");
@@ -161,20 +171,26 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DISABLED_Basic) {
 
     AutocompleteMatch match = result.match_at(4);
     EXPECT_EQ(AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, match.type);
+    EXPECT_EQ(AutocompleteProvider::TYPE_SEARCH,
+              result.match_at(4).provider->type());
     EXPECT_FALSE(match.deletable);
   }
 
+  // Flaky, see http://crbug.com/167158
+  /*
   {
+    LocationBar* location_bar = GetLocationBar(browser());
     ResultCatcher catcher;
     OmniboxView* omnibox_view = location_bar->GetLocationEntry();
     omnibox_view->OnBeforePossibleChange();
-    omnibox_view->SetUserText( ASCIIToUTF16("keyword command"));
+    omnibox_view->SetUserText(ASCIIToUTF16("keyword command"));
     omnibox_view->OnAfterPossibleChange();
     location_bar->AcceptInput();
     // This checks that the keyword provider (via javascript)
     // gets told to navigate to the string "command".
     EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
   }
+  */
 }
 
 // Tests that the autocomplete popup doesn't reopen after accepting input for