Omnibox: Make Exact Match Tab-to-Search Work

chrome/browser/autocomplete/autocomplete_controller.cc

Index: chrome/browser/autocomplete/autocomplete_controller.cc
diff --git a/chrome/browser/autocomplete/autocomplete_controller.cc b/chrome/browser/autocomplete/autocomplete_controller.cc
index f7771443dc70cf78a6874adce3062993c76f767c..19a87259bc37f91b440874fac01e847c1788f112 100644
--- a/chrome/browser/autocomplete/autocomplete_controller.cc
+++ b/chrome/browser/autocomplete/autocomplete_controller.cc
@@ -504,6 +504,10 @@ void AutocompleteController::UpdateAssociatedKeywords(
   if (!keyword_provider_)
     return;
 
+  // Determine if the user's input is an exact keyword match.
+  base::string16 exact_keyword = keyword_provider_->GetKeywordForText(
+      TemplateURLService::CleanUserInputKeyword(input_.text()));
+
   std::set<base::string16> keywords;
   for (ACMatches::iterator match(result->begin()); match != result->end();
        ++match) {
@@ -514,18 +518,31 @@ void AutocompleteController::UpdateAssociatedKeywords(
       continue;
     }
 
-    // Only add the keyword if the match does not have a duplicate keyword with
-    // a more relevant match.
+    // If the user has typed an exact keyword and we haven't yet associated
+    // this keyword with some match, ignore whatever keyword (if any) is
+    // already associated with this particular match and let tab-to-search
+    // work for this exact keyword.

[Peter Kasting, 2014/08/04 21:54:22]

Nit: This comment is a bit confusing.  How about this:

When the user has typed an exact keyword, we want tab-to-search on the default
match to select that keyword, even if the match inline-autocompletes to a
different keyword.  (This prevents inline autocompletions from blocking a user's
attempts to use an explicitly-set keyword of their own creation.)  So use
|exact_keyword| if it's available.

[Mark P, 2014/08/04 22:05:38]

Done.

+    if (!exact_keyword.empty() && !keywords.count(exact_keyword)) {
+      keywords.insert(exact_keyword);
+      match->associated_keyword.reset(new AutocompleteMatch(
+          keyword_provider_->CreateVerbatimMatch(exact_keyword,
+                                                 exact_keyword, input_)));
+      continue;
+    }
+
+    // Otherwise use the particular match's associated keyword, looking it up
+    // if necessary.
     keyword = match->associated_keyword.get() ?
         match->associated_keyword->keyword :
         keyword_provider_->GetKeywordForText(match->fill_into_edit);
+
+    // Only add the keyword if the match does not have a duplicate keyword with
+    // a more relevant match.
     if (!keyword.empty() && !keywords.count(keyword)) {
       keywords.insert(keyword);
-
-      if (!match->associated_keyword.get())

[Peter Kasting, 2014/08/04 21:54:22]

What does removing this conditional do?

[Mark P, 2014/08/04 22:05:38]

I was worried that the copy-old-match code will keep an old, inappropriate
associated_keyword.

For instance, suppose we have keywords "f" and "foo.com"
Suppose the user types "f" and get a match "foo.com".  Though naturally
associated with the latter keyword, we decide to associate it with the former
under the exact-match-trumps policy.
Now the user type o, yielding "fo".  This match may be preserved due to the
copy-old-matches logic.  I need to be sure to clobber the associated_keyword so
it no longer points to "f".  That's why I don't keep the value from before.

Actually, this explanation made me realize I'm doing this sligtly wrong. 
Revised code slightly.

BTW, I looked at the GetKeywordForText, new AutocompleteMatch, and
CreateVerbatimMatch functions.  They don't seem heavy-weight; I feel no qualms
about making them run a bit more often.

-        match->associated_keyword.reset(new AutocompleteMatch(
-            keyword_provider_->CreateVerbatimMatch(match->fill_into_edit,
-                                                   keyword, input_)));
+      match->associated_keyword.reset(new AutocompleteMatch(
+          keyword_provider_->CreateVerbatimMatch(match->fill_into_edit,
+                                                 keyword, input_)));
     } else {
       match->associated_keyword.reset();
     }