Update profile switcher button on Linux.

Update profile switcher button on Linux.

This is limited to Linux because it draws the avatar button to match
Linux caption buttons, but it can be enabled for custom themed browser
windows on Windows as well after we write a drawing routine that looks
appropriate next to those buttons.

1. Draw the button without the use of assets (scale better at larger scale factors).
2. Improve vertical centering of text.
3. Use ink drop highlight/ripple for interactions.
4. Keep active state going while profile switcher widget is open.
5. Apply same bg color as caption buttons.

BUG=591586

[Evan Stade, 2017-04-21 21:30:21]

Description was changed from

==========
Wip - update new avatar button

BUG=
==========

to

==========
Wip - update new avatar button

BUG=591586
==========

[Evan Stade, 2017-04-26 18:42:26]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 18:43:16]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Evan Stade, 2017-04-26 19:23:24]

Description was changed from

==========
Wip - update new avatar button

BUG=591586
==========

to

==========
Update profile switcher button on Linux.

This is limited to Linux because it draws the avatar button to match
Linux caption buttons, but it can be enabled for custom themed browser
windows on Windows as well after we write a drawing routine that looks
appropriate next to those buttons.

1. Draw the button without the use of assets (scale better at larger scale
factors).
2. Improve vertical centering of text.
3. Use ink drop highlight/ripple for interactions.
4. Keep active state going while profile switcher widget is open.
5. Apply same bg color as caption buttons.

BUG=591586
==========

[Evan Stade, 2017-04-26 19:23:24]

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

[Evan Stade, 2017-04-26 19:24:49]

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.cc (right):

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:205: return SK_ColorWHITE;
I stuck with white because the theme-colored background is drawn separately
(regardless of press state).

[Peter Kasting, 2017-04-27 02:13:18]

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.cc (right):

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:74: // There's a second,
light stroke that matches the fill bounds.
I'm a little worried about the radii of the corners not matching up precisely at
high zoom levels.  I'm also worried about the fact that the stroke width and the
insets have to match, but the code doesn't force this.

I wonder if it makes sense to do something like what we do for tabs, where we
use Op() to compute the fill by subtracting the stroke from the overall region. 
Something like:

  constexpr outer stroke width = 1
  constexpr inner stroke width = 1

  outer stroke region = GetLocalBounds
  fill region = (outer stroke region) inset by (outer stroke width)
  inner carveout region = (fill region) inset by (inner stroke width)

  fill path = round rect for (fill region)
  outer stroke path = (round rect for outer stroke region) - (fill path)
  inner stroke path = (fill path) - (round rect for inner carveout region)

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.cc:205: return SK_ColorWHITE;
On 2017/04/26 19:24:49, Evan Stade wrote:
> I stuck with white because the theme-colored background is drawn separately
> (regardless of press state).

Is the drop visible at all on a pure-white toolbar (or whatever causes the
theme-colored background to be white)?

I'd think we'd need to select a contrasting color as Bret did for the Win 10
custom titlebar.

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/new_avatar_button.h (right):

https://codereview.chromium.org/2832223003/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/new_avatar_button.h:54: void
OnWidgetClosing(views::Widget* widget) override;
Nit: Per recent chromium-dev discussion, make overrides have same visibility as
in original class

(Though in this case the best way to do that would be to also make the methods
above be public)

https://codereview.chromium.org/2832223003/diff/80001/ui/views/animation/ink_...
File ui/views/animation/ink_drop_mask.cc (right):

https://codereview.chromium.org/2832223003/diff/80001/ui/views/animation/ink_...
ui/views/animation/ink_drop_mask.cc:51: gfx::RectF bounds(layer()->bounds());
Nit: I think this should have stayed as = per
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Variab...

https://codereview.chromium.org/2832223003/diff/80001/ui/views/controls/butto...
File ui/views/controls/button/label_button.cc (right):

https://codereview.chromium.org/2832223003/diff/80001/ui/views/controls/butto...
ui/views/controls/button/label_button.cc:426:
InstallInkDropMask(ink_drop_layer);
It looks from code search like several other places called AddInkDropLayer()
besides just here.  Don't you need to modify them too?  (For example,
BubbleIconView)

[Evan Stade, 2017-04-27 20:28:52]

Thanks for looking. I'll come back to this and address your concerns after emx's
patch lands and I rebase on top of it.

https://codereview.chromium.org/2832223003/diff/80001/ui/views/animation/ink_...
File ui/views/animation/ink_drop_mask.cc (right):

https://codereview.chromium.org/2832223003/diff/80001/ui/views/animation/ink_...
ui/views/animation/ink_drop_mask.cc:51: gfx::RectF bounds(layer()->bounds());
On 2017/04/27 02:13:18, Peter Kasting wrote:
> Nit: I think this should have stayed as = per
>
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Variab...

can't do that for rect=>rectf

https://codereview.chromium.org/2832223003/diff/80001/ui/views/controls/butto...
File ui/views/controls/button/label_button.cc (right):

https://codereview.chromium.org/2832223003/diff/80001/ui/views/controls/butto...
ui/views/controls/button/label_button.cc:426:
InstallInkDropMask(ink_drop_layer);
On 2017/04/27 02:13:18, Peter Kasting wrote:
> It looks from code search like several other places called AddInkDropLayer()
> besides just here.  Don't you need to modify them too?  (For example,
> BubbleIconView)

They would need this if they were to use a mask, but I don't want to hunt them
all down. Adding this here doesn't make them any less (hypothetically) broken.

Removing the code but not adding back InstallInkDropMask() in InkDropHostView
was an oversight.