Commit 33508853 authored by noms@chromium.org's avatar noms@chromium.org

[Mac] Show a warning in the new avatar button/menu if there's an auth error.

This error is shown both in the avatar button, and in the avatar menu. When the 
account for which we're displaying the auth error is clicked in the avatar menu,
we show the Gaia reauth view.

Screenshot: 
https://drive.google.com/file/d/0B1B1Up4p2NRMQW90VmcwcGZQVEU/edit?usp=sharing

I've also fixed a bug where the available text for the account name was 
incorrectly calculated, and eliding overlapped the delete button. Screenshot:
https://drive.google.com/file/d/0B1B1Up4p2NRMaU9DcWdqTVZNMVk/edit?usp=sharing
BUG=311235
TEST= Sign into Chrome with the --new-profile-management flag on. Go to 
accounts.google.com and revoke Chrome's access to that account, from the 
Security tab. Try to bookmark the current page. The avatar button should be
badged like in the attached screenshot.

Review URL: https://codereview.chromium.org/303463010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277908 0039d316-1c4b-4281-b951-d872f2087c98
parent 7a46143e
......@@ -11,6 +11,7 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_metrics.h"
#include "chrome/browser/signin/signin_header_helper.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_window.h"
......@@ -18,6 +19,7 @@
#import "chrome/browser/ui/cocoa/browser_window_controller.h"
#import "chrome/browser/ui/cocoa/profiles/avatar_menu_bubble_controller.h"
#import "chrome/browser/ui/cocoa/profiles/profile_chooser_controller.h"
#include "components/signin/core/browser/signin_error_controller.h"
#include "components/signin/core/common/profile_management_switches.h"
#include "ui/base/resource/resource_bundle.h"
......@@ -27,50 +29,79 @@ const CGFloat kMenuYOffsetAdjust = 1.0;
@interface AvatarBaseController (Private)
// Shows the avatar bubble.
- (IBAction)buttonClicked:(id)sender;
- (void)bubbleWillClose:(NSNotification*)notif;
// Updates the profile name displayed by the avatar button. If |layoutParent| is
// yes, then the BrowserWindowController is notified to relayout the subviews,
// as the button needs to be repositioned.
- (void)updateAvatarButtonAndLayoutParent:(BOOL)layoutParent;
// Displays an error icon if any accounts associated with this profile have an
// auth error.
- (void)updateErrorStatus:(BOOL)hasError;
@end
class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver {
class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver,
public SigninErrorController::Observer {
public:
ProfileInfoUpdateObserver(AvatarBaseController* avatarButton)
: avatarButton_(avatarButton) {
ProfileInfoUpdateObserver(Profile* profile,
AvatarBaseController* avatarController)
: profile_(profile),
avatarController_(avatarController) {
g_browser_process->profile_manager()->
GetProfileInfoCache().AddObserver(this);
// Subscribe to authentication error changes so that the avatar button
// can update itself.
SigninErrorController* errorController =
profiles::GetSigninErrorController(profile_);
if (errorController)
errorController->AddObserver(this);
}
virtual ~ProfileInfoUpdateObserver() {
g_browser_process->profile_manager()->
GetProfileInfoCache().RemoveObserver(this);
SigninErrorController* errorController =
profiles::GetSigninErrorController(profile_);
if (errorController)
errorController->RemoveObserver(this);
}
// ProfileInfoCacheObserver:
virtual void OnProfileAdded(const base::FilePath& profile_path) OVERRIDE {
[avatarButton_ updateAvatarButtonAndLayoutParent:YES];
[avatarController_ updateAvatarButtonAndLayoutParent:YES];
}
virtual void OnProfileWasRemoved(
const base::FilePath& profile_path,
const base::string16& profile_name) OVERRIDE {
[avatarButton_ updateAvatarButtonAndLayoutParent:YES];
[avatarController_ updateAvatarButtonAndLayoutParent:YES];
}
virtual void OnProfileNameChanged(
const base::FilePath& profile_path,
const base::string16& old_profile_name) OVERRIDE {
[avatarButton_ updateAvatarButtonAndLayoutParent:YES];
[avatarController_ updateAvatarButtonAndLayoutParent:YES];
}
virtual void OnProfileAvatarChanged(
const base::FilePath& profile_path) OVERRIDE {
[avatarButton_ updateAvatarButtonAndLayoutParent:YES];
[avatarController_ updateAvatarButtonAndLayoutParent:YES];
}
// SigninErrorController::Observer:
virtual void OnErrorChanged() OVERRIDE {
SigninErrorController* errorController =
profiles::GetSigninErrorController(profile_);
if (errorController)
[avatarController_ updateErrorStatus:errorController->HasError()];
}
private:
AvatarBaseController* avatarButton_; // Weak; owns this.
Profile* profile_;
AvatarBaseController* avatarController_; // Weak; owns this.
DISALLOW_COPY_AND_ASSIGN(ProfileInfoUpdateObserver);
};
......@@ -80,7 +111,8 @@ class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver {
- (id)initWithBrowser:(Browser*)browser {
if ((self = [super init])) {
browser_ = browser;
profileInfoObserver_.reset(new ProfileInfoUpdateObserver(self));
profileInfoObserver_.reset(
new ProfileInfoUpdateObserver(browser_->profile(), self));
}
return self;
}
......@@ -170,6 +202,9 @@ class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver {
NOTREACHED();
}
- (void)updateErrorStatus:(BOOL)hasError {
}
- (BaseBubbleController*)menuController {
return menuController_;
}
......
......@@ -12,6 +12,7 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#import "chrome/browser/ui/cocoa/browser_window_controller.h"
#include "components/signin/core/browser/signin_error_controller.h"
#include "grit/generated_resources.h"
#include "grit/theme_resources.h"
#import "ui/base/cocoa/appkit_utils.h"
......@@ -19,6 +20,8 @@
#include "ui/base/l10n/l10n_util_mac.h"
#include "ui/base/nine_image_painter_factory.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/image/image_skia_operations.h"
#include "ui/gfx/image/image_skia_util_mac.h"
#include "ui/gfx/text_elider.h"
namespace {
......@@ -49,21 +52,26 @@ NSImage* GetImageFromResourceID(int resourceId) {
@interface CustomThemeButtonCell : NSButtonCell {
@private
BOOL isThemedWindow_;
base::scoped_nsobject<NSImage> authenticationErrorImage_;
}
- (void)setIsThemedWindow:(BOOL)isThemedWindow;
- (void)setHasError:(BOOL)hasError;
@end
@implementation CustomThemeButtonCell
- (id)initWithThemedWindow:(BOOL)isThemedWindow {
if ((self = [super init])) {
isThemedWindow_ = isThemedWindow;
authenticationErrorImage_.reset();
}
return self;
}
- (NSSize)cellSize {
NSSize buttonSize = [super cellSize];
buttonSize.width += 2 * kButtonPadding - 2 * kButtonDefaultPadding;
CGFloat errorWidth = [authenticationErrorImage_ size].width;
buttonSize.width += 2 * (kButtonPadding - kButtonDefaultPadding) + errorWidth;
buttonSize.height = kButtonHeight;
return buttonSize;
}
......@@ -72,7 +80,27 @@ NSImage* GetImageFromResourceID(int resourceId) {
withFrame:(NSRect)frame
inView:(NSView*)controlView {
frame.origin.x = kButtonPadding;
// Ensure there's always a padding between the text and the image.
// If there's an auth error, draw a warning icon before the cell image.
if (authenticationErrorImage_) {
NSSize imageSize = [authenticationErrorImage_ size];
NSRect rect = NSMakeRect(
frame.size.width - imageSize.width,
(kButtonHeight - imageSize.height) / 2,
imageSize.width,
imageSize.height);
[authenticationErrorImage_ drawInRect:rect
fromRect:NSZeroRect
operation:NSCompositeSourceOver
fraction:1.0
respectFlipped:YES
hints:nil];
// Padding between the title and the error image.
frame.size.width -= kButtonTitleImageSpacing;
}
// Padding between the title (or error image, if it exists) and the
// button's drop down image.
frame.size.width -= kButtonTitleImageSpacing;
return [super drawTitle:title withFrame:frame inView:controlView];
}
......@@ -106,11 +134,23 @@ NSImage* GetImageFromResourceID(int resourceId) {
- (void)setIsThemedWindow:(BOOL)isThemedWindow {
isThemedWindow_ = isThemedWindow;
}
- (void)setHasError:(BOOL)hasError {
if (hasError) {
authenticationErrorImage_.reset(
[ui::ResourceBundle::GetSharedInstance().GetImageNamed(
IDR_ICON_PROFILES_AVATAR_BUTTON_ERROR).ToNSImage() retain]);
} else {
authenticationErrorImage_.reset();
}
}
@end
@interface AvatarButtonController (Private)
- (base::string16)getElidedAvatarName;
- (void)updateAvatarButtonAndLayoutParent:(BOOL)layoutParent;
- (void)updateErrorStatus:(BOOL)hasError;
- (void)dealloc;
- (void)themeDidChangeNotification:(NSNotification*)aNotification;
@end
......@@ -135,6 +175,10 @@ NSImage* GetImageFromResourceID(int resourceId) {
button_.reset(hoverButton);
base::scoped_nsobject<CustomThemeButtonCell> cell(
[[CustomThemeButtonCell alloc] initWithThemedWindow:isThemedWindow_]);
SigninErrorController* errorController =
profiles::GetSigninErrorController(browser->profile());
if (errorController)
[cell setHasError:errorController->HasError()];
[button_ setCell:cell.get()];
[self setView:button_];
......@@ -156,7 +200,6 @@ NSImage* GetImageFromResourceID(int resourceId) {
selector:@selector(themeDidChangeNotification:)
name:kBrowserThemeDidChangeNotification
object:nil];
}
return self;
}
......@@ -189,7 +232,6 @@ NSImage* GetImageFromResourceID(int resourceId) {
// The button text has a black foreground and a white drop shadow for regular
// windows, and a light text with a dark drop shadow for guest windows
// which are themed with a dark background.
// TODO(noms): Figure out something similar for themed windows, if possible.
base::scoped_nsobject<NSShadow> shadow([[NSShadow alloc] init]);
[shadow setShadowOffset:NSMakeSize(0, -1)];
[shadow setShadowBlurRadius:0];
......@@ -239,4 +281,9 @@ NSImage* GetImageFromResourceID(int resourceId) {
}
}
- (void)updateErrorStatus:(BOOL)hasError {
[[button_ cell] setHasError:hasError];
[self updateAvatarButtonAndLayoutParent:YES];
}
@end
......@@ -106,6 +106,10 @@ class WebContents;
// account from the active profile if possible.
- (IBAction)showAccountRemovalView:(id)sender;
// Shows the account reauthentication view to re-sign in the currently selected
// account from the active profile if possible.
- (IBAction)showAccountReauthenticationView:(id)sender;
// Removes the current account |accountIdToRemove_|.
- (IBAction)removeAccount:(id)sender;
......
......@@ -228,6 +228,12 @@ NSView* BuildTitleCard(NSRect frame_rect,
return container.autorelease();
}
bool HasAuthError(Profile* profile) {
const SigninErrorController* error_controller =
profiles::GetSigninErrorController(profile);
return error_controller && error_controller->HasError();
}
} // namespace
// Class that listens to changes to the OAuth2Tokens for the active profile,
......@@ -279,7 +285,8 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
profiles::BubbleViewMode viewMode = [controller_ viewMode];
if (viewMode == profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT ||
viewMode == profiles::BUBBLE_VIEW_MODE_GAIA_SIGNIN ||
viewMode == profiles::BUBBLE_VIEW_MODE_GAIA_ADD_ACCOUNT) {
viewMode == profiles::BUBBLE_VIEW_MODE_GAIA_ADD_ACCOUNT ||
viewMode == profiles::BUBBLE_VIEW_MODE_GAIA_REAUTH) {
[controller_ initMenuContentsWithView:
profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT];
}
......@@ -746,11 +753,13 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
frameOrigin:(NSPoint)frameOrigin
action:(SEL)action;
// Creates an email account button with |title| and a remove icon. |tag|
// Creates an email account button with |title| and a remove icon. If
// |reauthRequired| is true, the button also displays a warning icon. |tag|
// indicates which account the button refers to.
- (NSButton*)accountButtonWithRect:(NSRect)rect
title:(const std::string&)title
tag:(int)tag;
tag:(int)tag
reauthRequired:(BOOL)reauthRequired;
@end
@implementation ProfileChooserController
......@@ -827,6 +836,11 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
[self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_ACCOUNT_REMOVAL];
}
- (IBAction)showAccountReauthenticationView:(id)sender {
DCHECK(!isGuestSession_);
[self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_GAIA_REAUTH];
}
- (IBAction)removeAccount:(id)sender {
DCHECK(!accountIdToRemove_.empty());
ProfileOAuth2TokenServiceFactory::GetPlatformSpecificForProfile(
......@@ -917,6 +931,13 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
// Guest profiles do not have a token service.
isGuestSession_ = browser_->profile()->IsGuestSession();
// If view mode is PROFILE_CHOOSER but there is an auth error, force
// ACCOUNT_MANAGEMENT mode.
if (viewMode_ == profiles::BUBBLE_VIEW_MODE_PROFILE_CHOOSER &&
HasAuthError(browser_->profile())) {
viewMode_ = profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT;
}
[[self bubble] setAlignment:info_bubble::kAlignRightEdgeToAnchorEdge];
[[self bubble] setArrowLocation:info_bubble::kNoArrow];
[[self bubble] setBackgroundColor:GetDialogBackgroundColor()];
......@@ -1508,21 +1529,32 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
std::vector<std::string>accounts =
profiles::GetSecondaryAccountsForProfile(profile, primaryAccount);
// If there is an account with an authentication error, it needs to be
// badged with a warning icon.
const SigninErrorController* errorController =
profiles::GetSigninErrorController(profile);
std::string errorAccountId =
errorController ? errorController->error_account_id() : std::string();
rect.origin.y = 0;
for (size_t i = 0; i < accounts.size(); ++i) {
// Save the original email address, as the button text could be elided.
currentProfileAccounts_[i] = accounts[i];
NSButton* accountButton = [self accountButtonWithRect:rect
NSButton* accountButton =
[self accountButtonWithRect:rect
title:accounts[i]
tag:i];
tag:i
reauthRequired:errorAccountId == accounts[i]];
[container addSubview:accountButton];
rect.origin.y = NSMaxY([accountButton frame]);
}
// The primary account should always be listed first.
NSButton* accountButton = [self accountButtonWithRect:rect
NSButton* accountButton =
[self accountButtonWithRect:rect
title:primaryAccount
tag:kPrimaryProfileTag];
tag:kPrimaryProfileTag
reauthRequired:errorAccountId == primaryAccount];
[container addSubview:accountButton];
[container setFrameSize:NSMakeSize(NSWidth([container frame]),
NSMaxY([accountButton frame]))];
......@@ -1534,17 +1566,38 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
[[NSView alloc] initWithFrame:NSZeroRect]);
CGFloat yOffset = 0;
bool addSecondaryAccount =
viewMode_ == profiles::BUBBLE_VIEW_MODE_GAIA_ADD_ACCOUNT;
signin::Source source = addSecondaryAccount ?
signin::SOURCE_AVATAR_BUBBLE_ADD_ACCOUNT :
signin::SOURCE_AVATAR_BUBBLE_SIGN_IN;
GURL url;
int messageId = -1;
SigninErrorController* errorController = NULL;
switch (viewMode_) {
case profiles::BUBBLE_VIEW_MODE_GAIA_SIGNIN:
url = signin::GetPromoURL(signin::SOURCE_AVATAR_BUBBLE_SIGN_IN,
false /* auto_close */,
true /* is_constrained */);
messageId = IDS_PROFILES_GAIA_SIGNIN_TITLE;
break;
case profiles::BUBBLE_VIEW_MODE_GAIA_ADD_ACCOUNT:
url = signin::GetPromoURL(signin::SOURCE_AVATAR_BUBBLE_ADD_ACCOUNT,
false /* auto_close */,
true /* is_constrained */);
messageId = IDS_PROFILES_GAIA_ADD_ACCOUNT_TITLE;
break;
case profiles::BUBBLE_VIEW_MODE_GAIA_REAUTH:
DCHECK(HasAuthError(browser_->profile()));
errorController = profiles::GetSigninErrorController(browser_->profile());
url = signin::GetReauthURL(
browser_->profile(),
errorController ? errorController->error_username() : std::string());
messageId = IDS_PROFILES_GAIA_REAUTH_TITLE;
break;
default:
NOTREACHED() << "Called with invalid mode=" << viewMode_;
break;
}
webContents_.reset(content::WebContents::Create(
content::WebContents::CreateParams(browser_->profile())));
webContents_->GetController().LoadURL(
signin::GetPromoURL(
source, false /* auto_close */, true /* is_constrained */),
webContents_->GetController().LoadURL(url,
content::Referrer(),
content::PAGE_TRANSITION_AUTO_TOPLEVEL,
std::string());
......@@ -1560,9 +1613,8 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
yOffset = NSMaxY([separator frame]) + kSmallVerticalSpacing;
NSView* titleView = BuildTitleCard(
NSMakeRect(0, yOffset, kFixedGaiaViewWidth,0),
addSecondaryAccount ? IDS_PROFILES_GAIA_ADD_ACCOUNT_TITLE :
IDS_PROFILES_GAIA_SIGNIN_TITLE,
NSMakeRect(0, yOffset, kFixedGaiaViewWidth, 0),
messageId,
self /* backButtonTarget*/,
@selector(navigateBackFromSigninPage:) /* backButtonAction */);
[container addSubview:titleView];
......@@ -1751,29 +1803,47 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
- (NSButton*)accountButtonWithRect:(NSRect)rect
title:(const std::string&)title
tag:(int)tag {
tag:(int)tag
reauthRequired:(BOOL)reauthRequired {
ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
NSImage* deleteImage = rb->GetNativeImageNamed(IDR_CLOSE_1).ToNSImage();
CGFloat deleteImageWidth = [deleteImage size].width;
NSImage* warningImage = reauthRequired ? rb->GetNativeImageNamed(
IDR_ICON_PROFILES_ACCOUNT_BUTTON_ERROR).ToNSImage() : nil;
CGFloat warningImageWidth = [warningImage size].width;
CGFloat availableTextWidth = rect.size.width - kHorizontalSpacing -
warningImageWidth - deleteImageWidth;
if (warningImage)
availableTextWidth -= kHorizontalSpacing;
NSColor* backgroundColor = gfx::SkColorToCalibratedNSColor(
profiles::kAvatarBubbleAccountsBackgroundColor);
base::scoped_nsobject<BackgroundColorHoverButton> button(
[[BackgroundColorHoverButton alloc] initWithFrame:rect
imageTitleSpacing:0
backgroundColor:backgroundColor]);
ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
NSImage* defaultImage = rb->GetNativeImageNamed(IDR_CLOSE_1).AsNSImage();
CGFloat kDeleteButtonWidth = [defaultImage size].width;
CGFloat availableWidth = rect.size.width -
kDeleteButtonWidth - kHorizontalSpacing;
[button setTitle:ElideEmail(title, availableWidth)];
[button setTitle:ElideEmail(title, availableTextWidth)];
[button setAlignment:NSLeftTextAlignment];
[button setBordered:NO];
if (reauthRequired) {
[button setDefaultImage:warningImage];
[button setImagePosition:NSImageLeft];
[button setTarget:self];
[button setAction:@selector(showAccountReauthenticationView:)];
[button setTag:tag];
}
// Delete button.
rect.origin = NSMakePoint(availableWidth, 0);
rect.size.width = kDeleteButtonWidth;
NSRect buttonRect;
NSDivideRect(rect, &buttonRect, &rect,
deleteImageWidth + kHorizontalSpacing, NSMaxXEdge);
buttonRect.origin.y = 0;
base::scoped_nsobject<HoverImageButton> deleteButton(
[[HoverImageButton alloc] initWithFrame:rect]);
[[HoverImageButton alloc] initWithFrame:buttonRect]);
[deleteButton setBordered:NO];
[deleteButton setDefaultImage:defaultImage];
[deleteButton setDefaultImage:deleteImage];
[deleteButton setHoverImage:rb->GetNativeImageNamed(
IDR_CLOSE_1_H).ToNSImage()];
[deleteButton setPressedImage:rb->GetNativeImageNamed(
......@@ -1783,6 +1853,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
[deleteButton setTag:tag];
[button addSubview:deleteButton];
return button.autorelease();
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment