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

chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import <Cocoa/Cocoa.h>
 
 #import "chrome/browser/ui/cocoa/profiles/profile_chooser_controller.h"
 
 #include "base/mac/bundle_locations.h"
 #include "base/prefs/pref_service.h"
@@ skipping 212 matching lines @@
 
   [container addSubview:button];
   [container addSubview:title_label];
   CGFloat height = std::max(NSMaxY([title_label frame]),
                             NSMaxY([button frame])) + kSmallVerticalSpacing;
   [container setFrameSize:NSMakeSize(NSWidth([container frame]), height)];
 
   return container.autorelease();
 }
 
+bool HasAuthError(Profile* profile) {

[msw, 2014/06/03 04:15:31]

nit: this is used twice, is a helper necessary?

[noms (inactive), 2014/06/06 20:33:44]

I thought it looked a little nicer, because the three lines of code look a
little annoying, but I can inline it if you want to. 
On 2014/06/03 04:15:31, msw wrote:

+  const SigninErrorController* error =
+      profiles::GetSigninErrorController(profile);
+  return error && error->HasError();
+}
+
+std::string GetAuthErrorAccountId(Profile* profile) {

[msw, 2014/06/03 04:15:31]

nit: this is used once, please inline it.

[noms (inactive), 2014/06/06 20:33:44]

Done.

+  SigninErrorController* error =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile)->
+          signin_error_controller();
+  if (!error)

[msw, 2014/06/03 04:15:31]

nit: return error ? error->error_account_id() : std::string() if you keep this.

[noms (inactive), 2014/06/06 20:33:44]

Done.

+    return std::string();
+
+  return error->error_account_id();
+}
+
+std::string GetAuthErrorUsername(Profile* profile) {

[msw, 2014/06/03 04:15:31]

nit: this is used once, please inline it.

[noms (inactive), 2014/06/06 20:33:43]

Done.

+  const SigninErrorController* error =
+      profiles::GetSigninErrorController(profile);
+  if (!error)

[msw, 2014/06/03 04:15:31]

nit: return error ? error->error_username() : std::string() if you keep this.

[noms (inactive), 2014/06/06 20:33:44]

Done.

+    return std::string();
+
+  return error->error_username();
+}
+
 }  // namespace
 
 // Class that listens to changes to the OAuth2Tokens for the active profile,
 // changes to the avatar menu model or browser close notifications.
 class ActiveProfileObserverBridge : public AvatarMenuObserver,
                                     public content::NotificationObserver,
                                     public OAuth2TokenService::Observer {
  public:
   ActiveProfileObserverBridge(ProfileChooserController* controller,
                               Browser* browser)
@@ skipping 31 matching lines @@
   }
 
   // OAuth2TokenService::Observer:
   virtual void OnRefreshTokenAvailable(const std::string& account_id) OVERRIDE {
     // Tokens can only be added by adding an account through the inline flow,
     // which is started from the account management view. Refresh it to show the
     // update.
     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];
     }
   }
 
   virtual void OnRefreshTokenRevoked(const std::string& account_id) OVERRIDE {
     // Tokens can only be removed from the account management view. Refresh it
     // to show the update.
     if ([controller_ viewMode] == profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT)
       [controller_ initMenuContentsWithView:
@@ skipping 434 matching lines @@
                  imageResourceId:(int)imageResourceId
         alternateImageResourceId:(int)alternateImageResourceId
                           action:(SEL)action;
 
 // Creates a generic link button with |title| and an |action| positioned at
 // |frameOrigin|.
 - (NSButton*)linkButtonWithTitle:(NSString*)title
                      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
 - (profiles::BubbleViewMode) viewMode {
   return viewMode_;
 }
 
 - (IBAction)switchToProfile:(id)sender {
   // Check the event flags to see if a new window should be created.
@@ skipping 55 matching lines @@
     accountIdToRemove_ = SigninManagerFactory::GetForProfile(
         browser_->profile())->GetAuthenticatedUsername();
   } else {
     DCHECK(ContainsKey(currentProfileAccounts_, tag));
     accountIdToRemove_ = currentProfileAccounts_[tag];
   }
 
   [self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_ACCOUNT_REMOVAL];
 }
 
+- (IBAction)showAccountReauthView:(id)sender {
+  DCHECK(!isGuestSession_);
+  [self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_GAIA_REAUTH];
+}
+
 - (IBAction)removeAccount:(id)sender {
   DCHECK(!accountIdToRemove_.empty());
   ProfileOAuth2TokenServiceFactory::GetPlatformSpecificForProfile(
       browser_->profile())->RevokeCredentials(accountIdToRemove_);
   accountIdToRemove_.clear();
 
   [self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT];
 }
 
 - (IBAction)openTutorialLearnMoreURL:(id)sender {
@@ skipping 67 matching lines @@
 
     avatarMenu_.reset(new AvatarMenu(
         &g_browser_process->profile_manager()->GetProfileInfoCache(),
         observer_.get(),
         browser_));
     avatarMenu_->RebuildMenu();
 
     // 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()];
     [self initMenuContentsWithView:viewMode_];
   }
 
   return self;
 }
 
 - (void)initMenuContentsWithView:(profiles::BubbleViewMode)viewToDisplay {
@@ skipping 558 matching lines @@
   base::scoped_nsobject<NSView> container([[NSView alloc] initWithFrame:rect]);
   currentProfileAccounts_.clear();
 
   Profile* profile = browser_->profile();
   std::string primaryAccount =
       SigninManagerFactory::GetForProfile(profile)->GetAuthenticatedUsername();
   DCHECK(!primaryAccount.empty());
   std::vector<std::string>accounts =
       profiles::GetSecondaryAccountsForProfile(profile, primaryAccount);
 
+  // Get state of authentication error, if any.

[msw, 2014/06/03 04:15:31]

nit: this comment should be made more helpful or removed.

[noms (inactive), 2014/06/06 20:33:43]

Done.

+  std::string errorAccountId = GetAuthErrorAccountId(profile);
+
   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
-                                                    title:accounts[i]
-                                                      tag:i];
+    NSButton* accountButton =
+        [self accountButtonWithRect:rect
+                              title:accounts[i]
+                                tag:i
+                     reauthRequired:errorAccountId == accounts[i]];

[msw, 2014/06/03 04:15:31]

Couldn't multiple accounts have auth errors at the same time?

[noms (inactive), 2014/06/06 20:33:44]

They could, but the way it works right now is that the SigninErrorController
will only return one error at a time. Once you resolve that, you might get
another error, for a different account.
On 2014/06/03 04:15:31, msw wrote:

[msw, 2014/06/09 19:02:38]

It's also unclear to me how this retains the authorization error for
account/button A if another error appears for account/button B. Does
SigninErrorController only express an authorization error for one account at a
time, and then express subsequent account errors once the first error has been
resolved? (if so, why?). Can you add a comment explaining the pattern here, it's
definitely not obvious. 

[noms (inactive), 2014/06/10 15:02:47]

A little background: this error controller used to be used in the hot dog menu,
where we could only badge the menu for one error at a time. So the controller
sort of queues up errors. If you're badging account A with an error, and account
B also has an error, the controller will only care about the account A error
until it gets resolved. Once it gets resolved, it goes and looks for any other
errors. This works the same in the Windows menu too.

In theory, now that we *could* display multiple errors in parallel, the
controller could change, but that's a bit of a giant yak shave that I don't
thing belongs in this CL. :)

On 2014/06/09 19:02:38, msw wrote:

     [container addSubview:accountButton];
     rect.origin.y = NSMaxY([accountButton frame]);
   }
 
   // The primary account should always be listed first.
-  NSButton* accountButton = [self accountButtonWithRect:rect
-                                                  title:primaryAccount
-                                                    tag:kPrimaryProfileTag];
+  NSButton* accountButton =
+      [self accountButtonWithRect:rect
+                            title:primaryAccount
+                              tag:kPrimaryProfileTag
+                   reauthRequired:errorAccountId == primaryAccount];
   [container addSubview:accountButton];
   [container setFrameSize:NSMakeSize(NSWidth([container frame]),
                                      NSMaxY([accountButton frame]))];
   return container.autorelease();
 }
 
 - (NSView*)buildGaiaEmbeddedView {
   base::scoped_nsobject<NSView> container(
       [[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;

[msw, 2014/06/03 04:15:31]

nit: initialize this value

[noms (inactive), 2014/06/06 20:33:44]

Done.

+  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: {

[msw, 2014/06/03 04:15:31]

nit: you shouldn't need these curly braces.

[noms (inactive), 2014/06/06 20:33:44]

Without the curly braces, the compiler is unhappy with defining the
SigninErrorController in only one of the cases, since all the cases are in the
same scope. I've moved the definition to outside the switch. 

+      DCHECK(HasAuthError(browser_->profile()));
+      url = signin::GetReauthURL(browser_->profile(),
+                                 GetAuthErrorUsername(browser_->profile()));
+      messageId = IDS_PROFILES_GAIA_REAUTH_TITLE;
+      break;
+    }
+    default:
+      NOTREACHED() << "Called with invalid mode=" << viewMode_;
+      return NULL;
+  }
 
   webContents_.reset(content::WebContents::Create(
       content::WebContents::CreateParams(browser_->profile())));
-  webContents_->GetController().LoadURL(
-      signin::GetPromoURL(
-          source, false /* auto_close */, true /* is_constrained */),
-      content::Referrer(),
-      content::PAGE_TRANSITION_AUTO_TOPLEVEL,
-      std::string());
+  webContents_->GetController().LoadURL(url,
+                                        content::Referrer(),
+                                        content::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                        std::string());
   NSView* webview = webContents_->GetNativeView();
   [webview setFrameSize:NSMakeSize(kFixedGaiaViewWidth, kFixedGaiaViewHeight)];
   [container addSubview:webview];
   yOffset = NSMaxY([webview frame]);
 
   // Adds the title card.
   NSBox* separator = [self separatorWithFrame:
       NSMakeRect(0, yOffset, kFixedGaiaViewWidth, 0)];
   [container addSubview:separator];
   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];
   yOffset = NSMaxY([titleView frame]);
 
   [container setFrameSize:NSMakeSize(kFixedGaiaViewWidth, yOffset)];
   return container.autorelease();
 }
 
 - (NSView*)buildAccountRemovalView {
@@ skipping 168 matching lines @@
   [link setTarget:self];
   [link setAction:action];
   [link setFrameOrigin:frameOrigin];
   [link sizeToFit];
 
   return link.autorelease();
 }
 
 - (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).AsNSImage();
+  CGFloat deleteImageWidth = [deleteImage size].width;
+
+  NSImage* reauthImage = nil;
+  CGFloat reauthImageWidth = 0;
+  if (reauthRequired) {
+    reauthImage = rb->GetNativeImageNamed(
+        IDR_ICON_PROFILES_ACCOUNT_BUTTON_ERROR).AsNSImage();
+    reauthImageWidth = [reauthImage size].width;

[msw, 2014/06/03 04:15:31]

Do you need to add some padding, like kHorizontalSpacing, here or below?

[noms (inactive), 2014/06/06 20:33:44]

Done.

+  }
+
+  CGFloat availableTextWidth = rect.size.width - kHorizontalSpacing -
+      deleteImageWidth - reauthImageWidth;
+
   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 setTarget:self];
+    [button setAction:@selector(showAccountReauthView:)];
+    [button setTag:tag];
+  }
 
   // Delete button.
-  rect.origin = NSMakePoint(availableWidth, 0);
-  rect.size.width = kDeleteButtonWidth;
+  rect.origin = NSMakePoint(availableTextWidth + reauthImageWidth, 0);
+  rect.size.width = deleteImageWidth;
   base::scoped_nsobject<HoverImageButton> deleteButton(
       [[HoverImageButton alloc] initWithFrame:rect]);
   [deleteButton setBordered:NO];
-  [deleteButton setDefaultImage:defaultImage];
+  [deleteButton setDefaultImage:deleteImage];
   [deleteButton setHoverImage:rb->GetNativeImageNamed(
       IDR_CLOSE_1_H).ToNSImage()];
   [deleteButton setPressedImage:rb->GetNativeImageNamed(
       IDR_CLOSE_1_P).ToNSImage()];
   [deleteButton setTarget:self];
   [deleteButton setAction:@selector(showAccountRemovalView:)];
   [deleteButton setTag:tag];
 
   [button addSubview:deleteButton];
+
+  // Warning icon.
+  if (reauthRequired) {
+    base::scoped_nsobject<NSImageView> reauthIcon(
+        [[NSImageView alloc] initWithFrame:NSMakeRect(
+            availableTextWidth, 0, reauthImageWidth, rect.size.height)]);
+    [reauthIcon setImage:reauthImage];
+    [button addSubview:reauthIcon];
+  }
+
   return button.autorelease();
 }
 
 @end