Fix search terms handling on commit

chrome/browser/instant/instant_controller.cc

Index: chrome/browser/instant/instant_controller.cc
diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index 50a394e99f0ddd137104d7da50172f7415ee8ffe..50da5b39c0be6523c30c20674dcbaa64fb887489 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -503,9 +503,15 @@ bool InstantController::CommitIfPossible(InstantCommitType type) {
     if (!google_util::IsInstantExtendedAPIGoogleSearchUrl(url) &&
         google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
                                        google_util::ALLOW_NON_STANDARD_PORTS)) {
+      // Hitting ENTER searches for what the user typed, so use
+      // last_omnibox_text_. Clicking on the overlay commits what is currently
+      // showing, so add in the gray text in that case.
+      std::string query(UTF16ToUTF8(last_omnibox_text_));
+      if (type != INSTANT_COMMIT_PRESSED_ENTER)

[Jered, 2012/12/17 17:14:25]

Does this work for a suggestion click?

[samarth, 2012/12/17 17:18:49]

Yes, see below.

+        query += UTF16ToUTF8(last_suggestion_.text);
       entry->SetVirtualURL(GURL(
           url + "#q=" +
-          net::EscapeQueryParamValue(UTF16ToUTF8(last_omnibox_text_), true)));
+          net::EscapeQueryParamValue(query, true)));
       chrome::search::SearchTabHelper::FromWebContents(preview)->
           NavigationEntryUpdated();
     }
@@ -684,10 +690,6 @@ void InstantController::SetSuggestions(
     const std::vector<InstantSuggestion>& suggestions) {
   DVLOG(1) << "SetSuggestions";
 
-  // Ignore if we are not currently accepting search suggestions.
-  if (!search_mode_.is_search_suggestions() || last_omnibox_text_.empty())
-    return;
-
   // Ignore if the message is from an unexpected source.
   if (instant_tab_) {
     if (instant_tab_->contents() != contents)
@@ -701,6 +703,20 @@ void InstantController::SetSuggestions(
   if (!suggestions.empty())
     suggestion = suggestions[0];
 
+  if (instant_tab_ && search_mode_.is_search_results() &&
+      suggestion.behavior == INSTANT_COMPLETE_REPLACE) {
+    // This means a committed page in state search called setValue(). We should

[Jered, 2012/12/17 17:14:25]

When does a committed results page call setValue() in this state?

[samarth, 2012/12/17 17:18:50]

The server calls setValue() when you click on a query suggestion (this was part
of the change to make clicking on query suggestions to work at all). The problem
though is that by the time the message is received, the page will be committed
as a search page and we ignore the message without this change.

At the moment, the search page should not call setValue() in any other case, but
in principle, there's no reason it can't.  Once things settle down, we should
rethink more broadly about how much second-guessing we want in Chrome for the
various API calls.

+    // update the omnibox to reflect what the search page says.
+    browser_->SetInstantSuggestion(suggestion);
+    // Don't update last_omnibox_text_ or last_suggestion_ since the user is not
+    // currently editing text in the omnibox.
+    return;
+  }
+
+  // Ignore if we are not currently accepting search suggestions.
+  if (!search_mode_.is_search_suggestions() || last_omnibox_text_.empty())
+    return;
+
   if (suggestion.behavior == INSTANT_COMPLETE_REPLACE) {
     // We don't get an Update() when changing the omnibox due to a REPLACE
     // suggestion (so that we don't inadvertently cause the preview to change