Fix a variety of omnibox bugs around zerosuggest, the escape key, or both.

Fix a variety of omnibox bugs around zerosuggest, the escape key, or both.

* When we changed to OmniboxViewViews, we broke the behavior that the omnibox
could handle the escape key before it was processed as a "stop" accelerator.
Instead, Chrome trunk current processes escape as stop first (and exclusively),
then if/when there is no load to stop passes it through to the omnibox.  Restore
the other order, though not precisely with the old behavior.  Other than when
arrowing through the popup (in which cases escape just resets to the top entry):
** Escape when not editing (including when zerosuggest is open) will always both
reset the omnibox and stop any underlying load.
** Otherwise, escape will reset the omnibox but not stop any load.

* As part of the above fix, we now ensure that escape will always close any open
zerosuggest window regardless of which entry in it might be selected or what the
omnibox caret/selection currently is.

* When unfocusing the omnibox causes a zerosuggest popup to close, we also force
the omnibox to reset.  This ensures that, if zerosuggest caused us to
temporarily avoid displaying updates to the underlying page URL, they will now
be shown.  When combined with the changes above, this should mean that it's
impossible to leave zerosuggest mode but still be stuck seeing an out-of-date
URL.

* When copying the text of any item in the zerosuggest popup, including the
first one, and regardless of whether the visible URL is up-to-date, ensure that
the clipboard actually gets the correct text.  Previously, select-all + copy on
any zerosuggest item would copy whatever the current underlying URL of the page
was.

BUG=158913, 423181, 466565
TEST=See bugs

Committed: https://crrev.com/e8585ee081a2b71c6a8af666be635de66b63f2f3
Cr-Commit-Position: refs/heads/master@{#324017}

[Peter Kasting, 2015-04-04 01:21:52]

pkasting@chromium.org changed reviewers:
+ hfung@chromium.org

[Peter Kasting, 2015-04-04 01:21:53]

This is sort of a conglomeration of several independent fixes, but I was working
on and testing them all at once, so...

[Mark P, 2015-04-06 18:18:19]

drive-by comment: can you fix and re-enable OmniboxViewTest.Escape (bug 158913)
in the process?  It looks like you're touching the same keypress handling code,
and the timing issue mentioned in that bug thread sounds related to the
loading-in-progress thing you mention in this changelist description.

--mark

[H Fung, 2015-04-07 00:38:23]

lgtm

https://codereview.chromium.org/1060613004/diff/20001/chrome/browser/ui/omnib...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/1060613004/diff/20001/chrome/browser/ui/omnib...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1034: // normal editing or
zerosuggest, is closed, and the full text is selected.
ZeroSuggest to be consistent with earlier comments?

https://codereview.chromium.org/1060613004/diff/20001/chrome/browser/ui/omnib...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1035: // This latter allows the
user to hit escape to quickly select all for ease of
This->The (I'm assuming you're referring to select all)

By replacement, you mean typing will clear the omnibox?

[Peter Kasting, 2015-04-07 02:18:08]

On 2015/04/06 18:18:19, Mark P wrote:
> drive-by comment: can you fix and re-enable OmniboxViewTest.Escape (bug
158913)
> in the process?  It looks like you're touching the same keypress handling
code,
> and the timing issue mentioned in that bug thread sounds related to the
> loading-in-progress thing you mention in this changelist description.

I re-enabled the test.  It passes locally and on the trybots, so fingers crossed
that it will work.

That said, I don't actually think the flakiness described in that bug is related
to what I'm touching.

[Peter Kasting, 2015-04-07 02:18:17]

https://codereview.chromium.org/1060613004/diff/20001/chrome/browser/ui/omnib...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

https://codereview.chromium.org/1060613004/diff/20001/chrome/browser/ui/omnib...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1034: // normal editing or
zerosuggest, is closed, and the full text is selected.
On 2015/04/07 00:38:23, H Fung wrote:
> ZeroSuggest to be consistent with earlier comments?

Done.

https://codereview.chromium.org/1060613004/diff/20001/chrome/browser/ui/omnib...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1035: // This latter allows the
user to hit escape to quickly select all for ease of
On 2015/04/07 00:38:23, H Fung wrote:
> This->The (I'm assuming you're referring to select all)

I tweaked the comment slightly to read better.

> By replacement, you mean typing will clear the omnibox?

Right, eliminating the omnibox text and replacing it with something different.

[Peter Kasting, 2015-04-07 02:18:22]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2015-04-07 02:18:23]

The patchset sent to the CQ was uploaded after l-g-t-m from hfung@chromium.org
Link to the patchset: https://codereview.chromium.org/1060613004/#ps40001
(title: "Comment fixes")

[Peter Kasting, 2015-04-07 02:18:31]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2015-04-07 02:18:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1060613004/40001

[commit-bot: I haz the power, 2015-04-07 02:18:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-07 02:18:45]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Peter Kasting, 2015-04-07 02:20:12]

pkasting@chromium.org changed reviewers:
+ mpearson@chromium.org

[Peter Kasting, 2015-04-07 02:20:28]

Mark, Harry's r+ isn't sufficient to land this.  Can you review?

[Mark P, 2015-04-07 05:25:55]

lgtm

[Peter Kasting, 2015-04-07 05:57:21]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2015-04-07 05:57:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1060613004/40001

[commit-bot: I haz the power, 2015-04-07 07:01:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-07 07:02:28]

Patchset 3 (id:??) landed as
https://crrev.com/e8585ee081a2b71c6a8af666be635de66b63f2f3
Cr-Commit-Position: refs/heads/master@{#324017}