Thanks, I forgot to assign.  Question for implementation:

Is there a general preferred way to add hover effects to views that don't already have them?  Right now I am starting with OnMouseEntered/Exited to detect hover but keeping custom state seems more error-prone than simply hooking up an existing behavior (if one exists).  It looks like the new tab button uses an ink drop while tabs themselves use a custom glow hover controller, so I guess it's fine to write something new - probably based on gfx::SlideAnimation driving a color tween: not hard, but not a simple matter of wiring up a component as I first thought.

I think OnMouseMoved is what you want. See for example how it's used for the suggestion result view hover here:

https://cs.chromium.org/chromium/src/chrome/browser/ui/views/omnibox/omnibox_result_view.cc?l=284&rcl=c2199d8197dc81aaae7aae70bb6d5f0319fdabf2

Okay, transitioning with SlideAnimation works, but seems like unnecessary bulk and complexity.  How about just instantly switching background color on MouseEntered/Exited?  I like the fast & subtle hint.

Note, these screenshots are with backgrounds at GG200 and GG300, applied only on OmniboxViewViews, NOT the entire LocationBarView.  Let me know if we want it applied to the whole location bar, but isolating to the text field makes logical sense to me, even though it appears a bit starker.

Deleted: OmniboxViewViews_Background_kGoogleGrey200.png 10.7 KB

	OmniboxViewViews_Background_kGoogleGrey200.png 10.7 KB View Download

Deleted: OmniboxViewViews_Background_kGoogleGrey300.png 7.8 KB

	OmniboxViewViews_Background_kGoogleGrey300.png 7.8 KB View Download

Aha we were commenting at the same time.  Thanks, yes, OnMouseMoved will be great and that example shows it toggling local boolean hover state - so I will do likewise instead of the animation.

Went ahead and applied the effect to whole location bar using its existing RefreshBackground machinery, ran this by tommycli@ with GG200 and we agree it looks pretty good. :)  He says it's definitely supposed to be the whole location bar, not just the text field changing color. So I think it's done and I will upload a CL to start review.  If UX wants another shade of gray, it's an easy adjustment; I'm looking at overall approach first.

Deleted: LocationBar_NotHovered_kGoogleGrey100.png 10.2 KB

	LocationBar_NotHovered_kGoogleGrey100.png 10.2 KB View Download

Deleted: LocationBar_Hovered_kGoogleGrey200.png 10.2 KB

	LocationBar_Hovered_kGoogleGrey200.png 10.2 KB View Download

Deleted: LocationBar_Hovered_kGoogleGrey300.png 10.3 KB

	LocationBar_Hovered_kGoogleGrey300.png 10.3 KB View Download

Yeah, I'm not sure if an animation is required or not.

bettes: can you comment on that and choose a color? Or would you like us to just land one and then you can see how it looks in practice in Canary?

tommycli@ and I think the instant switch from GG100 to GG200 looks great.  But how about for dark color schemes?  In the code I found a dark hand-coded RGB color, so I adjusted it by about the same amount, going about +7 shades brighter in R,G,B.  If there are some designer-approved colors to use for dark theme, I am all for giving dark chrome more love!

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

commit b62aafe923b1c7b6ce4b0bbb5218aad27d149976
Author: Orin Jaworski <orinj@chromium.org>
Date: Wed Jun 20 17:38:02 2018

[omnibox] Add a hover effect to the steady omnibox

In Refresh, the steady state (unfocused) omnibox gets a hover
effect with a darker shaded background for the LocationBarView.

Bug:  853241 
Change-Id: I64859b5395c5a9eee6115c727bceec160b52dc39
Reviewed-on: https://chromium-review.googlesource.com/1103547
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568905}
[modify] https://crrev.com/b62aafe923b1c7b6ce4b0bbb5218aad27d149976/chrome/browser/ui/omnibox/omnibox_theme.cc
[modify] https://crrev.com/b62aafe923b1c7b6ce4b0bbb5218aad27d149976/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/b62aafe923b1c7b6ce4b0bbb5218aad27d149976/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/b62aafe923b1c7b6ce4b0bbb5218aad27d149976/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/b62aafe923b1c7b6ce4b0bbb5218aad27d149976/chrome/browser/ui/views/omnibox/omnibox_view_views.h
[modify] https://crrev.com/b62aafe923b1c7b6ce4b0bbb5218aad27d149976/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc

Hello orinij@: I have a question regarding the hover effect. Is it intended that the hover effect has no fade in/out timing? If you hover e.g. over the Chip or the Bookmarks Star you see that the hover has a slightly fade in/out timing effect. Thanks in advance :)

Attached a screencast that is showing the difference.

Deleted: Hover_Omnibox.mov 927 KB

	Hover_Omnibox.mov 927 KB View Download

Hi! :)  Yes this was intentional.  My first understanding of the bug was that we should transition with a fade, but then I simplified it to an instant switch for two reasons:

1) Speed & Simplicity principles: the switch is very light, fast, and subtle, requiring less code complexity and memory (no animation state to track).
2) Unlike the Chip and Star UI elements, which will be hovered and clicked less frequently, the location bar itself is likely to have mouse crossings often.  Knowing exactly when you can click into it is actually helpful.

These were my reasons, and I went with them because tommycli@ and I both thought it looked good.  They're not the only domains of concern, though, so if it needs a fade for other reasons, let me know and I'll be happy to give the animation a go.

Thanks for your feedback and the clarification :) I like the Speed & Simplicity way it is working now 👍. Thank you very much!

We try to make pretty much all effects in Chrome animated; I think a fast hover effect here would be beneficial.  The frequent mouse crossings mean that without an animation, the UI flickers rapidly as you mouse to and away from the tab strip, which is worrisome.  I suggest a 200 ms timing (which matches the current background tab hover timing), but maybe something different is called for, and one of the UX motion designers would probably have input if you asked.

I'll let you file a bug on this if you agree.

Here's how one could implement an animation like that in the code:

https://chromium-review.googlesource.com/c/chromium/src/+/1081198

I do agree, but I see pros & cons both ways.  How many shades of gray does it take to turn the pro of instant feedback into the con of blink annoyance?  How many milliseconds can we take to smooth the transition before the original intent of usability is compromised?

Seems like a balance to be struck by UX, so I will ask bklmn@ to look this over and provide a transition spec with tween type and duration.  Thanks for the feedback, all. :)

Thanks for the attention to detail here. Id say to start, add this transition to match the background tab hover motion would be great. Ill add helenepark@ to weigh in on timing. 

Spoke to Joel offline, 200ms for a bg tab hover transition sounds good to the both of us

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

commit 8723a57ae1cc311716188900318290d166ef523b
Author: Orin Jaworski <orinj@chromium.org>
Date: Sat Jun 30 01:24:55 2018

[omnibox] Animate background color on hover

When the steady omnibox is hovered, the location bar
smoothly animates to a slightly different shade
of gray instead of switching instantly.

Bug:  853241 
Change-Id: I5c51607a602f7e7245dddd46060d6dc25f4f4581
Reviewed-on: https://chromium-review.googlesource.com/1119494
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571733}
[modify] https://crrev.com/8723a57ae1cc311716188900318290d166ef523b/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/8723a57ae1cc311716188900318290d166ef523b/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/8723a57ae1cc311716188900318290d166ef523b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/8723a57ae1cc311716188900318290d166ef523b/chrome/browser/ui/views/omnibox/omnibox_view_views.h
[modify] https://crrev.com/8723a57ae1cc311716188900318290d166ef523b/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc