Posted some concerns on the CL here ( https://chromium-review.googlesource.com/c/527877/ ).

After some local exploration, here's my idea of how to implement this:

* Rip the concept of the hovered line out of OmniboxPopupModel entirely.  It doesn't need to care about this.  Individual views can do hot-tracking themselves.

* Create some kind of class (call it OmniboxDropdownElement for now) to represent "a hoverable, clickable region in the omnibox dropdown".  This might be a button subclass of some sort or it might just be a plain view.  This class will implement things like the light blue round rect hover effect and dark blue round rect click effect.  The base class should probably be able to display an icon, which should auto-flip for RTL.  See the bits OmniboxResultView currently sets on the keyword icon.

* Make OmniboxResultView extend OmniboxDropdownElement and add the functionality needed to draw contents, descriptions, and answers.

* Rip the "associated keyword" stuff (extra contents/description, animations) out of OmniboxResultView.  Instead, OmniboxPopupContentsView will be a grid (or a vertical box of horizontal boxes, one of the two) where each row is an OmniboxResultView, an OmniboxDropdownElement (for the keyword '>'), and another OmniboxResultView.  To animate between states, we animate the size of the leading OmniboxResultView and set the layout manager flex so the trailing result view grows to take up the remaining space.

* The OmniboxResultView used to show the associated keyword (i.e. the "rightmost" one) should never show an icon.  The OmniboxDropdownElement showing the '>' will be positioned in place of its icon.

* Clicks on the various views can be wired to do different things; OmniboxResultViews can still call OmniboxPopupContentsView::OpenSelectedLine(), while clicking the keyword '>'s can maybe call OmniboxEditModel::AcceptKeyword() or the like (some plumbing reworking may be needed here; right now this updates the view state and also updates the popup model, but maybe instead we can make the updates to both view and popup happen from some common method on the omnibox controller, which both places can trigger in response to keypress/mouse events).

* When later we need to add the diagonal arrows, they become just another OmniboxDropdownElement in the grid, wired to do Yet A Different Thing.

Behaviorally, this would mean these icons are now completely separate hover/selection regions, which do not appear hovered/selected at the same time as the rest of the row.  This can initially look a bit surprising, but I think it's actually OK.  I attached a pair of screenshots comparing today's behavior hovering a line that has one of these icons with what the new behavior would look like when hovering the same line.  If the user hovered the '>', then only that (and not the rest of the line) would appear hovered.  Same for the selected state, if the user clicked-and-held the mouse without releasing (these items activate on mouseup but change their visible state on mousedown).

This is a little more interesting when we've animated into the keyword state, since the "selection gap" will now be on the left rather than the right.  Again, I think this is OK.

Now, if we assume this is a good plan (which may not be true! but let's leave that aside), is there a way to get there from here that's not just "implement all the above"?  I don't think so.  The designs on the current CL go in a pretty different direction from the above and implement different behavior.  I think if we land one of them, we just end up reverting it entirely to implement the above.  Meanwhile, if we do the above, I don't see an obvious way to implement only part of the change initially.  And I don't think the above is actually so much stuff that it would take a tremendously long time to implement.  However, it is more complex than I originally thought when I figured on just moving a few views around and having this done in a day.  This would take at least a couple of days to do and maybe more like a week.  On the plus side, it would also lay most of the groundwork for adding the diagonal arrows as well.

I'll check with the omnibox team to figure out how we can get this done, unless Evan thinks the above is both specific enough and reasonable enough that he wants to tackle it.  In the meantime, feedback welcome.

Deleted: current hover.png 36.1 KB

	current hover.png 36.1 KB View Download

Deleted: line hover.png 36.1 KB

	line hover.png 36.1 KB View Download

This is a larger rewrite of the omnibox than I have time for before I head off on vacation next week.

As far as the above plan goes, while it sounds fine, I don't think it will happen in a week, in part because "Rip the concept of the hovered line out of OmniboxPopupModel entirely" affects cocoa, views, and probably mobile platforms. Even if the current design is not what I would have created if I were starting now, it's served us well enough in the past and if it isn't broken, fixing it is likely to create new bugs and will definitely be time consuming. This is why the CL linked above, in earlier iterations, worked within this existing framework in a way that I thought was not unreasonable, nor would it make any potential future refactoring like the one proposed by Peter any harder to do. And as Peter points out, it also lays the groundwork for the NW arrow change.

pkasting and I and some other omnibox folks are discussing the best way forward to get this desired functionality in while also improving the architecture of this code in a sound way. One of us will report back here by the end of the day.

I'm going to have Justin be the owner-of-record here, but I'm still actively involved.

Evan is right to note that ripping out the hovered line bit affects other platforms -- but then, the whole idea of making this clickable across platforms affects other platforms, so we kinda can't get away from that.  I doubt this affects mobile, since mobile rolls all its own UI; this is probably just Mac and Views (but I didn't check to be sure).

I think the current model is "broken" in terms of the desired ability to make more things on each line individually clickable.  The design was built for what we used to need, and what we need now is different.

As I noted above, I think Evan and I have different assessments on the value of his other designs here if we were to decide to move toward this world.  I don't think they incrementally move toward this, and I think they would need to be reverted to effectively do this.  That said, it wouldn't be terribly hard to revert them if not much had been built on top of them.  So they don't make this design much harder to implement, but they certainly don't look to me like they make it easier.

Hwi and I chatted for an hour today.  We concluded that there are no really great answers :).

We decided to do the following:
* Potentially switch the '>' icon for a magnifying glass -- Hwi needs to mock this and see how it looks before we do it, though.
* Make both the activation and indication areas for these icons extend all the way to the edges of the row.  My crude mock attached.
* Make the icon hoverable/clickable only when it's on the right, not when it's on the left.
* Leave full-row hover/selection in place, and make the hover affordance for these icons be 0.2a (luma-flip the dropdown background color); make the selection affordance 0.5a.  I invented most of these specifics just now based off Hwi's general description, so this is subject to tweaking when testing.

Based on the above, I think a grid of views still makes sense to implement (we effectively have 1-3 adjacent buttons per row with no overlap), but I'm not 100% clear on how we make the other views in a row appear hovered when one of them is.

Oops, I didn't attach things.

Here are the button hover and select states atop the row hover and select states, respectively.  The select ones look a little dark to me but since they'll only appear briefly maybe it's OK.  Or maybe they should be 0.4a instead.

Looking at this, we'll want to implement this like "draw the rows as horizontal, and apply a roundrect clip mask after everything else is done".  For the backgrounds, maybe we still have a row class that handles the whole row, but it mostly serves to host the 4 individual components (left match, diagonal arrow, keyword indicator, right match) and draw the common hover/selection background colors behind them.  Then the individual pieces are buttons that have optional hot/selection colors that are semitransparent.

Deleted: hover over hover.png 36.2 KB

	hover over hover.png 36.2 KB View Download

Deleted: select over hover.png 36.2 KB

	select over hover.png 36.2 KB View Download

Deleted: hover over select.png 33.3 KB

	hover over select.png 33.3 KB View Download

Deleted: select over select.png 33.3 KB

	select over select.png 33.3 KB View Download

Update after some further offline discussion. Because this requires a bit of a rework of the code to support the independent hover and click/tap behaviors in the latest spec, someone from the LAX omnibox team will take over estade's CL (https://chromium-review.googlesource.com/c/527877/). The plan is to implement something like what pkasting describes in #9.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca

commit ce9d1bff96ff56dc75ea995d9202f9ef037f42ca
Author: Justin Donnelly <jdonnelly@chromium.org>
Date: Thu Aug 10 16:24:55 2017

Let omnibox result views manage their own mouse hover state.

Note: the popup model is used in the Cocoa and iOS code. However,
neither interacts in any way with the hover state so they are not
affected by this change.

All behavior should be identical with one exception. Hover state is
no longer canceled when the keyboard is used to change the selection.
This behavior could probably be restored, though it would require
additional plumbing.

However, I believe that maintaining the hover state is correct. When
the keyboard is used to change the selection state, the mouse pointer
is still visible and positioned over the hovered row. Preserving the
hover state is not surprising when you see it. Compare to the
behavior of the mouse pointer when positioned over a link or a form
field. The pointer changes to a hand or an i-beam. If you start using
the keyboard to interact with other parts of the page or the browser
UI, the pointer doesn't revert to an arrow.

Bug: 728192
Change-Id: I572a8c3470a789637d37064a242b82c5ab370774
Reviewed-on: https://chromium-review.googlesource.com/582198
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493417}
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/omnibox/omnibox_controller_unittest.cc
[add] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/omnibox/test_omnibox_client.cc
[add] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/omnibox/test_omnibox_client.h
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/views/omnibox/omnibox_result_view.h
[add] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/browser/ui/views/omnibox/omnibox_result_view_unittest.cc
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/chrome/test/BUILD.gn
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/components/omnibox/browser/omnibox_controller.cc
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/components/omnibox/browser/omnibox_popup_model.cc
[modify] https://crrev.com/ce9d1bff96ff56dc75ea995d9202f9ef037f42ca/components/omnibox/browser/omnibox_popup_model.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1f1bb311d82c510f49c19fabc67e81a75e5d6b0

commit b1f1bb311d82c510f49c19fabc67e81a75e5d6b0
Author: Justin Donnelly <jdonnelly@chromium.org>
Date: Mon Aug 28 16:10:37 2017

Cancel omnibox popup drag with SetMouseHandler instead of custom state.

The purpose of this change is to simplify drag handling in
OmniboxPopupContentsView.

Bug: 728192
Change-Id: Ia7177f327c7bf7869204a8ad009a52b5cd5fe2b7
Reviewed-on: https://chromium-review.googlesource.com/636029
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497769}
[modify] https://crrev.com/b1f1bb311d82c510f49c19fabc67e81a75e5d6b0/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/b1f1bb311d82c510f49c19fabc67e81a75e5d6b0/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca90521396b715ecdab29236f0a0c48f57529c63

commit ca90521396b715ecdab29236f0a0c48f57529c63
Author: Justin Donnelly <jdonnelly@chromium.org>
Date: Fri Sep 08 21:08:33 2017

Let omnibox result views handle thier own drag events.

This change on its own isn't intended to do much, though it does offer
a couple minor improvements:
- Restores hover effects during a middle-button drag and introduces
hover effects on right-button and multi-button drags.
- Adds tests to cover these behaviors.

The primary goal of this change is to prepare the result views for the
introduction of child view buttons (keyword search and copy text to
omnibox). Because the result views now manage all of their own hover and
selection events, they will be able to control how these events are
handled when the mouse is positioned over the child views.

Bug: 728192
Change-Id: I53c920f12b2dae8d9ed48fc75d05e13acf45a10e
Reviewed-on: https://chromium-review.googlesource.com/641573
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500662}
[modify] https://crrev.com/ca90521396b715ecdab29236f0a0c48f57529c63/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/ca90521396b715ecdab29236f0a0c48f57529c63/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h
[modify] https://crrev.com/ca90521396b715ecdab29236f0a0c48f57529c63/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/ca90521396b715ecdab29236f0a0c48f57529c63/chrome/browser/ui/views/omnibox/omnibox_result_view.h
[modify] https://crrev.com/ca90521396b715ecdab29236f0a0c48f57529c63/chrome/browser/ui/views/omnibox/omnibox_result_view_unittest.cc

Adding Target-66 based on internal tracker. 

Also is this feature only applicable to Windows and Chrome OS? 

This feature is on hold at the moment. It became less important as the indicator in the omnibox itself is becoming more prominent and is clickable/touchable. Also, this work was used as the foundation for a different button in this position ("switch to open tab") and we don't have a clear design on how we'd like to present multiple actions at the same time.

I'm going to leave this open as I believe it would might be worth adding. But it's low priority for now.