[Win] The new profiles UI: now at an immersive mode near you

[Win] The new profiles UI: now at an immersive mode near you

This includes
- actually drawing the button correctly in Metro/Win 8 CrOS/immersive mode
- fixing chrome://settings to display the multiple users box in these cases

Screenshots (including rtl and non-english characters YAY):
https://drive.google.com/open?id=0B1B1Up4p2NRMWmdhcklhUUtQVU0&authuser=1

BUG=422434
TEST=Start Chrome with --enable-new-avatar-menu. From the hot-dog menu,
relaunch Chrome in immersive mode. Everything should look like it does
in normal mode (avatar button on the right, tabs don't overlap it,
users are listed and can be edited in chrome://settings)

Committed: https://crrev.com/87ff53f5058cfdfb556092082cdbcb83869e9ed5
Cr-Commit-Position: refs/heads/master@{#300968}

[noms (inactive), 2014-10-21 15:13:42]

noms@chromium.org changed reviewers:
+ dbeam@chromium.org, msw@chromium.org

[noms (inactive), 2014-10-21 15:13:42]

The new avatar button wasn't being drawn correctly in Metro/immersive mode. This
CL fixes that. Please take a look.

msw: c/b/ui/views/frame/* and c/b/ui/views/profiles/*
dbeam: c/b/ui/webui/*

Thanks!

[Dan Beam, 2014-10-21 17:38:54]

c/b/ui/webui

https://codereview.chromium.org/668773002/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/668773002/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/browser_options_handler.cc:1044: desktop_type !=
chrome::HOST_DESKTOP_TYPE_ASH)
curlies

[Dan Beam, 2014-10-21 17:39:03]

... lgtm

[noms (inactive), 2014-10-21 18:49:59]

https://codereview.chromium.org/668773002/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/668773002/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/browser_options_handler.cc:1044: desktop_type !=
chrome::HOST_DESKTOP_TYPE_ASH)
On 2014/10/21 17:38:54, Dan Beam wrote:
> curlies

Done.

[msw, 2014-10-21 19:54:38]

I only see one screenshot, and the image there looks too high and too far left
within the highlighted button area...

You should probably begin seeking more comprehensive initial review from someone
other than myself, but I'll still help out as needed.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc (right):

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:60: // How
far the new avatar button is from the left of the minimize button.
nit: the comment should match the surrounding pattern and try to avoid saying
left/right when RTL UI flips those directions. Consider: "Space between the new
avatar button and the minimize button."

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:464:
chrome::ShowAvatarMenu(browser_view()->browser());
nit: use IDC_SHOW_AVATAR_MENU to match the abstracted command pattern above.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:467: 
nit: remove blank line

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:583:
caption_button_container_->GetPreferredSize().height());
nit: do you want the preferred height or the actual height?
(it might not matter, or it might depend on layout ordering)

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:1043: if
(desktop_type != chrome::HOST_DESKTOP_TYPE_NATIVE &&
What? I don't think there is any other desktop type besides native or ash...
afaict, this if statement will never evaluate to true.

[noms (inactive), 2014-10-21 19:57:02]

Fixed the screenshot links. Sorry about that!

[noms (inactive), 2014-10-21 20:55:18]

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc (right):

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:60: // How
far the new avatar button is from the left of the minimize button.
On 2014/10/21 19:54:37, msw wrote:
> nit: the comment should match the surrounding pattern and try to avoid saying
> left/right when RTL UI flips those directions. Consider: "Space between the
new
> avatar button and the minimize button."

Done.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:464:
chrome::ShowAvatarMenu(browser_view()->browser());
On 2014/10/21 19:54:38, msw wrote:
> nit: use IDC_SHOW_AVATAR_MENU to match the abstracted command pattern above.

Done.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:467: 
On 2014/10/21 19:54:37, msw wrote:
> nit: remove blank line

Done.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:583:
caption_button_container_->GetPreferredSize().height());
On 2014/10/21 19:54:37, msw wrote:
> nit: do you want the preferred height or the actual height?
> (it might not matter, or it might depend on layout ordering)

The rest of the file uses preferred height, so I kept it as that to be
consistent. It also seems to look like the height I want.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:1043: if
(desktop_type != chrome::HOST_DESKTOP_TYPE_NATIVE &&
On 2014/10/21 19:54:38, msw wrote:
> What? I don't think there is any other desktop type besides native or ash...
> afaict, this if statement will never evaluate to true.

Oh, yeah, you're totally right. I misread that enum as having 4 values, but
that's a lie.

[msw, 2014-10-21 21:00:40]

Again, the screenshot with the little person image and no text looks misaligned.
Do you plan to address that in a followup CL? Otherwise, this lgtm.

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc (right):

https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:583:
caption_button_container_->GetPreferredSize().height());
On 2014/10/21 20:55:17, Monica Dinculescu wrote:
> On 2014/10/21 19:54:37, msw wrote:
> > nit: do you want the preferred height or the actual height?
> > (it might not matter, or it might depend on layout ordering)
> 
> The rest of the file uses preferred height, so I kept it as that to be
> consistent. It also seems to look like the height I want.

Acknowledged.

[noms (inactive), 2014-10-21 21:08:16]

On 2014/10/21 21:00:40, msw wrote:
> Again, the screenshot with the little person image and no text looks
misaligned.
> Do you plan to address that in a followup CL? Otherwise, this lgtm.
> 
>
https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
> File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
(right):
> 
>
https://codereview.chromium.org/668773002/diff/20001/chrome/browser/ui/views/...
> chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc:583:
> caption_button_container_->GetPreferredSize().height());
> On 2014/10/21 20:55:17, Monica Dinculescu wrote:
> > On 2014/10/21 19:54:37, msw wrote:
> > > nit: do you want the preferred height or the actual height?
> > > (it might not matter, or it might depend on layout ordering)
> > 
> > The rest of the file uses preferred height, so I kept it as that to be
> > consistent. It also seems to look like the height I want.
> 
> Acknowledged.

I'm working on fixing the little person icon in this CL.

[noms (inactive), 2014-10-23 20:20:45]

Annnnd centered the image horizontally. It's centered vertically, but not
aligned with the minimize icon. When I do align the bottom of the little hooman
to the bottom of the minimize button I find it looks (as it is) vertically very
uncentered, so I'll leave it like this for now.

[noms (inactive), 2014-10-23 20:33:47]

The CQ bit was checked by noms@chromium.org

[commit-bot: I haz the power, 2014-10-23 20:34:32]

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

[commit-bot: I haz the power, 2014-10-23 23:02:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-23 23:03:26]

Patchset 4 (id:??) landed as
https://crrev.com/87ff53f5058cfdfb556092082cdbcb83869e9ed5
Cr-Commit-Position: refs/heads/master@{#300968}