Multi-Profiles: Only show the avatar icon if user has multiple profiles

Multi-Profiles: Only show the avatar icon if user has multiple profiles

BUG=88878
TEST=None


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

[Robert Sesek, 2011-07-24 13:56:36]

Drive-by feedback

http://codereview.chromium.org/7484040/diff/1/chrome/browser/ui/cocoa/browser...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7484040/diff/1/chrome/browser/ui/cocoa/browser...
chrome/browser/ui/cocoa/browser_window_controller.mm:1326: -
(BOOL)shouldShowAvatar {
Use YES/NO return values rather than true/false.

[Miranda Callahan, 2011-07-25 12:01:54]

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

http://codereview.chromium.org/7484040/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:484: return
cache.GetNumberOfProfiles() > 1;
Seems like you only need this file, with the check (correctly, IMO) internalized
in the IsMultipleProfilesEnabled method -- the checking in the other two files
is, I assume, from an earlier version of this CL?

[sail, 2011-08-03 20:51:49]

http://codereview.chromium.org/7484040/diff/1/chrome/browser/ui/cocoa/browser...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7484040/diff/1/chrome/browser/ui/cocoa/browser...
chrome/browser/ui/cocoa/browser_window_controller.mm:1326: -
(BOOL)shouldShowAvatar {
On 2011/07/24 13:56:37, rsesek wrote:
> Use YES/NO return values rather than true/false.

Done.

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

http://codereview.chromium.org/7484040/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:484: return
cache.GetNumberOfProfiles() > 1;
On 2011/07/25 12:01:54, Miranda Callahan wrote:
> Seems like you only need this file, with the check (correctly, IMO)
internalized
> in the IsMultipleProfilesEnabled method -- the checking in the other two files
> is, I assume, from an earlier version of this CL?

This check can't be in IsMultipleProfilesEnabled.

IsMultipleProfilesEnabled disables the entire feature, including the DOM UI.

In this check, if multi-profiles is enabled we'll only show the avatar UI if the
user has more than one profile.

The other two (gtk/cocoa) also need to do this test.

[Robert Sesek, 2011-08-03 21:13:00]

Mac bits LGTM

http://codereview.chromium.org/7484040/diff/4001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7484040/diff/4001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:1364: // Show the profile
avatar after the user has create more than one profile.
createD?

[Elliot Glaysher, 2011-08-03 21:15:11]

LGTM rubber stamp since you already know this is broken.

[sail, 2011-08-03 21:17:14]

http://codereview.chromium.org/7484040/diff/4001/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/7484040/diff/4001/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.mm:1364: // Show the profile
avatar after the user has create more than one profile.
On 2011/08/03 21:13:00, rsesek wrote:
> createD?

Done.

[sail, 2011-08-03 21:17:36]

On 2011/08/03 21:17:14, sail wrote:
>
http://codereview.chromium.org/7484040/diff/4001/chrome/browser/ui/cocoa/brow...
> File chrome/browser/ui/cocoa/browser_window_controller.mm (right):
> 
>
http://codereview.chromium.org/7484040/diff/4001/chrome/browser/ui/cocoa/brow...
> chrome/browser/ui/cocoa/browser_window_controller.mm:1364: // Show the profile
> avatar after the user has create more than one profile.
> On 2011/08/03 21:13:00, rsesek wrote:
> > createD?
> 
> Done.

Yea, this broken on gtk but works on views and mac.

[commit-bot: I haz the power, 2011-08-04 01:15:16]

Change committed as 95362