InstantExtended: disallow setValue() without omnibox focus.

chrome/browser/ui/search/instant_controller.cc

Index: chrome/browser/ui/search/instant_controller.cc
diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc
index 106f474c4b88de454c189c22600685729be53317..58ebd398a06c7faffefe4aea5f895f709a26786d 100644
--- a/chrome/browser/ui/search/instant_controller.cc
+++ b/chrome/browser/ui/search/instant_controller.cc
@@ -1297,6 +1297,24 @@ void InstantController::SetSuggestions(
     return;
 
   if (suggestion.behavior == INSTANT_COMPLETE_REPLACE) {
+    if (omnibox_focus_state_ == OMNIBOX_FOCUS_NONE &&
+        !(last_suggestion_.behavior == INSTANT_COMPLETE_NEVER &&
+          !last_omnibox_text_has_inline_autocompletion_ &&
+          suggestion.text == last_omnibox_text_ + last_suggestion_.text)) {

[sreeram, 2013/05/28 22:45:02]

Does this work in the following case?
1. In Tab A, type something that gives you gray text.
2. Switch to another tab, do a bunch of queries or whatever.
3. Switch back to Tab A, click on the suggestion that matches the gray text.

[samarth, 2013/05/29 19:03:46]

I took out this check since we don't need to deal with gray text in M28 / M29. 
Now we just throw out setValue if the omnibox doesn't have focus.

+      // TODO(samarth,skanuj): setValue() needs to be handled differently when
+      // the omnibox doesn't have focus. Instead of setting temporary text, we
+      // should be setting search terms on the appopriate NavigationEntry.
+      // (Among other things, this ensures that URL-shaped values will get the
+      // additional security token.)
+      //
+      // However, today, we need to allow setValue() to happen in one particular
+      // case which is when a user clicks on the suggestion correponding to a
+      // gray-text completion. Otherwise, we can't distinguish between the user
+      // clicking on white space (where we don't accept the gray text) and the
+      // user clicking on the suggestion (when we do accept the gray text).
+      return;
+    }
+
     // We don't get an Update() when changing the omnibox due to a REPLACE
     // suggestion (so that we don't inadvertently cause the overlay to change
     // what it's showing, as the user arrows up/down through the page-provided
@@ -1382,7 +1400,9 @@ void InstantController::FocusOmnibox(const content::WebContents* contents,
   // doing nothing instead of crashing the browser process (intentional no-op).
   switch (state) {
     case OMNIBOX_FOCUS_VISIBLE:
-      browser_->FocusOmnibox(true);
+      // TODO(samarth): re-enable this once setValue() correctly handles
+      // URL-shaped queries.
+      // browser_->FocusOmnibox(true);
       break;
     case OMNIBOX_FOCUS_INVISIBLE:
       browser_->FocusOmnibox(false);