[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

Index: chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm
diff --git a/chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm b/chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm
index 88b8f00def2ce9589f6e84011039e21fccc5f9cb..23db640bda449c735227500101c5bb319f0d5270 100644
--- a/chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm
@@ -10,6 +10,7 @@
 #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"
@@ -17,6 +18,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"
 
@@ -31,19 +33,32 @@ const CGFloat kMenuYOffsetAdjust = 1.0;
 // 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:
@@ -68,7 +83,15 @@ class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver {
     [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);
@@ -79,7 +102,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;
 }
@@ -166,6 +190,9 @@ class ProfileInfoUpdateObserver : public ProfileInfoCacheObserver {
   NOTREACHED();
 }
 
+- (void)updateErrorStatus:(BOOL)hasError {
+}
+
 - (BaseBubbleController*)menuController {
   return menuController_;
 }