[Mac] Redesign the avatar bubble UI

[Mac] Redesign the avatar bubble UI. Again.

Mocks: https://docs.google.com/a/google.com/file/d/0BxMIIGI80eU-VW16NEloUFdyaXc/edit

This CL does not implement the "round" avatar icon, nor the top-left corner
info button, but it does set up the layout for the other elements.

There is also an intermediate flag now, --new-avatar-menu, which enables 
the new UI but not the mirror-specific features (such as account management 
or the user manager). In this menu we always display the fast user switcher.

BUG=353398
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265432

[noms (inactive), 2014-04-14 22:10:16]

Hi Hui,

Mind taking a brief look at this, before I send it out for review? Just to make
sure it still aligns with your future refactoring plans and everything :)

[guohui, 2014-04-15 13:51:13]

yup, it looks good with my refactoring plan, though there are some overlapping
changes between your CL and my CL under implementation, such as the check for
IsNewAvatarMenu instead of IsNewProfileManagement..

[noms (inactive), 2014-04-15 15:02:21]

Hi Rachel,

There are new mocks for the new avatar bubble, which means: brace yourself, new
avatar bubble CLs are coming :)

This first one implements most of the new mocks (link included), minus the round
avatar icon and the info icon in the top left corner. Please take a look.
Thanks!

[groby-ooo-7-16, 2014-04-16 18:50:37]

c/b/ui/cocoa LGTM w/ nits

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:592: SkColor
backgroundColor_;
You might want to use an NSColor so you don't have to color-convert on every
hover.

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1116: SEL
linkSelector = viewMode_ ==  BUBBLE_VIEW_MODE_PROFILE_CHOOSER ?
Would you mind parenthesizing the condition at least? Multiple equal signs on a
line confuse me :)

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1137: // This is
needed because the BlueLabelButton cell resets the style on
Maybe file a bug on BlueLabelButton for this - it shouldn't do that.

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1240:
gfx::SkColorToSRGBNSColor(profiles::kAvatarBubbleAccountsBackgroundColor);
Is there a reason this is an SRGBNSColor instead of a calibrated one?

[noms (inactive), 2014-04-16 19:43:35]

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (right):

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:592: SkColor
backgroundColor_;
On 2014/04/16 18:50:37, groby wrote:
> You might want to use an NSColor so you don't have to color-convert on every
> hover.

Done.

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1116: SEL
linkSelector = viewMode_ ==  BUBBLE_VIEW_MODE_PROFILE_CHOOSER ?
Absolutely! I prefer that, but have got comments about it before. Done.
On 2014/04/16 18:50:37, groby wrote:
> Would you mind parenthesizing the condition at least? Multiple equal signs on
a
> line confuse me :)

https://codereview.chromium.org/235833002/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:1240:
gfx::SkColorToSRGBNSColor(profiles::kAvatarBubbleAccountsBackgroundColor);
On 2014/04/16 18:50:37, groby wrote:
> Is there a reason this is an SRGBNSColor instead of a calibrated one?

Done.

[noms (inactive), 2014-04-16 19:46:14]

+ Elliot for c/b/profiles stamp
+ Scott for c/b/ui/views and c/common stamps. I've added my team to the OWNERS
of the profile_management_switches file because bcwhite@ isn't a committer, so
he can review, but not lgtm.

[sky, 2014-04-16 21:18:59]

LGTM

[Elliot Glaysher, 2014-04-16 21:24:48]

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:187: SkColorSetRGB(0xea,
0xea, 0xea);
Be careful here; bubble background varies between platforms. Is this safe?

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (left):

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:622:
ui::NativeTheme::kColorId_DialogBackground));
Like, you probably want to maintain this structure. If you have a specific color
that you want to use, you should make a kColorId and access that instead.

[noms (inactive), 2014-04-17 14:13:40]

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (left):

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:622:
ui::NativeTheme::kColorId_DialogBackground));
But If I make a specific ColorId, how is that different than just making a
constant? I assume it won't change on different themes -- i think in that menu,
we always want the button to have a grey hover background. I guess I've only
tested this on Mac and Windows, is Linux the theme that is different, or can
there be different themes for each platform?

The reason why here I had used the menuSeparatorColor is that initially I didn't
know what the exact colour of the hover should be (not because it should
actually be the same as the separator)...

On 2014/04/16 21:24:48, Elliot Glaysher wrote:
> Like, you probably want to maintain this structure. If you have a specific
color
> that you want to use, you should make a kColorId and access that instead.

[noms (inactive), 2014-04-17 15:06:01]

+ Roger for components/signin/core/common/profile_management_switches.cc stamp.

[Roger Tawa OOO till Jul 10th, 2014-04-17 15:16:42]

components/signin lgtm, nit about variable name

https://codereview.chromium.org/235833002/diff/200001/components/signin/core/...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/235833002/diff/200001/components/signin/core/...
components/signin/core/common/profile_management_switches.cc:53:
switches::kFastUserSwitching) || is_just_new_menu;
Rename variable maybe?
|is_just_new_menu| -> |use_mirror_promo_menu|?

[noms (inactive), 2014-04-17 15:20:09]

https://codereview.chromium.org/235833002/diff/200001/components/signin/core/...
File components/signin/core/common/profile_management_switches.cc (right):

https://codereview.chromium.org/235833002/diff/200001/components/signin/core/...
components/signin/core/common/profile_management_switches.cc:53:
switches::kFastUserSwitching) || is_just_new_menu;
On 2014/04/17 15:16:43, Roger Tawa wrote:
> Rename variable maybe?
> |is_just_new_menu| -> |use_mirror_promo_menu|?

Done.

[Elliot Glaysher, 2014-04-17 17:29:25]

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm (left):

https://codereview.chromium.org/235833002/diff/180001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm:622:
ui::NativeTheme::kColorId_DialogBackground));
On 2014/04/17 14:13:41, Monica Dinculescu wrote:
> But If I make a specific ColorId, how is that different than just making a
> constant? I assume it won't change on different themes -- i think in that
menu,
> we always want the button to have a grey hover background. I guess I've only
> tested this on Mac and Windows, is Linux the theme that is different, or can
> there be different themes for each platform?

There can be different NativeTheme subclasses compiled into the same platform.
The Linux theme (chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc) plumbs mostly
native system colors. We plumb the native dialog color through on Linux (even
though it's hardcoded white everywhere(?) else) and probably want to do the same
for something that interacts like a bubble.

[noms (inactive), 2014-04-22 17:06:55]

Hi Elliot,

I've done some things that are possibly very incorrect about converting the hard
coded colour to a kColorId. Is this sort of what you had in mind?

[Elliot Glaysher, 2014-04-22 17:55:10]

On 2014/04/22 17:06:55, Monica Dinculescu wrote:
> Hi Elliot,
> 
> I've done some things that are possibly very incorrect about converting the
hard
> coded colour to a kColorId. Is this sort of what you had in mind?

Yeah, this is what I had in mind. lgtm.

[noms (inactive), 2014-04-22 18:20:37]

Yay! Thanks!

[noms (inactive), 2014-04-22 18:20:47]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2014-04-22 18:21:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noms@chromium.org/235833002/260001

[commit-bot: I haz the power, 2014-04-22 19:29:16]

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

[commit-bot: I haz the power, 2014-04-22 19:29:16]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[noms (inactive), 2014-04-22 20:04:13]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2014-04-22 20:05:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noms@chromium.org/235833002/280001

[commit-bot: I haz the power, 2014-04-22 21:34:57]

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

[commit-bot: I haz the power, 2014-04-22 21:34:58]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_x64_rel

[noms (inactive), 2014-04-22 21:43:00]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2014-04-22 21:43:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noms@chromium.org/235833002/280001

[commit-bot: I haz the power, 2014-04-23 00:09:02]

Change committed as 265432