InstantExtended: disallow setValue() without omnibox focus.

InstantExtended: disallow setValue() without omnibox focus.

Don't allow the the page to call setValue() if the omnibox doesn't have focus.
This breaks the "clearing out gray text" usecase (when the user clicks on
the suggestion corresponding to a gray-text completion), but we aren't
worried about it because we are aiming for a launch without any gray text.
We'll re-enable setValue() in the general case when we handle it properly.

Also disable the focus() API to prevent a trivial workaround (where the page
focuses the omnibox and then calls setValue).

BUG=229193
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203035

[samarth, 2013-05-09 22:05:03]

I distilled this down to the minimal change we can make to prevent arbitrary
setValue calls, since we may have to merge this into M28.

I will add a test, but please take a look in the meantime.

Thanks,
Samarth

[sreeram, 2013-05-09 22:20:18]

There's still a race: User types something. Suggestions are showing. They
arrow-down and focus the page (by clicking on some whitespace). setValue() comes
back, but doesn't update the omnibox.

So now, the page is showing a different entry being highlighted than what's
showing in the omnibox. The race would be rare, of course, but the UI effect is
quite bad.

[samarth, 2013-05-16 17:47:08]

As discussed offline, I think we can live with the race condition in M28.  Also
disabled the focus() API call.

Thanks,
Samarth

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

https://codereview.chromium.org/15001020/diff/7001/chrome/browser/ui/search/i...
File chrome/browser/ui/search/instant_controller.cc (right):

https://codereview.chromium.org/15001020/diff/7001/chrome/browser/ui/search/i...
chrome/browser/ui/search/instant_controller.cc:1303: suggestion.text ==
last_omnibox_text_ + last_suggestion_.text)) {
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]

PTAL.

Thanks,
Samarth

https://codereview.chromium.org/15001020/diff/7001/chrome/browser/ui/search/i...
File chrome/browser/ui/search/instant_controller.cc (right):

https://codereview.chromium.org/15001020/diff/7001/chrome/browser/ui/search/i...
chrome/browser/ui/search/instant_controller.cc:1303: suggestion.text ==
last_omnibox_text_ + last_suggestion_.text)) {
On 2013/05/28 22:45:02, sreeram wrote:
> 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.

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.

[sreeram, 2013-05-29 19:36:22]

lgtm

https://codereview.chromium.org/15001020/diff/15001/chrome/browser/ui/search/...
File chrome/browser/ui/search/instant_controller.cc (right):

https://codereview.chromium.org/15001020/diff/15001/chrome/browser/ui/search/...
chrome/browser/ui/search/instant_controller.cc:1340: // should be setting search
terms on the appopriate NavigationEntry.
"appropriate"

[samarth, 2013-05-29 23:42:38]

Thanks for the review!

https://codereview.chromium.org/15001020/diff/15001/chrome/browser/ui/search/...
File chrome/browser/ui/search/instant_controller.cc (right):

https://codereview.chromium.org/15001020/diff/15001/chrome/browser/ui/search/...
chrome/browser/ui/search/instant_controller.cc:1340: // should be setting search
terms on the appopriate NavigationEntry.
On 2013/05/29 19:36:22, sreeram wrote:
> "appropriate"

Done.

[commit-bot: I haz the power, 2013-05-29 23:43:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/15001020/20001

[commit-bot: I haz the power, 2013-05-30 03:23:44]

Change committed as 203035