Multi-Profile: Move multi-profile commands from wrench menu to avatar menu

Multi-Profile: Move multi-profile commands from wrench menu to avatar menu

This change moves the multi-profile commands from the wrench menu to the avatar menu.

To make multi-profiles usuable until the new options UI is ready I'm making the avatar always visible. I'll change it back once the options UI code is checked in.

BUG=
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91872

[Miranda Callahan, 2011-07-08 14:39:25]

LGTM with a couple nits and a question.

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
File chrome/browser/ui/profile_menu_model.cc (right):

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
chrome/browser/ui/profile_menu_model.cc:26: 
could you add a comment here that these are overridden from
SimpleMenuModel::Delegate?

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
chrome/browser/ui/profile_menu_model.cc:96:
delegate_->ExecuteCommand(command_id);
I wonder if you should make explicit that this only works because the only
command you're passing through to the delegate is an IDC command, with a very
high command id value -- if a later user ever adds any other command to the
menu, it will almost certainly interfere with / be stepped on the dynamically
created profile switching command ids.

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/views/frame/b...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:481: // show the avatar if hte
user has more than one profile.
nit: I think in the internet age all spellings of "teh" should be acceptable,
but others disagree.

[Robert Sesek, 2011-07-08 14:50:42]

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
File chrome/browser/ui/profile_menu_model.h (right):

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
chrome/browser/ui/profile_menu_model.h:21: explicit ProfileMenuModel(Browser*
browser);
Can Browser be NULL? I'm looking at the main menu implementation and I think it
needs to be possible for it to be so.

[sail, 2011-07-08 15:52:06]

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
File chrome/browser/ui/profile_menu_model.cc (right):

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
chrome/browser/ui/profile_menu_model.cc:26: 
On 2011/07/08 14:39:25, Miranda Callahan wrote:
> could you add a comment here that these are overridden from
> SimpleMenuModel::Delegate?

Done.

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
chrome/browser/ui/profile_menu_model.cc:96:
delegate_->ExecuteCommand(command_id);
On 2011/07/08 14:39:25, Miranda Callahan wrote:
> I wonder if you should make explicit that this only works because the only
> command you're passing through to the delegate is an IDC command, with a very
> high command id value -- if a later user ever adds any other command to the
> menu, it will almost certainly interfere with / be stepped on the dynamically
> created profile switching command ids. 

Good point. I fixed this by getting rid of IDC_CREATE_NEW_PROFILE and making it
an enum.

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/views/frame/b...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:481: // show the avatar if hte
user has more than one profile.
On 2011/07/08 14:39:25, Miranda Callahan wrote:
> nit: I think in the internet age all spellings of "teh" should be acceptable,
> but others disagree.

Done.

[sail, 2011-07-08 15:54:22]

On 2011/07/08 14:50:42, rsesek wrote:
>
http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
> File chrome/browser/ui/profile_menu_model.h (right):
> 
>
http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
> chrome/browser/ui/profile_menu_model.h:21: explicit ProfileMenuModel(Browser*
> browser);
> Can Browser be NULL? I'm looking at the main menu implementation and I think
it
> needs to be possible for it to be so.

Right now it can't but it shouldn't be hard to fix it so that it can. The main
thing this class needs is a profile. The browser parameter is nice so that we
can reuse the browser window when doing things like opening the customize
profile tab.

Mind if we do this as a separate change?

[Robert Sesek, 2011-07-08 16:00:21]

On 2011/07/08 15:54:22, sail wrote:
> On 2011/07/08 14:50:42, rsesek wrote:
> >
>
http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
> > File chrome/browser/ui/profile_menu_model.h (right):
> > 
> >
>
http://codereview.chromium.org/7328005/diff/1/chrome/browser/ui/profile_menu_...
> > chrome/browser/ui/profile_menu_model.h:21: explicit
ProfileMenuModel(Browser*
> > browser);
> > Can Browser be NULL? I'm looking at the main menu implementation and I think
> it
> > needs to be possible for it to be so.
> 
> Right now it can't but it shouldn't be hard to fix it so that it can. The main
> thing this class needs is a profile. The browser parameter is nice so that we
> can reuse the browser window when doing things like opening the customize
> profile tab.
> 
> Mind if we do this as a separate change?

That's fine.

[commit-bot: I haz the power, 2011-07-08 17:49:35]

Change committed as 91872