When the new profile management flag is enabled, make sure to use the

When the new profile management flag is enabled, make sure to use the
new avatar menu button instead of the old.

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

[Roger Tawa OOO till Jul 10th, 2013-11-04 22:01:21]

Hi Monica,

Please take a look?  Thanks.

https://codereview.chromium.org/53153009/diff/1/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/1/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2582:
button->OnMouseReleased(mouse_event);
Is there a better way to programmatically trigger the button?

[noms (inactive), 2013-11-04 22:03:41]

https://codereview.chromium.org/53153009/diff/1/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/1/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2576: if
(CommandLine::ForCurrentProcess()->HasSwitch(
If browser_view can use things from profiles, you can use
profiles::IsNewProfileManagementEnabled() instead of manually checking the flag.

[Roger Tawa OOO till Jul 10th, 2013-11-04 22:10:50]

Thanks Monica.  Changes uploaded.

https://codereview.chromium.org/53153009/diff/1/chrome/browser/ui/views/frame...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/1/chrome/browser/ui/views/frame...
chrome/browser/ui/views/frame/browser_view.cc:2576: if
(CommandLine::ForCurrentProcess()->HasSwitch(
On 2013/11/04 22:03:42, Monica Dinculescu wrote:
> If browser_view can use things from profiles, you can use
> profiles::IsNewProfileManagementEnabled() instead of manually checking the
flag.

Done.

[noms (inactive), 2013-11-04 22:11:40]

lgtm

[Roger Tawa OOO till Jul 10th, 2013-11-04 22:12:15]

Hi Scott,

Can you please do an owner review?  Please note a question for you about
triggering the button.  Thanks.

[sky, 2013-11-04 22:18:52]

https://codereview.chromium.org/53153009/diff/60001/chrome/browser/ui/views/f...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/60001/chrome/browser/ui/views/f...
chrome/browser/ui/views/frame/browser_view.cc:2582:
button->OnMouseReleased(mouse_event);
We shouldn't be faking events like this to trigger the button.

[Roger Tawa OOO till Jul 10th, 2013-11-05 00:27:33]

Thanks Scott.  Comments addressed, changes uploade.  PTAL.

https://codereview.chromium.org/53153009/diff/60001/chrome/browser/ui/views/f...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/60001/chrome/browser/ui/views/f...
chrome/browser/ui/views/frame/browser_view.cc:2582:
button->OnMouseReleased(mouse_event);
On 2013/11/04 22:18:53, sky wrote:
> We shouldn't be faking events like this to trigger the button.

Done.

[sky, 2013-11-05 04:08:25]

https://codereview.chromium.org/53153009/diff/160001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.h (right):

https://codereview.chromium.org/53153009/diff/160001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view.h:64: void
ShowProfileChooserViewBubble();
Move implementation to match new position in header.

https://codereview.chromium.org/53153009/diff/160001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/160001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_view.cc:2578: BrowserNonClientFrameView*
frame_view =
Here you can do: frame_->GetFrameView()... And if you need it as a
BrowserNonClientFrameView, then you should change GetFrameView() to return a
BrowserNonClientFrameView.

[sky, 2013-11-05 15:56:12]

https://codereview.chromium.org/53153009/diff/270001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.h (right):

https://codereview.chromium.org/53153009/diff/270001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view.h:64: void
ShowProfileChooserViewBubble();
On second thought. I think you should move this to BrowserView and expose the
new_avatar_button.

[Roger Tawa OOO till Jul 10th, 2013-11-05 20:04:24]

Hi Scott,

Please take a look.  Thanks.

https://codereview.chromium.org/53153009/diff/160001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_non_client_frame_view.h (right):

https://codereview.chromium.org/53153009/diff/160001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_non_client_frame_view.h:64: void
ShowProfileChooserViewBubble();
On 2013/11/05 04:08:25, sky wrote:
> Move implementation to match new position in header.

Moved to browser view.

[sky, 2013-11-05 20:24:29]

https://codereview.chromium.org/53153009/diff/400001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/400001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_view.cc:2580: if (button) {
Under what conditions can we end up here and there be no button? Should this and
2591 be a DCHECK?

[Roger Tawa OOO till Jul 10th, 2013-11-05 20:53:12]

https://codereview.chromium.org/53153009/diff/400001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/53153009/diff/400001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_view.cc:2580: if (button) {
On 2013/11/05 20:24:29, sky wrote:
> Under what conditions can we end up here and there be no button? Should this
and
> 2591 be a DCHECK?

With the new profile management flag enabled, this could happen in an app
window, if this function is triggered from the web contents.  When we get app
support done this could be changed to a DCHECK.  For line 2591, the old code,
I'm not sure when it could be null.  I'd rather leave it as is.

[sky, 2013-11-05 22:26:18]

LGTM

[commit-bot: I haz the power, 2013-11-05 22:41:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerta@chromium.org/53153009/400001

[commit-bot: I haz the power, 2013-11-06 02:49:19]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 04:37:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerta@chromium.org/53153009/400001

[commit-bot: I haz the power, 2013-11-06 08:08:15]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 15:54:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerta@chromium.org/53153009/710001

[commit-bot: I haz the power, 2013-11-06 21:31:25]

Change committed as 233376