[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 210 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) {
+  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,
 // 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 446 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.
   bool alwaysCreate = ui::WindowOpenDispositionFromNSEvent(
@@ skipping 56 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_);
   [self postActionPerformed:ProfileMetrics::PROFILE_DESKTOP_MENU_REMOVE_ACCT];
   accountIdToRemove_.clear();
 
   [self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT];
 }
 
@@ skipping 70 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 568 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);
 
+  // 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]))];
   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 = -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];
   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).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(
       IDR_CLOSE_1_P).ToNSImage()];
   [deleteButton setTarget:self];
   [deleteButton setAction:@selector(showAccountRemovalView:)];
   [deleteButton setTag:tag];
 
   [button addSubview:deleteButton];
+
   return button.autorelease();
 }
 
 - (void)postActionPerformed:(ProfileMetrics::ProfileDesktopMenu)action {
   ProfileMetrics::LogProfileDesktopMenu(action, serviceType_);
   serviceType_ = signin::GAIA_SERVICE_TYPE_NONE;
 }
 
 @end