Omnibox: Make Exact Match Tab-to-Search Work

chrome/browser/autocomplete/autocomplete_controller.cc

 // Copyright (c) 2012 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_controller.h"
 
 #include <set>
 #include <string>
 
 #include "base/format_macros.h"
@@ skipping 486 matching lines @@
     last_time_default_match_changed_ = base::TimeTicks::Now();
 
   NotifyChanged(force_notify_default_match_changed || notify_default_match);
 }
 
 void AutocompleteController::UpdateAssociatedKeywords(
     AutocompleteResult* result) {
   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) {
     base::string16 keyword(
         match->GetSubstitutingExplicitlyInvokedKeyword(template_url_service_));
     if (!keyword.empty()) {
       keywords.insert(keyword);
       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();
     }
   }
 }
 
 void AutocompleteController::UpdateKeywordDescriptions(
     AutocompleteResult* result) {
   base::string16 last_keyword;
   for (AutocompleteResult::iterator i(result->begin()); i != result->end();
@@ skipping 111 matching lines @@
                         this, &AutocompleteController::ExpireCopiedEntries);
 }
 
 void AutocompleteController::StartStopTimer() {
   stop_timer_.Start(FROM_HERE,
                     stop_timer_duration_,
                     base::Bind(&AutocompleteController::Stop,
                                base::Unretained(this),
                                false));
 }