Update omnibox dropdown selection/hover background drawing for MD.

Update omnibox dropdown selection/hover background drawing for MD.

Rather than use full-window-width selections, return to using omnibox-width
selections, but with a rounded corner to match the omnibox corner shape.

BUG=612758
TEST=Type in the omnibox.  The selected dropdown entry should have a selection background the same width as the omnibox, with the same rounded corners.

Committed: https://crrev.com/367d9021063deb29a90571768d9b4b822f2f50be
Cr-Commit-Position: refs/heads/master@{#400254}

[Peter Kasting, 2016-06-15 22:37:15]

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

[Evan Stade, 2016-06-15 23:56:45]

there's a slight behavioral change here that I would argue with, which is that
now when you hover the far left side of the dropdown, nothing gets selected. You
have to hover the actual text. Clicking in the rather wide margins does nothing
(doesn't even dismiss the bubble). As far as the visuals go, I have a slight
preference for the visual hover matching the functional hover, therefore full
width of the dropdown. But primarily I care about the functionality.

[Peter Kasting, 2016-06-16 00:52:48]

On 2016/06/15 23:56:45, Evan Stade wrote:
> there's a slight behavioral change here that I would argue with, which is that
> now when you hover the far left side of the dropdown, nothing gets selected.
You
> have to hover the actual text. Clicking in the rather wide margins does
nothing
> (doesn't even dismiss the bubble). As far as the visuals go, I have a slight
> preference for the visual hover matching the functional hover, therefore full
> width of the dropdown. But primarily I care about the functionality.

Sebastien and I actually discussed that before I started this change.  His call
was that matching the visible to the functional hover was more important than
letting people hover off to the sides of the text (which, arguably, actually
felt strange).  That this is also what the pre-MD behavior was for the last
couple years helps.

Personally I really don't like the wide dropdown, and now that we've killed
search term replacement, I think we should go back to a narrow one, but
Sebastien still liked the wide dropdown even with narrow selections inside.  :P

It's definitely not ideal, IMO.

[Evan Stade, 2016-06-16 16:11:32]

https://codereview.chromium.org/2070733002/diff/1/chrome/browser/ui/views/omn...
File chrome/browser/ui/views/omnibox/omnibox_result_view.cc (right):

https://codereview.chromium.org/2070733002/diff/1/chrome/browser/ui/views/omn...
chrome/browser/ui/views/omnibox/omnibox_result_view.cc:712: rect,
BackgroundWith1PxBorder::kCornerRadius * scale + 1, paint);
Would it be possible to set a bgwith1pxborder on |this|, making the border and
bg the same color?

[Peter Kasting, 2016-06-16 17:57:13]

https://codereview.chromium.org/2070733002/diff/1/chrome/browser/ui/views/omn...
File chrome/browser/ui/views/omnibox/omnibox_result_view.cc (right):

https://codereview.chromium.org/2070733002/diff/1/chrome/browser/ui/views/omn...
chrome/browser/ui/views/omnibox/omnibox_result_view.cc:712: rect,
BackgroundWith1PxBorder::kCornerRadius * scale + 1, paint);
On 2016/06/16 16:11:32, Evan Stade wrote:
> Would it be possible to set a bgwith1pxborder on |this|, making the border and
> bg the same color?

Yes, although doing it right took more effort than I expected.  I decided not to
expose a color-setting method on the background, choosing instead to recreate it
when the line state changes -- which corresponds almost exactly to when
Invalidate() gets called.  (Our caller takes care not to call us when nothing
has changed, e.g. the mouse moves around within the same line.)

But Invalidate() is only called for changes after initial creation; to get the
initial state right, we also need to hook SetMatch().  Turns out it wanted to do
a lot of the same things as Invalidate(), so I just made one call the other.

I also had to touch the popup model to have it update the hovered line before
invalidating.  It already did this with the selected line, so this seemed like a
good consistency change anyway.  I used the same name/comment style as in the
selected line code.

[Evan Stade, 2016-06-16 18:30:47]

lgtm

[Peter Kasting, 2016-06-16 20:07:17]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-06-16 20:07:49]

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

[commit-bot: I haz the power, 2016-06-16 20:56:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-16 20:59:15]

Description was changed from

==========
Update omnibox dropdown selection/hover background drawing for MD.

Rather than use full-window-width selections, return to using omnibox-width
selections, but with a rounded corner to match the omnibox corner shape.

BUG=612758
TEST=Type in the omnibox.  The selected dropdown entry should have a selection
background the same width as the omnibox, with the same rounded corners.
==========

to

==========
Update omnibox dropdown selection/hover background drawing for MD.

Rather than use full-window-width selections, return to using omnibox-width
selections, but with a rounded corner to match the omnibox corner shape.

BUG=612758
TEST=Type in the omnibox.  The selected dropdown entry should have a selection
background the same width as the omnibox, with the same rounded corners.

Committed: https://crrev.com/367d9021063deb29a90571768d9b4b822f2f50be
Cr-Commit-Position: refs/heads/master@{#400254}
==========

[commit-bot: I haz the power, 2016-06-16 20:59:16]

Patchset 2 (id:??) landed as
https://crrev.com/367d9021063deb29a90571768d9b4b822f2f50be
Cr-Commit-Position: refs/heads/master@{#400254}