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

chrome/browser/ui/cocoa/profiles/avatar_base_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 "chrome/browser/ui/cocoa/profiles/avatar_base_controller.h"
 
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_info_cache_observer.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.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"
 #import "chrome/browser/ui/cocoa/base_bubble_controller.h"
 #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"
 
 // Space between the avatar icon and the avatar menu bubble.
 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;
+- (void)updateErrorStatus:(BOOL)hasError;

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

nit: consider a more descriptive name or add a comment.

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

Done.

 @end
 
-class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver {
+class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver,
+                                  public SigninErrorController::Observer {
  public:
-  ProfileInfoUpdateObserver(AvatarBaseController* avatarButton)
-      : avatarButton_(avatarButton) {
+  ProfileInfoUpdateObserver(Profile* profile,
+                            AvatarBaseController* avatarButton)
+      : profile_(profile),
+        avatarButton_(avatarButton) {
     g_browser_process->profile_manager()->
         GetProfileInfoCache().AddObserver(this);
+
+    // Subscribe to authentication error changes so that the avatar button
+    // can update itself.
+    SigninErrorController* error = profiles::GetSigninErrorController(profile_);
+    if (error)

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

nit: Can this generally ever be NIL? (remove if statements everywhere otherwise)

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

Yes. The SigninErrorController can be null for Guest or Incognito profiles.

On 2014/06/03 04:15:31, msw wrote:

+      error->AddObserver(this);
   }
 
   virtual ~ProfileInfoUpdateObserver() {
     g_browser_process->profile_manager()->
         GetProfileInfoCache().RemoveObserver(this);
+    SigninErrorController* error = profiles::GetSigninErrorController(profile_);
+    if(error)

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

nit: space after if

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

Done.

+      error->RemoveObserver(this);
   }
 
   // ProfileInfoCacheObserver:
   virtual void OnProfileAdded(const base::FilePath& profile_path) OVERRIDE {
     [avatarButton_ updateAvatarButtonAndLayoutParent:YES];
   }
 
   virtual void OnProfileWasRemoved(
       const base::FilePath& profile_path,
       const base::string16& profile_name) OVERRIDE {
     [avatarButton_ updateAvatarButtonAndLayoutParent:YES];
   }
 
   virtual void OnProfileNameChanged(
       const base::FilePath& profile_path,
       const base::string16& old_profile_name) OVERRIDE {
     [avatarButton_ updateAvatarButtonAndLayoutParent:YES];
   }
 
   virtual void OnProfileAvatarChanged(
       const base::FilePath& profile_path) OVERRIDE {
     [avatarButton_ updateAvatarButtonAndLayoutParent:YES];
   }
 
+  // SigninErrorController::Observer:
+  virtual void OnErrorChanged() OVERRIDE {
+    SigninErrorController* error = profiles::GetSigninErrorController(profile_);
+    if (error)
+      [avatarButton_ updateErrorStatus:error->HasError()];

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

When won't error exist? Should the button be updated then?

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

|error| can be null for guest or incognito profiles. Since the profile can't
change types, |error| will always be null or not-null for the same profile. 
Now, |error->HasError()| can change as you encounter/clear auth errors, in which
case we need to remove the notification. OnErrorChanged() will be called for
both error->no error and no error->error transitions, and the icon needs to
change around that.

On 2014/06/03 04:15:31, msw wrote:

+  }
+
  private:
+  Profile* profile_;
   AvatarBaseController* avatarButton_;  // Weak; owns this.
 
   DISALLOW_COPY_AND_ASSIGN(ProfileInfoUpdateObserver);
 };
 
 @implementation AvatarBaseController
 
 - (id)initWithBrowser:(Browser*)browser {
   if ((self = [super init])) {
     browser_ = browser;
-    profileInfoObserver_.reset(new ProfileInfoUpdateObserver(self));
+    profileInfoObserver_.reset(
+        new ProfileInfoUpdateObserver(browser_->profile(), self));
   }
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter]
       removeObserver:self
                 name:NSWindowWillCloseNotification
               object:[menuController_ window]];
   [super dealloc];
@@ skipping 66 matching lines @@
     [static_cast<BrowserWindowController*>(wc)
         releaseBarVisibilityForOwner:self withAnimation:YES delay:NO];
   }
   menuController_ = nil;
 }
 
 - (void)updateAvatarButtonAndLayoutParent:(BOOL)layoutParent {
   NOTREACHED();
 }
 
+- (void)updateErrorStatus:(BOOL)hasError {
+}
+
 - (BaseBubbleController*)menuController {
   return menuController_;
 }
 
 @end