[Sync] Propagate XMPP auth errors to SyncNotifierObservers

Detect XMPP auth errors and add new notifications to SingleLoginAttempt
and Login.

Replace PushClientObserver::OnNotificationStateChange with
OnNotifications{Enabled,Disabled} notifications.

Change SyncNotifierObserver similarly.

Handle InvalidationClient errors and propagate auth errors from that,
too.

Propagate XMPP auth errors all the way up to SyncManager.  It will be
handled in a future CL.

BUG=38091
TEST=


Review URL: https://chromiumcodereview.appspot.com/10545170

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142806 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 218 insertions, 86 deletions

diff --git a/jingle/notifier/communicator/login.cc b/jingle/notifier/communicator/login.cc
index 886ea84..db4813b 100644
--- a/jingle/notifier/communicator/login.cc
+++ b/jingle/notifier/communicator/login.cc
@@ -27,6 +27,8 @@ namespace notifier {
 // Redirect valid for 5 minutes.
 static const int kRedirectTimeoutMinutes = 5;
 
+Login::Delegate::~Delegate() {}
+
 Login::Login(Delegate* delegate,
              const buzz::XmppClientSettings& user_settings,
              const scoped_refptr<net::URLRequestContextGetter>&
@@ -57,19 +59,31 @@ void Login::UpdateXmppSettings(const buzz::XmppClientSettings& user_settings) {
   login_settings_.set_user_settings(user_settings);
 }
 
+// In the code below, we assume that calling a delegate method may end
+// up in ourselves being deleted, so we always call it last.
+//
+// TODO(akalin): Add unit tests to enforce the behavior above.
+
 void Login::OnConnect(base::WeakPtr<buzz::XmppTaskParentInterface> base_task) {
   ResetReconnectState();
   delegate_->OnConnect(base_task);
 }
 
-void Login::OnNeedReconnect() {
-  TryReconnect();
-}
-
 void Login::OnRedirect(const ServerInformation& redirect_server) {
   login_settings_.SetRedirectServer(redirect_server);
   // Drop the current connection, and start the login process again.
   StartConnection();
+  delegate_->OnTransientDisconnection();
+}
+
+void Login::OnCredentialsRejected() {
+  TryReconnect();
+  delegate_->OnCredentialsRejected();
+}
+
+void Login::OnSettingsExhausted() {
+  TryReconnect();
+  delegate_->OnTransientDisconnection();
 }
 
 void Login::OnIPAddressChanged() {
@@ -78,6 +92,7 @@ void Login::OnIPAddressChanged() {
   // avoid spikey behavior on network hiccups).
   reconnect_interval_ = base::TimeDelta::FromSeconds(base::RandInt(1, 9));
   TryReconnect();
+  delegate_->OnTransientDisconnection();
 }
 
 void Login::ResetReconnectState() {
@@ -94,7 +109,6 @@ void Login::TryReconnect() {
           << reconnect_interval_.InSeconds() << " seconds";
   reconnect_timer_.Start(
       FROM_HERE, reconnect_interval_, this, &Login::DoReconnect);
-  delegate_->OnDisconnect();
 }
 
 void Login::DoReconnect() {
diff --git a/jingle/notifier/communicator/login.h b/jingle/notifier/communicator/login.h
index d089cb6d6..29be403 100644
--- a/jingle/notifier/communicator/login.h
+++ b/jingle/notifier/communicator/login.h
@@ -33,24 +33,31 @@ namespace notifier {
 
 class LoginSettings;
 
-// Workaround for MSVS 2005 bug that fails to handle inheritance from a nested
-// class properly if it comes directly on a base class list.
-typedef SingleLoginAttempt::Delegate SingleLoginAttemptDelegate;
-
-// Does the login, keeps it alive (with refreshing cookies and reattempting
-// login when disconnected), figures out what actions to take on the various
-// errors that may occur.
+// Does the login, keeps it alive (with refreshing cookies and
+// reattempting login when disconnected), and figures out what actions
+// to take on the various errors that may occur.
 class Login : public net::NetworkChangeNotifier::IPAddressObserver,
-              public SingleLoginAttemptDelegate {
+              public SingleLoginAttempt::Delegate {
  public:
   class Delegate {
    public:
+    // Called when a connection has been successfully established.
     virtual void OnConnect(
         base::WeakPtr<buzz::XmppTaskParentInterface> base_task) = 0;
-    virtual void OnDisconnect() = 0;
+
+    // Called when there's no connection to the server but we expect
+    // it to come back come back eventually.  The connection will be
+    // retried with exponential backoff.
+    virtual void OnTransientDisconnection() = 0;
+
+    // Called when the current login credentials have been rejected.
+    // The connection will still be retried with exponential backoff;
+    // it's up to the delegate to stop connecting and/or prompt for
+    // new credentials.
+    virtual void OnCredentialsRejected() = 0;
 
    protected:
-    virtual ~Delegate() {}
+    virtual ~Delegate();
   };
 
   // Does not take ownership of |delegate|, which must not be NULL.
@@ -63,9 +70,12 @@ class Login : public net::NetworkChangeNotifier::IPAddressObserver,
         const std::string& auth_mechanism);
   virtual ~Login();
 
+  // Starts connecting (or forces a reconnection if we're backed off).
   void StartConnection();
-  // The updated settings only take effect the next time StartConnection
-  // is called.
+
+  // The updated settings take effect only the next time when a
+  // connection is attempted (either via reconnection or a call to
+  // StartConnection()).
   void UpdateXmppSettings(const buzz::XmppClientSettings& user_settings);
 
   // net::NetworkChangeNotifier::IPAddressObserver implementation.
@@ -74,8 +84,9 @@ class Login : public net::NetworkChangeNotifier::IPAddressObserver,
   // SingleLoginAttempt::Delegate implementation.
   virtual void OnConnect(
       base::WeakPtr<buzz::XmppTaskParentInterface> base_task) OVERRIDE;
-  virtual void OnNeedReconnect() OVERRIDE;
   virtual void OnRedirect(const ServerInformation& redirect_server) OVERRIDE;
+  virtual void OnCredentialsRejected() OVERRIDE;
+  virtual void OnSettingsExhausted() OVERRIDE;
 
  private:
   void OnLogoff();
@@ -103,10 +114,6 @@ class Login : public net::NetworkChangeNotifier::IPAddressObserver,
   DISALLOW_COPY_AND_ASSIGN(Login);
 };
 
-// Workaround for MSVS 2005 bug that fails to handle inheritance from a nested
-// class properly if it comes directly on a base class list.
-typedef Login::Delegate LoginDelegate;
-
 }  // namespace notifier
 
 #endif  // JINGLE_NOTIFIER_COMMUNICATOR_LOGIN_H_
diff --git a/jingle/notifier/communicator/single_login_attempt.cc b/jingle/notifier/communicator/single_login_attempt.cc
index 0c789ee..c2405fe 100644
--- a/jingle/notifier/communicator/single_login_attempt.cc
+++ b/jingle/notifier/communicator/single_login_attempt.cc
@@ -39,6 +39,11 @@ SingleLoginAttempt::SingleLoginAttempt(const LoginSettings& login_settings,
 
 SingleLoginAttempt::~SingleLoginAttempt() {}
 
+// In the code below, we assume that calling a delegate method may end
+// up in ourselves being deleted, so we always call it last.
+//
+// TODO(akalin): Add unit tests to enforce the behavior above.
+
 void SingleLoginAttempt::OnConnect(
     base::WeakPtr<buzz::XmppTaskParentInterface> base_task) {
   DVLOG(1) << "Connected to " << current_settings_->ToString();
@@ -131,6 +136,11 @@ void SingleLoginAttempt::OnError(buzz::XmppEngine::Error error, int subcode,
     }
   }
 
+  if (error == buzz::XmppEngine::ERROR_UNAUTHORIZED) {
+    delegate_->OnCredentialsRejected();
+    return;
+  }
+
   if (current_settings_ == settings_list_.end()) {
     NOTREACHED();
     return;
@@ -139,7 +149,7 @@ void SingleLoginAttempt::OnError(buzz::XmppEngine::Error error, int subcode,
   ++current_settings_;
   if (current_settings_ == settings_list_.end()) {
     VLOG(1) << "Could not connect to any XMPP server";
-    delegate_->OnNeedReconnect();
+    delegate_->OnSettingsExhausted();
     return;
   }
 
diff --git a/jingle/notifier/communicator/single_login_attempt.h b/jingle/notifier/communicator/single_login_attempt.h
index 1e0332d..baf45e1 100644
--- a/jingle/notifier/communicator/single_login_attempt.h
+++ b/jingle/notifier/communicator/single_login_attempt.h
@@ -26,13 +26,30 @@ struct ServerInformation;
 // login attempt.
 class SingleLoginAttempt : public XmppConnection::Delegate {
  public:
+  // At most one delegate method will be called, depending on the
+  // result of the login attempt.  After the delegate method is
+  // called, this class won't do anything anymore until it is
+  // destroyed, at which point it will disconnect if necessary.
   class Delegate {
    public:
+    // Called when the login attempt is successful.
     virtual void OnConnect(
         base::WeakPtr<buzz::XmppTaskParentInterface> base_task) = 0;
-    virtual void OnNeedReconnect() = 0;
+
+    // Called when the server responds with a redirect.  A new login
+    // attempt should be made to the given redirect server.
     virtual void OnRedirect(const ServerInformation& redirect_server) = 0;
 
+    // Called when a server rejects the client's login credentials.  A
+    // new login attempt should be made once the client provides new
+    // credentials.
+    virtual void OnCredentialsRejected() = 0;
+
+    // Called when no server could be logged into for reasons other
+    // than redirection or rejected credentials.  A new login attempt
+    // may be created, but it should be done with exponential backoff.
+    virtual void OnSettingsExhausted() = 0;
+
    protected:
     virtual ~Delegate();
   };
diff --git a/jingle/notifier/communicator/single_login_attempt_unittest.cc b/jingle/notifier/communicator/single_login_attempt_unittest.cc
index 700b117..9076015 100644
--- a/jingle/notifier/communicator/single_login_attempt_unittest.cc
+++ b/jingle/notifier/communicator/single_login_attempt_unittest.cc
@@ -14,10 +14,10 @@
 #include "jingle/notifier/communicator/login_settings.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/url_request/url_request_test_util.h"
-#include "testing/gtest/include/gtest/gtest.h"
 #include "talk/xmllite/xmlelement.h"
 #include "talk/xmpp/constants.h"
 #include "talk/xmpp/xmppengine.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace buzz {
 class XmppTaskParentInterface;
@@ -27,26 +27,33 @@ namespace notifier {
 
 namespace {
 
-enum DelegateState { IDLE, CONNECTED, NEED_RECONNECT, REDIRECTED };
+enum DelegateState {
+  IDLE, CONNECTED, REDIRECTED, CREDENTIALS_REJECTED, SETTINGS_EXHAUSTED
+};
 
 class FakeDelegate : public SingleLoginAttempt::Delegate {
  public:
   FakeDelegate() : state_(IDLE) {}
 
-  void OnConnect(base::WeakPtr<buzz::XmppTaskParentInterface> base_task) {
+  void OnConnect(
+      base::WeakPtr<buzz::XmppTaskParentInterface> base_task) OVERRIDE {
     state_ = CONNECTED;
     base_task_ = base_task;
   }
 
-  virtual void OnNeedReconnect() {
-    state_ = NEED_RECONNECT;
-  }
-
   virtual void OnRedirect(const ServerInformation& redirect_server) OVERRIDE {
     state_ = REDIRECTED;
     redirect_server_ = redirect_server;
   }
 
+  virtual void OnCredentialsRejected() OVERRIDE {
+    state_ = CREDENTIALS_REJECTED;
+  }
+
+  virtual void OnSettingsExhausted() OVERRIDE {
+    state_ = SETTINGS_EXHAUSTED;
+  }
+
   DelegateState state() const { return state_; }
 
   base::WeakPtr<buzz::XmppTaskParentInterface> base_task() const {
@@ -115,14 +122,14 @@ TEST_F(SingleLoginAttemptTest, Basic) {
             fake_delegate_.base_task().get());
 }
 
-// Fire OnErrors and make sure the delegate gets the OnNeedReconnect()
-// event.
+// Fire OnErrors and make sure the delegate gets the
+// OnSettingsExhausted() event.
 TEST_F(SingleLoginAttemptTest, Error) {
   for (int i = 0; i < 2; ++i) {
     EXPECT_EQ(IDLE, fake_delegate_.state());
     attempt_.OnError(buzz::XmppEngine::ERROR_NONE, 0, NULL);
   }
-  EXPECT_EQ(NEED_RECONNECT, fake_delegate_.state());
+  EXPECT_EQ(SETTINGS_EXHAUSTED, fake_delegate_.state());
 }
 
 // Fire OnErrors but replace the last one with OnConnect, and make
@@ -233,6 +240,13 @@ TEST_F(SingleLoginAttemptTest, RedirectMissingSeeOtherHost) {
   EXPECT_EQ(IDLE, fake_delegate_.state());
 }
 
+// Fire 'Unauthorized' errors and make sure the delegate gets the
+// OnCredentialsRejected() event.
+TEST_F(SingleLoginAttemptTest, CredentialsRejected) {
+  attempt_.OnError(buzz::XmppEngine::ERROR_UNAUTHORIZED, 0, NULL);
+  EXPECT_EQ(CREDENTIALS_REJECTED, fake_delegate_.state());
+}
+
 }  // namespace
 
 }  // namespace notifier
diff --git a/jingle/notifier/listener/fake_push_client.cc b/jingle/notifier/listener/fake_push_client.cc
index cfe4a99..0565c3c 100644
--- a/jingle/notifier/listener/fake_push_client.cc
+++ b/jingle/notifier/listener/fake_push_client.cc
@@ -35,10 +35,15 @@ void FakePushClient::SendNotification(const Notification& notification) {
   sent_notifications_.push_back(notification);
 }
 
-void FakePushClient::SimulateNotificationStateChange(
-    bool notifications_enabled) {
+void FakePushClient::EnableNotifications() {
   FOR_EACH_OBSERVER(PushClientObserver, observers_,
-                    OnNotificationStateChange(notifications_enabled));
+                    OnNotificationsEnabled());
+}
+
+void FakePushClient::DisableNotifications(
+    NotificationsDisabledReason reason) {
+  FOR_EACH_OBSERVER(PushClientObserver, observers_,
+                    OnNotificationsDisabled(reason));
 }
 
 void FakePushClient::SimulateIncomingNotification(
diff --git a/jingle/notifier/listener/fake_push_client.h b/jingle/notifier/listener/fake_push_client.h
index 61ba72b..90e086c 100644
--- a/jingle/notifier/listener/fake_push_client.h
+++ b/jingle/notifier/listener/fake_push_client.h
@@ -11,6 +11,7 @@
 #include "base/compiler_specific.h"
 #include "base/observer_list.h"
 #include "jingle/notifier/listener/push_client.h"
+#include "jingle/notifier/listener/push_client_observer.h"
 
 namespace notifier {
 
@@ -29,8 +30,11 @@ class FakePushClient : public PushClient {
       const std::string& email, const std::string& token) OVERRIDE;
   virtual void SendNotification(const Notification& notification) OVERRIDE;
 
-  // Triggers OnNotificationStateChange on all observers.
-  void SimulateNotificationStateChange(bool notifications_enabled);
+  // Triggers OnNotificationsEnabled on all observers.
+  void EnableNotifications();
+
+  // Triggers OnNotificationsDisabled on all observers.
+  void DisableNotifications(NotificationsDisabledReason reason);
 
   // Triggers OnIncomingNotification on all observers.
   void SimulateIncomingNotification(const Notification& notification);
diff --git a/jingle/notifier/listener/fake_push_client_observer.cc b/jingle/notifier/listener/fake_push_client_observer.cc
index ec1b9e1..7b907d0 100644
--- a/jingle/notifier/listener/fake_push_client_observer.cc
+++ b/jingle/notifier/listener/fake_push_client_observer.cc
@@ -7,13 +7,17 @@
 namespace notifier {
 
 FakePushClientObserver::FakePushClientObserver()
-    : notifications_enabled_(false) {}
+    :last_notifications_disabled_reason_(DEFAULT_NOTIFICATION_ERROR) {}
 
 FakePushClientObserver::~FakePushClientObserver() {}
 
-void FakePushClientObserver::OnNotificationStateChange(
-    bool notifications_enabled) {
-  notifications_enabled_ = notifications_enabled;
+void FakePushClientObserver::OnNotificationsEnabled() {
+  last_notifications_disabled_reason_ = NO_NOTIFICATION_ERROR;
+}
+
+void FakePushClientObserver::OnNotificationsDisabled(
+    NotificationsDisabledReason reason) {
+  last_notifications_disabled_reason_ = reason;
 }
 
 void FakePushClientObserver::OnIncomingNotification(
@@ -21,8 +25,9 @@ void FakePushClientObserver::OnIncomingNotification(
   last_incoming_notification_ = notification;
 }
 
-bool FakePushClientObserver::notifications_enabled() const {
-  return notifications_enabled_;
+NotificationsDisabledReason
+FakePushClientObserver::last_notifications_disabled_reason() const {
+  return last_notifications_disabled_reason_;
 }
 
 const Notification&
diff --git a/jingle/notifier/listener/fake_push_client_observer.h b/jingle/notifier/listener/fake_push_client_observer.h
index b8ee6d6..d350021 100644
--- a/jingle/notifier/listener/fake_push_client_observer.h
+++ b/jingle/notifier/listener/fake_push_client_observer.h
@@ -17,16 +17,17 @@ class FakePushClientObserver : public PushClientObserver {
   virtual ~FakePushClientObserver();
 
   // PushClientObserver implementation.
-  virtual void OnNotificationStateChange(
-      bool notifications_enabled) OVERRIDE;
+  virtual void OnNotificationsEnabled() OVERRIDE;
+  virtual void OnNotificationsDisabled(
+      NotificationsDisabledReason reason) OVERRIDE;
   virtual void OnIncomingNotification(
       const Notification& notification) OVERRIDE;
 
-  bool notifications_enabled() const;
+  NotificationsDisabledReason last_notifications_disabled_reason() const;
   const Notification& last_incoming_notification() const;
 
  private:
-  bool notifications_enabled_;
+  NotificationsDisabledReason last_notifications_disabled_reason_;
   Notification last_incoming_notification_;
 };
 
diff --git a/jingle/notifier/listener/non_blocking_push_client.cc b/jingle/notifier/listener/non_blocking_push_client.cc
index 23a8402..57ec240 100644
--- a/jingle/notifier/listener/non_blocking_push_client.cc
+++ b/jingle/notifier/listener/non_blocking_push_client.cc
@@ -5,9 +5,9 @@
 #include "jingle/notifier/listener/non_blocking_push_client.h"
 
 #include "base/bind.h"
-#include "base/message_loop_proxy.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/message_loop_proxy.h"
 #include "jingle/notifier/listener/push_client_observer.h"
 
 namespace notifier {
@@ -39,8 +39,9 @@ class NonBlockingPushClient::Core
   void UpdateCredentials(const std::string& email, const std::string& token);
   void SendNotification(const Notification& data);
 
-  virtual void OnNotificationStateChange(
-      bool notifications_enabled) OVERRIDE;
+  virtual void OnNotificationsEnabled() OVERRIDE;
+  virtual void OnNotificationsDisabled(
+      NotificationsDisabledReason reason) OVERRIDE;
   virtual void OnIncomingNotification(
       const Notification& notification) OVERRIDE;
 
@@ -109,13 +110,21 @@ void NonBlockingPushClient::Core::SendNotification(
   delegate_push_client_->SendNotification(notification);
 }
 
-void NonBlockingPushClient::Core::OnNotificationStateChange(
-    bool notifications_enabled) {
+void NonBlockingPushClient::Core::OnNotificationsEnabled() {
   DCHECK(delegate_task_runner_->BelongsToCurrentThread());
   parent_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&NonBlockingPushClient::OnNotificationStateChange,
-                 parent_push_client_, notifications_enabled));
+      base::Bind(&NonBlockingPushClient::OnNotificationsEnabled,
+                 parent_push_client_));
+}
+
+void NonBlockingPushClient::Core::OnNotificationsDisabled(
+    NotificationsDisabledReason reason) {
+  DCHECK(delegate_task_runner_->BelongsToCurrentThread());
+  parent_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&NonBlockingPushClient::OnNotificationsDisabled,
+                 parent_push_client_, reason));
 }
 
 void NonBlockingPushClient::Core::OnIncomingNotification(
@@ -186,11 +195,17 @@ void NonBlockingPushClient::SendNotification(
                  notification));
 }
 
-void NonBlockingPushClient::OnNotificationStateChange(
-    bool notifications_enabled) {
+void NonBlockingPushClient::OnNotificationsEnabled() {
+  DCHECK(non_thread_safe_.CalledOnValidThread());
+  FOR_EACH_OBSERVER(PushClientObserver, observers_,
+                    OnNotificationsEnabled());
+}
+
+void NonBlockingPushClient::OnNotificationsDisabled(
+    NotificationsDisabledReason reason) {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   FOR_EACH_OBSERVER(PushClientObserver, observers_,
-                    OnNotificationStateChange(notifications_enabled));
+                    OnNotificationsDisabled(reason));
 }
 
 void NonBlockingPushClient::OnIncomingNotification(
diff --git a/jingle/notifier/listener/non_blocking_push_client.h b/jingle/notifier/listener/non_blocking_push_client.h
index ca57229..9205d71 100644
--- a/jingle/notifier/listener/non_blocking_push_client.h
+++ b/jingle/notifier/listener/non_blocking_push_client.h
@@ -14,6 +14,7 @@
 #include "base/observer_list.h"
 #include "base/threading/non_thread_safe.h"
 #include "jingle/notifier/listener/push_client.h"
+#include "jingle/notifier/listener/push_client_observer.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -52,7 +53,8 @@ class NonBlockingPushClient : public PushClient {
  private:
   class Core;
 
-  void OnNotificationStateChange(bool notifications_enabled);
+  void OnNotificationsEnabled();
+  void OnNotificationsDisabled(NotificationsDisabledReason reason);
   void OnIncomingNotification(const Notification& notification);
 
   base::NonThreadSafe non_thread_safe_;
diff --git a/jingle/notifier/listener/non_blocking_push_client_unittest.cc b/jingle/notifier/listener/non_blocking_push_client_unittest.cc
index c415225..f9e0282 100644
--- a/jingle/notifier/listener/non_blocking_push_client_unittest.cc
+++ b/jingle/notifier/listener/non_blocking_push_client_unittest.cc
@@ -115,13 +115,17 @@ TEST_F(NonBlockingPushClientTest, SendNotification) {
 // Make sure notification state changes get propagated back to the
 // parent.
 TEST_F(NonBlockingPushClientTest, NotificationStateChange) {
-  EXPECT_FALSE(fake_observer_.notifications_enabled());
-  fake_push_client_->SimulateNotificationStateChange(true);
+  EXPECT_EQ(DEFAULT_NOTIFICATION_ERROR,
+            fake_observer_.last_notifications_disabled_reason());
+  fake_push_client_->EnableNotifications();
   message_loop_.RunAllPending();
-  EXPECT_TRUE(fake_observer_.notifications_enabled());
-  fake_push_client_->SimulateNotificationStateChange(false);
+  EXPECT_EQ(NO_NOTIFICATION_ERROR,
+            fake_observer_.last_notifications_disabled_reason());
+  fake_push_client_->DisableNotifications(
+      NOTIFICATION_CREDENTIALS_REJECTED);
   message_loop_.RunAllPending();
-  EXPECT_FALSE(fake_observer_.notifications_enabled());
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            fake_observer_.last_notifications_disabled_reason());
 }
 
 // Make sure incoming notifications get propagated back to the parent.
diff --git a/jingle/notifier/listener/push_client_observer.h b/jingle/notifier/listener/push_client_observer.h
index 0fe7d46..d51deb2 100644
--- a/jingle/notifier/listener/push_client_observer.h
+++ b/jingle/notifier/listener/push_client_observer.h
@@ -9,18 +9,32 @@
 
 namespace notifier {
 
-// A PushClientObserver is notified whenever an incoming notification
-// is received or when the state of the push client changes.
+enum NotificationsDisabledReason {
+  // There is an underlying transient problem (e.g., network- or
+  // XMPP-related).
+  TRANSIENT_NOTIFICATION_ERROR,
+  DEFAULT_NOTIFICATION_ERROR = TRANSIENT_NOTIFICATION_ERROR,
+  // Our credentials have been rejected.
+  NOTIFICATION_CREDENTIALS_REJECTED,
+  // No error (useful for avoiding keeping a separate bool for
+  // notifications enabled/disabled).
+  NO_NOTIFICATION_ERROR
+};
+
+// A PushClientObserver is notified when notifications are enabled or
+// disabled, and when a notification is received.
 class PushClientObserver {
  protected:
   virtual ~PushClientObserver();
 
  public:
-  // Called when the state of the push client changes.  If
-  // |notifications_enabled| is true, that means notifications can be
-  // sent and received freely.  If it is false, that means no
-  // notifications can be sent or received.
-  virtual void OnNotificationStateChange(bool notifications_enabled) = 0;
+  // Called when notifications are enabled.
+  virtual void OnNotificationsEnabled() = 0;
+
+  // Called when notifications are disabled, with the reason (not
+  // equal to NO_ERROR) in |reason|.
+  virtual void OnNotificationsDisabled(
+      NotificationsDisabledReason reason) = 0;
 
   // Called when a notification is received.  The details of the
   // notification are in |notification|.
diff --git a/jingle/notifier/listener/xmpp_push_client.cc b/jingle/notifier/listener/xmpp_push_client.cc
index b1272f3..66521f2 100644
--- a/jingle/notifier/listener/xmpp_push_client.cc
+++ b/jingle/notifier/listener/xmpp_push_client.cc
@@ -58,11 +58,19 @@ void XmppPushClient::OnConnect(
   }
 }
 
-void XmppPushClient::OnDisconnect() {
+void XmppPushClient::OnTransientDisconnection() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   base_task_.reset();
   FOR_EACH_OBSERVER(PushClientObserver, observers_,
-                    OnNotificationStateChange(false));
+                    OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+}
+
+void XmppPushClient::OnCredentialsRejected() {
+  DCHECK(non_thread_safe_.CalledOnValidThread());
+  base_task_.reset();
+  FOR_EACH_OBSERVER(
+      PushClientObserver, observers_,
+      OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
 }
 
 void XmppPushClient::OnNotificationReceived(
@@ -75,13 +83,13 @@ void XmppPushClient::OnNotificationReceived(
 void XmppPushClient::OnSubscribed() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   FOR_EACH_OBSERVER(PushClientObserver, observers_,
-                    OnNotificationStateChange(true));
+                    OnNotificationsEnabled());
 }
 
 void XmppPushClient::OnSubscriptionError() {
   DCHECK(non_thread_safe_.CalledOnValidThread());
   FOR_EACH_OBSERVER(PushClientObserver, observers_,
-                    OnNotificationStateChange(false));
+                    OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
 }
 
 void XmppPushClient::AddObserver(PushClientObserver* observer) {
diff --git a/jingle/notifier/listener/xmpp_push_client.h b/jingle/notifier/listener/xmpp_push_client.h
index 9c4a6f3..34cede5 100644
--- a/jingle/notifier/listener/xmpp_push_client.h
+++ b/jingle/notifier/listener/xmpp_push_client.h
@@ -10,8 +10,8 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/threading/non_thread_safe.h"
@@ -34,7 +34,7 @@ namespace notifier {
 // This class must be used on a single thread.
 class XmppPushClient :
       public PushClient,
-      public LoginDelegate,
+      public Login::Delegate,
       public PushNotificationsListenTaskDelegate,
       public PushNotificationsSubscribeTaskDelegate {
  public:
@@ -53,7 +53,8 @@ class XmppPushClient :
   // Login::Delegate implementation.
   virtual void OnConnect(
       base::WeakPtr<buzz::XmppTaskParentInterface> base_task) OVERRIDE;
-  virtual void OnDisconnect() OVERRIDE;
+  virtual void OnTransientDisconnection() OVERRIDE;
+  virtual void OnCredentialsRejected() OVERRIDE;
 
   // PushNotificationsListenTaskDelegate implementation.
   virtual void OnNotificationReceived(
diff --git a/jingle/notifier/listener/xmpp_push_client_unittest.cc b/jingle/notifier/listener/xmpp_push_client_unittest.cc
index 6a92bcf..3487479 100644
--- a/jingle/notifier/listener/xmpp_push_client_unittest.cc
+++ b/jingle/notifier/listener/xmpp_push_client_unittest.cc
@@ -24,7 +24,8 @@ using ::testing::StrictMock;
 
 class MockObserver : public PushClientObserver {
  public:
-  MOCK_METHOD1(OnNotificationStateChange, void(bool));
+  MOCK_METHOD0(OnNotificationsEnabled, void());
+  MOCK_METHOD1(OnNotificationsDisabled, void(NotificationsDisabledReason));
   MOCK_METHOD1(OnIncomingNotification, void(const Notification&));
 };
 
@@ -67,7 +68,7 @@ TEST_F(XmppPushClientTest, OnIncomingNotification) {
 // Make sure the XMPP push client notifies its observers of a
 // successful connection properly.
 TEST_F(XmppPushClientTest, ConnectAndSubscribe) {
-  EXPECT_CALL(mock_observer_, OnNotificationStateChange(true));
+  EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
   xmpp_push_client_->OnConnect(fake_base_task_.AsWeakPtr());
   xmpp_push_client_->OnSubscribed();
 }
@@ -75,14 +76,24 @@ TEST_F(XmppPushClientTest, ConnectAndSubscribe) {
 // Make sure the XMPP push client notifies its observers of a
 // terminated connection properly.
 TEST_F(XmppPushClientTest, Disconnect) {
-  EXPECT_CALL(mock_observer_, OnNotificationStateChange(false));
-  xmpp_push_client_->OnDisconnect();
+  EXPECT_CALL(mock_observer_,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+  xmpp_push_client_->OnTransientDisconnection();
+}
+
+// Make sure the XMPP push client notifies its observers of
+// rejected credentials properly.
+TEST_F(XmppPushClientTest, RejectCredentials) {
+  EXPECT_CALL(mock_observer_,
+              OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
+  xmpp_push_client_->OnCredentialsRejected();
 }
 
 // Make sure the XMPP push client notifies its observers of a
 // subscription error properly.
 TEST_F(XmppPushClientTest, SubscriptionError) {
-  EXPECT_CALL(mock_observer_, OnNotificationStateChange(false));
+  EXPECT_CALL(mock_observer_,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
   xmpp_push_client_->OnSubscriptionError();
 }
 
@@ -92,7 +103,7 @@ TEST_F(XmppPushClientTest, SubscriptionError) {
 // TODO(akalin): Figure out how to test that the notification was
 // actually sent.
 TEST_F(XmppPushClientTest, SendNotification) {
-  EXPECT_CALL(mock_observer_, OnNotificationStateChange(true));
+  EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
 
   xmpp_push_client_->OnConnect(fake_base_task_.AsWeakPtr());
   xmpp_push_client_->OnSubscribed();
@@ -109,7 +120,7 @@ TEST_F(XmppPushClientTest, SendNotificationPending) {
 
   Mock::VerifyAndClearExpectations(&mock_observer_);
 
-  EXPECT_CALL(mock_observer_, OnNotificationStateChange(true));
+  EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
 
   xmpp_push_client_->OnConnect(fake_base_task_.AsWeakPtr());
   xmpp_push_client_->OnSubscribed();