[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

Index: chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
index c1b84666aebe8bc76e466633013286a66a890d66..16358303c433928d1f71b12467358ce9fb3f2ed6 100644
--- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
@@ -228,6 +228,12 @@ NSView* BuildTitleCard(NSRect frame_rect,
   return container.autorelease();
 }
 
+bool HasAuthError(Profile* profile) {
+  const SigninErrorController* error =

[groby-ooo-7-16, 2014/06/10 19:28:14]

error_controller, please - here and elsewhere.

[noms (inactive), 2014/06/17 17:07:33]

Done.

+      profiles::GetSigninErrorController(profile);
+  return error && error->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)showAccountReauthView:(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()];
@@ -1505,21 +1526,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.

[groby-ooo-7-16, 2014/06/10 19:28:15]

Question: Is there ever a chance of multiple accounts with an auth error?

[noms (inactive), 2014/06/17 17:07:32]

Nope. Mike asked the same question on a previous patch. The tl; dr is: this is
the same ErrorController that was used on the hot dog menu, so it can only badge
the menu for one error at a time. If there's multiple accounts that have errors,
only the first one is returned. Once that's fixed, you get another error and so
on.

On 2014/06/10 19:28:15, groby wrote:

+  const SigninErrorController* error =
+      profiles::GetSigninErrorController(profile);
+  std::string errorAccountId =
+      error ? error->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
-                                                    title:accounts[i]
-                                                      tag:i];
+    NSButton* accountButton =
+        [self accountButtonWithRect:rect
+                              title:accounts[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
-                                                  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]))];
@@ -1531,20 +1563,40 @@ 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* error = 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()));
+      error = profiles::GetSigninErrorController(browser_->profile());
+      url = signin::GetReauthURL(
+          browser_->profile(), error ? error->error_username() : std::string());
+      messageId = IDS_PROFILES_GAIA_REAUTH_TITLE;
+      break;
+    default:
+      NOTREACHED() << "Called with invalid mode=" << viewMode_;
+      return NULL;

[groby-ooo-7-16, 2014/06/10 19:28:14]

use break instead of NULL, or kill NOTREACHED. (Style guide says either handle
errors or DCHECK/NOTREACHED, but not both.

In fact, if this handles all viewModes, you can kill the default - that way,
clang will warn if a new enum is added that would lead to an unhandled case.

[noms (inactive), 2014/06/17 17:07:33]

Done.

+  }
 
   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];
@@ -1557,9 +1609,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];
@@ -1748,29 +1799,43 @@ 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 = reauthRequired ? [warningImage size].width : 0;

[groby-ooo-7-16, 2014/06/10 19:28:14]

Technically, no need to check - just [warningImage size].width. [nil size]
returns an NSRect initialized to 0

(See
https://developer.apple.com/library/mac/documentation/cocoa/conceptual/Progra...


"If you expect a return value from a message sent to nil, the return value will
be nil for object return types, 0 for numeric types, and NO for BOOL types.
Returned structures have all members initialized to zero.")

Objective-C is awesome :)

[noms (inactive), 2014/06/17 17:07:32]

Oh god. Ok. I see. Done.

On 2014/06/10 19:28:14, groby wrote:

+
+  CGFloat availableTextWidth = rect.size.width - 2 * kHorizontalSpacing -

[msw, 2014/06/10 17:13:03]

nit: should the available text width only include one kHorizontalSpacing if
warningImage will not be shown?

[noms (inactive), 2014/06/17 17:07:32]

Done.

+      warningImageWidth - deleteImageWidth;
+
   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(showAccountReauthView:)];
+    [button setTag:tag];
+  }
 
   // Delete button.
-  rect.origin = NSMakePoint(availableWidth, 0);
-  rect.size.width = kDeleteButtonWidth;
+  rect.origin = NSMakePoint(

[groby-ooo-7-16, 2014/06/10 19:28:15]

Easier:
NSRect buttonRect;
NSDivideRect(rect, &buttonRect, &rect, deleteImageWidth, NSMaxXEdge);

This slices off a rect of |deleteImageWidth| at the MaxXEdge, i.e. the
right-hand side, stores it in buttonRect, and stuffs the remainder in rect.

Less math is good math :)

[noms (inactive), 2014/06/17 17:07:32]

Done.

+      rect.size.width - kHorizontalSpacing - deleteImageWidth, 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(
@@ -1780,6 +1845,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   [deleteButton setTag:tag];
 
   [button addSubview:deleteButton];
+
   return button.autorelease();
 }