[Sync] Rework unit tests for ChromeInvalidationClient

In particular, add unit tests that would have caught bug 139424.

Dep-inject InvalidationClient into ChromeInvalidationClient.

Use the function name 'UpdateRegisteredIds' consistently.

Replace some mocks with fakes.

BUG=139424


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

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

13 files changed, 803 insertions, 317 deletions

diff --git a/sync/notifier/chrome_invalidation_client.cc b/sync/notifier/chrome_invalidation_client.cc
index 8b9cd4f..6ec4659 100644
--- a/sync/notifier/chrome_invalidation_client.cc
+++ b/sync/notifier/chrome_invalidation_client.cc
@@ -7,10 +7,10 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/tracked_objects.h"
-#include "google/cacheinvalidation/include/invalidation-client-factory.h"
 #include "google/cacheinvalidation/include/invalidation-client.h"
 #include "google/cacheinvalidation/include/types.h"
 #include "google/cacheinvalidation/types.pb.h"
@@ -48,6 +48,8 @@ ChromeInvalidationClient::~ChromeInvalidationClient() {
 }
 
 void ChromeInvalidationClient::Start(
+    const CreateInvalidationClientCallback&
+        create_invalidation_client_callback,
     const std::string& client_id, const std::string& client_info,
     const std::string& state,
     const InvalidationVersionMap& initial_max_invalidation_versions,
@@ -85,7 +87,7 @@ void ChromeInvalidationClient::Start(
 
   int client_type = ipc::invalidation::ClientType::CHROME_SYNC;
   invalidation_client_.reset(
-      invalidation::CreateInvalidationClient(
+      create_invalidation_client_callback.Run(
           &chrome_system_resources_, client_type, client_id,
           kApplicationName, this));
   invalidation_client_->Start();
@@ -100,23 +102,25 @@ void ChromeInvalidationClient::UpdateCredentials(
   chrome_system_resources_.network()->UpdateCredentials(email, token);
 }
 
-void ChromeInvalidationClient::RegisterIds(const ObjectIdSet& ids) {
+void ChromeInvalidationClient::UpdateRegisteredIds(const ObjectIdSet& ids) {
   DCHECK(CalledOnValidThread());
   registered_ids_ = ids;
   // |ticl_state_| can go to NO_NOTIFICATION_ERROR even without a
   // working XMPP connection (as observed by us), so check it instead
   // of GetState() (see http://crbug.com/139424).
   if (ticl_state_ == NO_NOTIFICATION_ERROR && registration_manager_.get()) {
-    registration_manager_->SetRegisteredIds(registered_ids_);
+    registration_manager_->UpdateRegisteredIds(registered_ids_);
   }
   // TODO(akalin): Clear invalidation versions for unregistered types.
 }
 
 void ChromeInvalidationClient::Ready(
     invalidation::InvalidationClient* client) {
+  DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   ticl_state_ = NO_NOTIFICATION_ERROR;
   EmitStateChange();
-  registration_manager_->SetRegisteredIds(registered_ids_);
+  registration_manager_->UpdateRegisteredIds(registered_ids_);
 }
 
 void ChromeInvalidationClient::Invalidate(
@@ -124,6 +128,7 @@ void ChromeInvalidationClient::Invalidate(
     const invalidation::Invalidation& invalidation,
     const invalidation::AckHandle& ack_handle) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   DVLOG(1) << "Invalidate: " << InvalidationToString(invalidation);
 
   const invalidation::ObjectId& id = invalidation.object_id();
@@ -170,6 +175,7 @@ void ChromeInvalidationClient::InvalidateUnknownVersion(
     const invalidation::ObjectId& object_id,
     const invalidation::AckHandle& ack_handle) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   DVLOG(1) << "InvalidateUnknownVersion";
 
   ObjectIdPayloadMap id_payloads;
@@ -186,6 +192,7 @@ void ChromeInvalidationClient::InvalidateAll(
     invalidation::InvalidationClient* client,
     const invalidation::AckHandle& ack_handle) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   DVLOG(1) << "InvalidateAll";
 
   ObjectIdPayloadMap id_payloads;
@@ -210,6 +217,7 @@ void ChromeInvalidationClient::InformRegistrationStatus(
       const invalidation::ObjectId& object_id,
       InvalidationListener::RegistrationState new_state) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   DVLOG(1) << "InformRegistrationStatus: "
            << ObjectIdToString(object_id) << " " << new_state;
 
@@ -225,6 +233,7 @@ void ChromeInvalidationClient::InformRegistrationFailure(
     bool is_transient,
     const std::string& error_message) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   DVLOG(1) << "InformRegistrationFailure: "
            << ObjectIdToString(object_id)
            << "is_transient=" << is_transient
@@ -248,6 +257,7 @@ void ChromeInvalidationClient::ReissueRegistrations(
     const std::string& prefix,
     int prefix_length) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   DVLOG(1) << "AllRegistrationsLost";
   registration_manager_->MarkAllRegistrationsLost();
 }
@@ -255,6 +265,8 @@ void ChromeInvalidationClient::ReissueRegistrations(
 void ChromeInvalidationClient::InformError(
     invalidation::InvalidationClient* client,
     const invalidation::ErrorInfo& error_info) {
+  DCHECK(CalledOnValidThread());
+  DCHECK_EQ(client, invalidation_client_.get());
   LOG(ERROR) << "Ticl error " << error_info.error_reason() << ": "
              << error_info.error_message()
              << " (transient = " << error_info.is_transient() << ")";
@@ -273,6 +285,11 @@ void ChromeInvalidationClient::WriteState(const std::string& state) {
       FROM_HERE, &InvalidationStateTracker::SetInvalidationState, state);
 }
 
+void ChromeInvalidationClient::StopForTest() {
+  DCHECK(CalledOnValidThread());
+  Stop();
+}
+
 void ChromeInvalidationClient::Stop() {
   DCHECK(CalledOnValidThread());
   if (!invalidation_client_.get()) {
diff --git a/sync/notifier/chrome_invalidation_client.h b/sync/notifier/chrome_invalidation_client.h
index 2328f99a..e05808c 100644
--- a/sync/notifier/chrome_invalidation_client.h
+++ b/sync/notifier/chrome_invalidation_client.h
@@ -11,6 +11,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -44,6 +45,13 @@ class ChromeInvalidationClient
       public notifier::PushClientObserver,
       public base::NonThreadSafe {
  public:
+  typedef base::Callback<invalidation::InvalidationClient*(
+      invalidation::SystemResources*,
+      int,
+      const invalidation::string&,
+      const invalidation::string&,
+      invalidation::InvalidationListener*)> CreateInvalidationClientCallback;
+
   class Listener {
    public:
     virtual ~Listener();
@@ -65,6 +73,8 @@ class ChromeInvalidationClient
   // Does not take ownership of |listener| or |state_writer|.
   // |invalidation_state_tracker| must be initialized.
   void Start(
+      const CreateInvalidationClientCallback&
+          create_invalidation_client_callback,
       const std::string& client_id, const std::string& client_info,
       const std::string& state,
       const InvalidationVersionMap& initial_max_invalidation_versions,
@@ -73,9 +83,9 @@ class ChromeInvalidationClient
 
   void UpdateCredentials(const std::string& email, const std::string& token);
 
-  // Register the object IDs that we're interested in getting
+  // Update the set of object IDs that we're interested in getting
   // notifications for.  May be called at any time.
-  void RegisterIds(const ObjectIdSet& ids);
+  void UpdateRegisteredIds(const ObjectIdSet& ids);
 
   // invalidation::InvalidationListener implementation.
   virtual void Ready(
@@ -118,9 +128,9 @@ class ChromeInvalidationClient
   virtual void OnIncomingNotification(
       const notifier::Notification& notification) OVERRIDE;
 
- private:
-  friend class ChromeInvalidationClientTest;
+  void StopForTest();
 
+ private:
   void Stop();
 
   NotificationsDisabledReason GetState() const;
diff --git a/sync/notifier/chrome_invalidation_client_unittest.cc b/sync/notifier/chrome_invalidation_client_unittest.cc
index 98907a3..1cc9bc5 100644
--- a/sync/notifier/chrome_invalidation_client_unittest.cc
+++ b/sync/notifier/chrome_invalidation_client_unittest.cc
@@ -2,364 +2,763 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <cstddef>
+#include <set>
 #include <string>
 
+#include "base/compiler_specific.h"
 #include "base/message_loop.h"
 #include "google/cacheinvalidation/include/invalidation-client.h"
 #include "google/cacheinvalidation/include/types.h"
-#include "google/cacheinvalidation/types.pb.h"
 #include "jingle/notifier/listener/fake_push_client.h"
 #include "sync/internal_api/public/util/weak_handle.h"
 #include "sync/notifier/chrome_invalidation_client.h"
-#include "sync/notifier/mock_invalidation_state_tracker.h"
-#include "testing/gmock/include/gmock/gmock.h"
+#include "sync/notifier/fake_invalidation_state_tracker.h"
+#include "sync/notifier/invalidation_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
 
-using ::testing::_;
-using ::testing::InSequence;
-using ::testing::Return;
-using ::testing::StrictMock;
-
 namespace {
 
+using invalidation::AckHandle;
+using invalidation::ObjectId;
+
 const char kClientId[] = "client_id";
 const char kClientInfo[] = "client_info";
+
 const char kState[] = "state";
 const char kNewState[] = "new_state";
 
+const char kPayload1[] = "payload1";
+const char kPayload2[] = "payload2";
+
+const int64 kMinVersion = FakeInvalidationStateTracker::kMinVersion;
+const int64 kVersion1 = 1LL;
+const int64 kVersion2 = 2LL;
+
 const int kChromeSyncSourceId = 1004;
 
-class MockInvalidationClient : public invalidation::InvalidationClient {
- public:
-  MOCK_METHOD0(Start, void());
-  MOCK_METHOD0(Stop, void());
-  MOCK_METHOD1(Register, void(const invalidation::ObjectId&));
-  MOCK_METHOD1(Register, void(const std::vector<invalidation::ObjectId>&));
-  MOCK_METHOD1(Unregister, void(const invalidation::ObjectId&));
-  MOCK_METHOD1(Unregister, void(const std::vector<invalidation::ObjectId>&));
-  MOCK_METHOD1(Acknowledge, void(const invalidation::AckHandle&));
+struct AckHandleLessThan {
+  bool operator()(const AckHandle& lhs, const AckHandle& rhs) const {
+    return lhs.handle_data() < rhs.handle_data();
+  }
 };
 
-class MockListener : public ChromeInvalidationClient::Listener {
+typedef std::set<AckHandle, AckHandleLessThan> AckHandleSet;
+
+// Fake invalidation::InvalidationClient implementation that keeps
+// track of registered IDs and acked handles.
+class FakeInvalidationClient : public invalidation::InvalidationClient {
  public:
-  MOCK_METHOD1(OnInvalidate, void(const ObjectIdPayloadMap&));
-  MOCK_METHOD0(OnNotificationsEnabled, void());
-  MOCK_METHOD1(OnNotificationsDisabled, void(NotificationsDisabledReason));
+  FakeInvalidationClient() : started_(false) {}
+  virtual ~FakeInvalidationClient() {}
+
+  const ObjectIdSet& GetRegisteredIds() const {
+    return registered_ids_;
+  }
+
+  void ClearAckedHandles() {
+    acked_handles_.clear();
+  }
+
+  bool IsAckedHandle(const AckHandle& ack_handle) const {
+    return (acked_handles_.find(ack_handle) != acked_handles_.end());
+  }
+
+  // invalidation::InvalidationClient implementation.
+
+  virtual void Start() OVERRIDE {
+    started_ = true;
+  }
+
+  virtual void Stop() OVERRIDE {
+    started_ = false;
+  }
+
+  virtual void Register(const ObjectId& object_id) OVERRIDE {
+    if (!started_) {
+      ADD_FAILURE();
+      return;
+    }
+    registered_ids_.insert(object_id);
+  }
+
+  virtual void Register(
+      const invalidation::vector<ObjectId>& object_ids) OVERRIDE {
+    if (!started_) {
+      ADD_FAILURE();
+      return;
+    }
+    registered_ids_.insert(object_ids.begin(), object_ids.end());
+  }
+
+  virtual void Unregister(const ObjectId& object_id) OVERRIDE {
+    if (!started_) {
+      ADD_FAILURE();
+      return;
+    }
+    registered_ids_.erase(object_id);
+  }
+
+  virtual void Unregister(
+      const invalidation::vector<ObjectId>& object_ids) OVERRIDE {
+    if (!started_) {
+      ADD_FAILURE();
+      return;
+    }
+    for (invalidation::vector<ObjectId>::const_iterator
+             it = object_ids.begin(); it != object_ids.end(); ++it) {
+      registered_ids_.erase(*it);
+    }
+  }
+
+  virtual void Acknowledge(const AckHandle& ack_handle) OVERRIDE {
+    if (!started_) {
+      ADD_FAILURE();
+      return;
+    }
+    acked_handles_.insert(ack_handle);
+  }
+
+ private:
+  bool started_;
+  ObjectIdSet registered_ids_;
+  AckHandleSet acked_handles_;
 };
 
-ObjectIdSet MakeSetFromId(const invalidation::ObjectId& id) {
-  ObjectIdSet ids;
-  ids.insert(id);
-  return ids;
-}
+// Fake listener tkat keeps track of invalidation counts, payloads,
+// and state.
+class FakeListener : public ChromeInvalidationClient::Listener {
+ public:
+  FakeListener() : reason_(TRANSIENT_NOTIFICATION_ERROR) {}
+  virtual ~FakeListener() {}
 
-ObjectIdPayloadMap ObjectIdsAndPayloadToMap(const ObjectIdSet& ids,
-                                            const std::string& payload) {
-  ObjectIdPayloadMap id_payloads;
-  for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
-    id_payloads[*it] = payload;
+  int GetInvalidationCount(const ObjectId& id) const {
+    ObjectIdCountMap::const_iterator it = invalidation_counts_.find(id);
+    return (it == invalidation_counts_.end()) ? 0 : it->second;
   }
-  return id_payloads;
-}
 
-}  // namespace
+  std::string GetPayload(const ObjectId& id) const {
+    ObjectIdPayloadMap::const_iterator it = payloads_.find(id);
+    return (it == payloads_.end()) ? "" : it->second;
+  }
+
+  // NO_NOTIFICATION_ERROR is the enabled state.
+  NotificationsDisabledReason GetNotificationsDisabledReason() const {
+    return reason_;
+  }
+
+  // ChromeInvalidationClient::Listener implementation.
+
+  virtual void OnInvalidate(const ObjectIdPayloadMap& id_payloads) OVERRIDE {
+    for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin();
+         it != id_payloads.end(); ++it) {
+      ++invalidation_counts_[it->first];
+      payloads_[it->first] = it->second;
+    }
+  }
+
+  virtual void OnNotificationsEnabled() {
+    reason_ = NO_NOTIFICATION_ERROR;
+  }
+
+  virtual void OnNotificationsDisabled(NotificationsDisabledReason reason) {
+    reason_ = reason;
+  }
+
+ private:
+  typedef std::map<ObjectId, int, ObjectIdLessThan> ObjectIdCountMap;
+  ObjectIdCountMap invalidation_counts_;
+  ObjectIdPayloadMap payloads_;
+  NotificationsDisabledReason reason_;
+};
+
+invalidation::InvalidationClient* CreateFakeInvalidationClient(
+    FakeInvalidationClient** fake_invalidation_client,
+    invalidation::SystemResources* resources,
+    int client_type,
+    const invalidation::string& client_name,
+    const invalidation::string& application_name,
+    invalidation::InvalidationListener* listener) {
+  *fake_invalidation_client = new FakeInvalidationClient();
+  return *fake_invalidation_client;
+}
 
 class ChromeInvalidationClientTest : public testing::Test {
  protected:
   ChromeInvalidationClientTest()
-      : fake_push_client_(new notifier::FakePushClient()),
-        client_(scoped_ptr<notifier::PushClient>(fake_push_client_)),
-        kBookmarksId_(kChromeSyncSourceId, "BOOKMARK"),
+      : kBookmarksId_(kChromeSyncSourceId, "BOOKMARK"),
         kPreferencesId_(kChromeSyncSourceId, "PREFERENCE"),
         kExtensionsId_(kChromeSyncSourceId, "EXTENSION"),
-        kAppsId_(kChromeSyncSourceId, "APP") {}
+        kAppsId_(kChromeSyncSourceId, "APP"),
+        fake_push_client_(new notifier::FakePushClient()),
+        fake_invalidation_client_(NULL),
+        client_(scoped_ptr<notifier::PushClient>(fake_push_client_)) {}
 
   virtual void SetUp() {
-    client_.Start(kClientId, kClientInfo, kState,
-                  InvalidationVersionMap(),
-                  MakeWeakHandle(mock_invalidation_state_tracker_.AsWeakPtr()),
-                  &mock_listener_);
+    StartClient();
+
+    registered_ids_.insert(kBookmarksId_);
+    registered_ids_.insert(kPreferencesId_);
+    client_.UpdateRegisteredIds(registered_ids_);
   }
 
   virtual void TearDown() {
-    // client_.Stop() stops the invalidation scheduler, which deletes any
-    // pending tasks without running them.  Some tasks "run and delete" another
-    // task, so they must be run in order to avoid leaking the inner task.
-    // client_.Stop() does not schedule any tasks, so it's both necessary and
-    // sufficient to drain the task queue before calling it.
-    message_loop_.RunAllPending();
-    client_.Stop();
+    StopClient();
   }
 
-  // |payload| can be NULL, but not |type_name|.
-  void FireInvalidate(const char* type_name,
+  // Restart client without re-registering IDs.
+  void RestartClient() {
+    StopClient();
+    StartClient();
+  }
+
+  int GetInvalidationCount(const ObjectId& id) const {
+    return fake_listener_.GetInvalidationCount(id);
+  }
+
+  std::string GetPayload(const ObjectId& id) const {
+    return fake_listener_.GetPayload(id);
+  }
+
+  NotificationsDisabledReason GetNotificationsDisabledReason() const {
+    return fake_listener_.GetNotificationsDisabledReason();
+  }
+
+  int64 GetMaxVersion(const ObjectId& id) const {
+    return fake_tracker_.GetMaxVersion(id);
+  }
+
+  std::string GetInvalidationState() const {
+    return fake_tracker_.GetInvalidationState();
+  }
+
+  ObjectIdSet GetRegisteredIds() const {
+    return fake_invalidation_client_->GetRegisteredIds();
+  }
+
+  // |payload| can be NULL.
+  void FireInvalidate(const ObjectId& object_id,
                       int64 version, const char* payload) {
-    const invalidation::ObjectId object_id(
-        ipc::invalidation::ObjectSource::CHROME_SYNC, type_name);
-    std::string payload_tmp = payload ? payload : "";
     invalidation::Invalidation inv;
     if (payload) {
       inv = invalidation::Invalidation(object_id, version, payload);
     } else {
       inv = invalidation::Invalidation(object_id, version);
     }
-    invalidation::AckHandle ack_handle("fakedata");
-    EXPECT_CALL(mock_invalidation_client_, Acknowledge(ack_handle));
-    client_.Invalidate(&mock_invalidation_client_, inv, ack_handle);
+    const AckHandle ack_handle("fakedata");
+    fake_invalidation_client_->ClearAckedHandles();
+    client_.Invalidate(fake_invalidation_client_, inv, ack_handle);
+    EXPECT_TRUE(fake_invalidation_client_->IsAckedHandle(ack_handle));
     // Pump message loop to trigger
     // InvalidationStateTracker::SetMaxVersion().
     message_loop_.RunAllPending();
   }
 
   // |payload| can be NULL, but not |type_name|.
-  void FireInvalidateUnknownVersion(const char* type_name) {
-    const invalidation::ObjectId object_id(
-        ipc::invalidation::ObjectSource::CHROME_SYNC, type_name);
-
-    invalidation::AckHandle ack_handle("fakedata");
-    EXPECT_CALL(mock_invalidation_client_, Acknowledge(ack_handle));
-    client_.InvalidateUnknownVersion(&mock_invalidation_client_, object_id,
+  void FireInvalidateUnknownVersion(const ObjectId& object_id) {
+    const AckHandle ack_handle("fakedata_unknown");
+    fake_invalidation_client_->ClearAckedHandles();
+    client_.InvalidateUnknownVersion(fake_invalidation_client_, object_id,
                                      ack_handle);
+    EXPECT_TRUE(fake_invalidation_client_->IsAckedHandle(ack_handle));
   }
 
   void FireInvalidateAll() {
-    invalidation::AckHandle ack_handle("fakedata");
-    EXPECT_CALL(mock_invalidation_client_, Acknowledge(ack_handle));
-    client_.InvalidateAll(&mock_invalidation_client_, ack_handle);
+    const AckHandle ack_handle("fakedata_all");
+    fake_invalidation_client_->ClearAckedHandles();
+    client_.InvalidateAll(fake_invalidation_client_, ack_handle);
+    EXPECT_TRUE(fake_invalidation_client_->IsAckedHandle(ack_handle));
+  }
+
+  void WriteState(const std::string& new_state) {
+    client_.WriteState(new_state);
+    // Pump message loop to trigger
+    // InvalidationStateTracker::WriteState().
+    message_loop_.RunAllPending();
+  }
+
+  void EnableNotifications() {
+    fake_push_client_->EnableNotifications();
+  }
+
+  void DisableNotifications(notifier::NotificationsDisabledReason reason) {
+    fake_push_client_->DisableNotifications(reason);
+  }
+
+  const ObjectId kBookmarksId_;
+  const ObjectId kPreferencesId_;
+  const ObjectId kExtensionsId_;
+  const ObjectId kAppsId_;
+
+  ObjectIdSet registered_ids_;
+
+ private:
+  void StartClient() {
+    fake_invalidation_client_ = NULL;
+    client_.Start(base::Bind(&CreateFakeInvalidationClient,
+                             &fake_invalidation_client_),
+                  kClientId, kClientInfo, kState,
+                  InvalidationVersionMap(),
+                  MakeWeakHandle(fake_tracker_.AsWeakPtr()),
+                  &fake_listener_);
+    DCHECK(fake_invalidation_client_);
+  }
+
+  void StopClient() {
+    // client_.StopForTest() stops the invalidation scheduler, which
+    // deletes any pending tasks without running them.  Some tasks
+    // "run and delete" another task, so they must be run in order to
+    // avoid leaking the inner task.  client_.StopForTest() does not
+    // schedule any tasks, so it's both necessary and sufficient to
+    // drain the task queue before calling it.
+    message_loop_.RunAllPending();
+    fake_invalidation_client_ = NULL;
+    client_.StopForTest();
   }
 
   MessageLoop message_loop_;
-  StrictMock<MockListener> mock_listener_;
-  StrictMock<MockInvalidationStateTracker>
-      mock_invalidation_state_tracker_;
-  StrictMock<MockInvalidationClient> mock_invalidation_client_;
+
+  FakeListener fake_listener_;
+  FakeInvalidationStateTracker fake_tracker_;
   notifier::FakePushClient* const fake_push_client_;
-  ChromeInvalidationClient client_;
 
-  const invalidation::ObjectId kBookmarksId_;
-  const invalidation::ObjectId kPreferencesId_;
-  const invalidation::ObjectId kExtensionsId_;
-  const invalidation::ObjectId kAppsId_;
+ protected:
+  // Tests need to access these directly.
+  FakeInvalidationClient* fake_invalidation_client_;
+  ChromeInvalidationClient client_;
 };
 
-// Checks that we still dispatch an invalidation for something that's not
-// currently registered (perhaps it was unregistered while it was still in
-// flight).
-TEST_F(ChromeInvalidationClientTest, InvalidateBadObjectId) {
-  ObjectIdSet ids;
-  ids.insert(kBookmarksId_);
-  ids.insert(kAppsId_);
-  client_.RegisterIds(ids);
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(
-          MakeSetFromId(invalidation::ObjectId(kChromeSyncSourceId, "bad")),
-                        std::string())));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(invalidation::ObjectId(kChromeSyncSourceId, "bad"),
-                            1));
-  FireInvalidate("bad", 1, NULL);
+// Write a new state to the client.  It should propagate to the
+// tracker.
+TEST_F(ChromeInvalidationClientTest, WriteState) {
+  WriteState(kNewState);
+
+  EXPECT_EQ(kNewState, GetInvalidationState());
 }
 
+// Invalidation tests.
+
+// Fire an invalidation without a payload.  It should be processed,
+// the payload should remain empty, and the version should be updated.
 TEST_F(ChromeInvalidationClientTest, InvalidateNoPayload) {
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kBookmarksId_), std::string())));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kBookmarksId_, 1));
-  FireInvalidate("BOOKMARK", 1, NULL);
+  const ObjectId& id = kBookmarksId_;
+
+  FireInvalidate(id, kVersion1, NULL);
+
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ("", GetPayload(id));
+  EXPECT_EQ(kVersion1, GetMaxVersion(id));
+}
+
+// Fire an invalidation with an empty payload.  It should be
+// processed, the payload should remain empty, and the version should
+// be updated.
+TEST_F(ChromeInvalidationClientTest, InvalidateEmptyPayload) {
+  const ObjectId& id = kBookmarksId_;
+
+  FireInvalidate(id, kVersion1, "");
+
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ("", GetPayload(id));
+  EXPECT_EQ(kVersion1, GetMaxVersion(id));
 }
 
+// Fire an invalidation with a payload.  It should be processed, and
+// both the payload and the version should be updated.
 TEST_F(ChromeInvalidationClientTest, InvalidateWithPayload) {
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kPreferencesId_), "payload")));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kPreferencesId_, 1));
-  FireInvalidate("PREFERENCE", 1, "payload");
+  const ObjectId& id = kPreferencesId_;
+
+  FireInvalidate(id, kVersion1, kPayload1);
+
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ(kPayload1, GetPayload(id));
+  EXPECT_EQ(kVersion1, GetMaxVersion(id));
+}
+
+// Fire an invalidation with a payload.  It should still be processed,
+// and both the payload and the version should be updated.
+TEST_F(ChromeInvalidationClientTest, InvalidateUnregistered) {
+  const ObjectId kUnregisteredId(
+      kChromeSyncSourceId, "unregistered");
+  const ObjectId& id = kUnregisteredId;
+
+  EXPECT_EQ(0, GetInvalidationCount(id));
+  EXPECT_EQ("", GetPayload(id));
+  EXPECT_EQ(kMinVersion, GetMaxVersion(id));
+
+  FireInvalidate(id, kVersion1, NULL);
+
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ("", GetPayload(id));
+  EXPECT_EQ(kVersion1, GetMaxVersion(id));
 }
 
+// Fire an invalidation, then fire another one with a lower version.
+// The first one should be processed and should update the payload and
+// version, but the second one shouldn't.
 TEST_F(ChromeInvalidationClientTest, InvalidateVersion) {
-  using ::testing::Mock;
+  const ObjectId& id = kPreferencesId_;
 
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kAppsId_), std::string())));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kAppsId_, 1));
+  FireInvalidate(id, kVersion2, kPayload2);
 
-  // Should trigger.
-  FireInvalidate("APP", 1, NULL);
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ(kPayload2, GetPayload(id));
+  EXPECT_EQ(kVersion2, GetMaxVersion(id));
 
-  Mock::VerifyAndClearExpectations(&mock_listener_);
+  FireInvalidate(id, kVersion1, kPayload1);
 
-  // Should be dropped.
-  FireInvalidate("APP", 1, NULL);
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ(kPayload2, GetPayload(id));
+  EXPECT_EQ(kVersion2, GetMaxVersion(id));
 }
 
+// Fire an invalidation with an unknown version twice.  It shouldn't
+// update the payload or version either time, but it should still be
+// processed.
 TEST_F(ChromeInvalidationClientTest, InvalidateUnknownVersion) {
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kExtensionsId_),
-                               std::string()))).Times(2);
+  const ObjectId& id = kBookmarksId_;
+
+  FireInvalidateUnknownVersion(id);
+
+  EXPECT_EQ(1, GetInvalidationCount(id));
+  EXPECT_EQ("", GetPayload(id));
+  EXPECT_EQ(kMinVersion, GetMaxVersion(id));
+
+  FireInvalidateUnknownVersion(id);
+
+  EXPECT_EQ(2, GetInvalidationCount(id));
+  EXPECT_EQ("", GetPayload(id));
+  EXPECT_EQ(kMinVersion, GetMaxVersion(id));
+}
+
+// Fire an invalidation for all enabled IDs.  It shouldn't update the
+// payload or version, but it should still invalidate the IDs.
+TEST_F(ChromeInvalidationClientTest, InvalidateAll) {
+  FireInvalidateAll();
 
-  // Should trigger twice.
-  FireInvalidateUnknownVersion("EXTENSION");
-  FireInvalidateUnknownVersion("EXTENSION");
+  for (ObjectIdSet::const_iterator it = registered_ids_.begin();
+       it != registered_ids_.end(); ++it) {
+    EXPECT_EQ(1, GetInvalidationCount(*it));
+    EXPECT_EQ("", GetPayload(*it));
+    EXPECT_EQ(kMinVersion, GetMaxVersion(*it));
+  }
 }
 
-// Comprehensive test of various invalidations that we might receive from Tango
-// and how they interact.
-TEST_F(ChromeInvalidationClientTest, InvalidateVersionMultipleTypes) {
-  using ::testing::Mock;
+// Comprehensive test of various scenarios for multiple IDs.
+TEST_F(ChromeInvalidationClientTest, InvalidateMultipleIds) {
+  FireInvalidate(kBookmarksId_, 3, NULL);
+
+  EXPECT_EQ(1, GetInvalidationCount(kBookmarksId_));
+  EXPECT_EQ("", GetPayload(kBookmarksId_));
+  EXPECT_EQ(3, GetMaxVersion(kBookmarksId_));
 
-  ObjectIdSet ids;
-  ids.insert(kBookmarksId_);
-  ids.insert(kAppsId_);
-  client_.RegisterIds(ids);
+  FireInvalidate(kExtensionsId_, 2, NULL);
 
-  // Initial invalidations to the client should be recorded and dispatched to
-  // the listener.
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kAppsId_), std::string())));
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kExtensionsId_), std::string())));
+  EXPECT_EQ(1, GetInvalidationCount(kExtensionsId_));
+  EXPECT_EQ("", GetPayload(kExtensionsId_));
+  EXPECT_EQ(2, GetMaxVersion(kExtensionsId_));
 
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kAppsId_, 3));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kExtensionsId_, 2));
+  // Invalidations with lower version numbers should be ignored.
 
-  FireInvalidate("APP", 3, NULL);
-  FireInvalidate("EXTENSION", 2, NULL);
+  FireInvalidate(kBookmarksId_, 1, NULL);
 
-  Mock::VerifyAndClearExpectations(&mock_listener_);
-  Mock::VerifyAndClearExpectations(&mock_invalidation_state_tracker_);
+  EXPECT_EQ(1, GetInvalidationCount(kBookmarksId_));
+  EXPECT_EQ("", GetPayload(kBookmarksId_));
+  EXPECT_EQ(3, GetMaxVersion(kBookmarksId_));
 
-  // Out-of-order invalidations with lower version numbers should be ignored.
-  FireInvalidate("APP", 1, NULL);
-  FireInvalidate("EXTENSION", 1, NULL);
+  FireInvalidate(kExtensionsId_, 1, NULL);
 
-  Mock::VerifyAndClearExpectations(&mock_listener_);
-  Mock::VerifyAndClearExpectations(&mock_invalidation_state_tracker_);
+  EXPECT_EQ(1, GetInvalidationCount(kExtensionsId_));
+  EXPECT_EQ("", GetPayload(kExtensionsId_));
+  EXPECT_EQ(2, GetMaxVersion(kExtensionsId_));
 
   // InvalidateAll shouldn't change any version state.
-  EXPECT_CALL(mock_listener_,
-              OnInvalidate(ObjectIdsAndPayloadToMap(ids, std::string())));
+
   FireInvalidateAll();
 
-  Mock::VerifyAndClearExpectations(&mock_listener_);
-  Mock::VerifyAndClearExpectations(&mock_invalidation_state_tracker_);
-
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kPreferencesId_), std::string())));
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kExtensionsId_), std::string())));
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(MakeSetFromId(kAppsId_), std::string())));
-
-  // Normal invalidations with monotonically increasing version numbers.
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kPreferencesId_, 5));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kExtensionsId_, 3));
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetMaxVersion(kAppsId_, 4));
-
-  // All three should be triggered.
-  FireInvalidate("PREFERENCE", 5, NULL);
-  FireInvalidate("EXTENSION", 3, NULL);
-  FireInvalidate("APP", 4, NULL);
+  EXPECT_EQ(2, GetInvalidationCount(kBookmarksId_));
+  EXPECT_EQ("", GetPayload(kBookmarksId_));
+  EXPECT_EQ(3, GetMaxVersion(kBookmarksId_));
+
+  EXPECT_EQ(1, GetInvalidationCount(kPreferencesId_));
+  EXPECT_EQ("", GetPayload(kPreferencesId_));
+  EXPECT_EQ(kMinVersion, GetMaxVersion(kPreferencesId_));
+
+  EXPECT_EQ(1, GetInvalidationCount(kExtensionsId_));
+  EXPECT_EQ("", GetPayload(kExtensionsId_));
+  EXPECT_EQ(2, GetMaxVersion(kExtensionsId_));
+
+  // Invalidations with higher version numbers should be processed.
+
+  FireInvalidate(kPreferencesId_, 5, NULL);
+  EXPECT_EQ(2, GetInvalidationCount(kPreferencesId_));
+  EXPECT_EQ("", GetPayload(kPreferencesId_));
+  EXPECT_EQ(5, GetMaxVersion(kPreferencesId_));
+
+  FireInvalidate(kExtensionsId_, 3, NULL);
+  EXPECT_EQ(2, GetInvalidationCount(kExtensionsId_));
+  EXPECT_EQ("", GetPayload(kExtensionsId_));
+  EXPECT_EQ(3, GetMaxVersion(kExtensionsId_));
+
+  FireInvalidate(kBookmarksId_, 4, NULL);
+  EXPECT_EQ(3, GetInvalidationCount(kBookmarksId_));
+  EXPECT_EQ("", GetPayload(kBookmarksId_));
+  EXPECT_EQ(4, GetMaxVersion(kBookmarksId_));
 }
 
-TEST_F(ChromeInvalidationClientTest, InvalidateAll) {
-  ObjectIdSet ids;
-  ids.insert(kPreferencesId_);
-  ids.insert(kExtensionsId_);
-  client_.RegisterIds(ids);
-  EXPECT_CALL(mock_listener_, OnInvalidate(
-      ObjectIdsAndPayloadToMap(ids, std::string())));
-  FireInvalidateAll();
+// Registration tests.
+
+// With IDs already registered, enable notifications then ready the
+// client.  The IDs should be registered only after the client is
+// readied.
+TEST_F(ChromeInvalidationClientTest, RegisterEnableReady) {
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  EnableNotifications();
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
 }
 
-TEST_F(ChromeInvalidationClientTest, RegisterTypes) {
-  ObjectIdSet ids;
-  ids.insert(kPreferencesId_);
-  ids.insert(kExtensionsId_);
-  client_.RegisterIds(ids);
-  // Registered types should be preserved across Stop/Start.
-  TearDown();
-  SetUp();
-  EXPECT_CALL(mock_listener_,OnInvalidate(
-      ObjectIdsAndPayloadToMap(ids, std::string())));
-  FireInvalidateAll();
+// With IDs already registered, ready the client then enable
+// notifications.  The IDs should be registered after the client is
+// readied.
+TEST_F(ChromeInvalidationClientTest, RegisterReadyEnable) {
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+
+  EnableNotifications();
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
 }
 
-TEST_F(ChromeInvalidationClientTest, WriteState) {
-  EXPECT_CALL(mock_invalidation_state_tracker_,
-              SetInvalidationState(kNewState));
-  client_.WriteState(kNewState);
+// Unregister the IDs, enable notifications, re-register the IDs, then
+// ready the client.  The IDs should be registered only after the
+// client is readied.
+TEST_F(ChromeInvalidationClientTest, EnableRegisterReady) {
+  client_.UpdateRegisteredIds(ObjectIdSet());
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  EnableNotifications();
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.UpdateRegisteredIds(registered_ids_);
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
 }
 
-TEST_F(ChromeInvalidationClientTest, StateChangesNotReady) {
-  InSequence dummy;
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+// Unregister the IDs, enable notifications, ready the client, then
+// re-register the IDs.  The IDs should be registered only after the
+// client is readied.
+TEST_F(ChromeInvalidationClientTest, EnableReadyRegister) {
+  client_.UpdateRegisteredIds(ObjectIdSet());
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  EnableNotifications();
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.UpdateRegisteredIds(registered_ids_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+}
+
+// Unregister the IDs, ready the client, enable notifications, then
+// re-register the IDs.  The IDs should be registered only after the
+// client is readied.
+TEST_F(ChromeInvalidationClientTest, ReadyEnableRegister) {
+  client_.UpdateRegisteredIds(ObjectIdSet());
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  EnableNotifications();
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.UpdateRegisteredIds(registered_ids_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+}
 
-  fake_push_client_->DisableNotifications(
+// Unregister the IDs, ready the client, re-register the IDs, then
+// enable notifications. The IDs should be registered only after the
+// client is readied.
+//
+// This test is important: see http://crbug.com/139424.
+TEST_F(ChromeInvalidationClientTest, ReadyRegisterEnable) {
+  client_.UpdateRegisteredIds(ObjectIdSet());
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.UpdateRegisteredIds(registered_ids_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+
+  EnableNotifications();
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+}
+
+// With IDs already registered, ready the client, restart the client,
+// then re-ready it.  The IDs should still be registered.
+TEST_F(ChromeInvalidationClientTest, RegisterTypesPreserved) {
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+
+  RestartClient();
+
+  EXPECT_TRUE(GetRegisteredIds().empty());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(registered_ids_, GetRegisteredIds());
+}
+
+// Without readying the client, disable notifications, then enable
+// them.  The listener should still think notifications are disabled.
+TEST_F(ChromeInvalidationClientTest, EnableNotificationsNotReady) {
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR,
+            GetNotificationsDisabledReason());
+
+  DisableNotifications(
       notifier::TRANSIENT_NOTIFICATION_ERROR);
-  fake_push_client_->DisableNotifications(
+
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR,
+            GetNotificationsDisabledReason());
+
+  DisableNotifications(
       notifier::NOTIFICATION_CREDENTIALS_REJECTED);
-  fake_push_client_->EnableNotifications();
+
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            GetNotificationsDisabledReason());
+
+  EnableNotifications();
+
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR,
+            GetNotificationsDisabledReason());
 }
 
-TEST_F(ChromeInvalidationClientTest, StateChangesReady) {
-  InSequence dummy;
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-  EXPECT_CALL(mock_listener_, OnNotificationsEnabled());
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
-  EXPECT_CALL(mock_listener_, OnNotificationsEnabled());
-
-  fake_push_client_->EnableNotifications();
-  client_.Ready(NULL);
-  fake_push_client_->DisableNotifications(
-      notifier::TRANSIENT_NOTIFICATION_ERROR);
-  fake_push_client_->DisableNotifications(
+// Enable notifications then Ready the invalidation client.  The
+// listener should then be ready.
+TEST_F(ChromeInvalidationClientTest, EnableNotificationsThenReady) {
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+
+  EnableNotifications();
+
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+}
+
+// Ready the invalidation client then enable notifications.  The
+// listener should then be ready.
+TEST_F(ChromeInvalidationClientTest, ReadyThenEnableNotifications) {
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(TRANSIENT_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+
+  EnableNotifications();
+
+  EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+}
+
+// Enable notifications and ready the client.  Then disable
+// notifications with an auth error and re-enable notifications.  The
+// listener should go into an auth error mode and then back out.
+TEST_F(ChromeInvalidationClientTest, PushClientAuthError) {
+  EnableNotifications();
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
+
+  DisableNotifications(
       notifier::NOTIFICATION_CREDENTIALS_REJECTED);
-  fake_push_client_->EnableNotifications();
+
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            GetNotificationsDisabledReason());
+
+  EnableNotifications();
+
+  EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
 }
 
-TEST_F(ChromeInvalidationClientTest, StateChangesAuthError) {
-  InSequence dummy;
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-  EXPECT_CALL(mock_listener_, OnNotificationsEnabled());
-  EXPECT_CALL(mock_listener_,
-              OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED))
-      .Times(4);
-  EXPECT_CALL(mock_listener_, OnNotificationsEnabled());
+// Enable notifications and ready the client.  Then simulate an auth
+// error from the invalidation client.  Simulate some notification
+// events, then re-ready the client.  The listener should go into an
+// auth error mode and come out of it only after the client is ready.
+TEST_F(ChromeInvalidationClientTest, InvalidationClientAuthError) {
+  EnableNotifications();
+  client_.Ready(fake_invalidation_client_);
 
-  fake_push_client_->EnableNotifications();
-  client_.Ready(NULL);
+  EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
 
   client_.InformError(
-      NULL,
+      fake_invalidation_client_,
       invalidation::ErrorInfo(
           invalidation::ErrorReason::AUTH_FAILURE,
           false /* is_transient */,
           "auth error",
           invalidation::ErrorContext()));
-  fake_push_client_->DisableNotifications(
+
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            GetNotificationsDisabledReason());
+
+  DisableNotifications(
       notifier::TRANSIENT_NOTIFICATION_ERROR);
-  fake_push_client_->DisableNotifications(
-      notifier::NOTIFICATION_CREDENTIALS_REJECTED);
-  fake_push_client_->EnableNotifications();
-  client_.Ready(NULL);
+
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            GetNotificationsDisabledReason());
+
+  DisableNotifications(
+      notifier::TRANSIENT_NOTIFICATION_ERROR);
+
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            GetNotificationsDisabledReason());
+
+  EnableNotifications();
+
+  EXPECT_EQ(NOTIFICATION_CREDENTIALS_REJECTED,
+            GetNotificationsDisabledReason());
+
+  client_.Ready(fake_invalidation_client_);
+
+  EXPECT_EQ(NO_NOTIFICATION_ERROR, GetNotificationsDisabledReason());
 }
 
+}  // namespace
+
 }  // namespace syncer
diff --git a/sync/notifier/fake_invalidation_state_tracker.cc b/sync/notifier/fake_invalidation_state_tracker.cc
new file mode 100644
index 0000000..426166b
--- /dev/null
+++ b/sync/notifier/fake_invalidation_state_tracker.cc
@@ -0,0 +1,47 @@
+// Copyright (c) 2012 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.
+
+#include "sync/notifier/fake_invalidation_state_tracker.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+const int64 FakeInvalidationStateTracker::kMinVersion = kint64min;
+
+FakeInvalidationStateTracker::FakeInvalidationStateTracker() {}
+
+FakeInvalidationStateTracker::~FakeInvalidationStateTracker() {}
+
+InvalidationVersionMap
+FakeInvalidationStateTracker::GetAllMaxVersions() const {
+  return versions_;
+}
+
+void FakeInvalidationStateTracker::SetMaxVersion(
+    const invalidation::ObjectId& id, int64 max_version) {
+  InvalidationVersionMap::const_iterator it = versions_.find(id);
+  if ((it != versions_.end()) && (max_version <= it->second)) {
+    ADD_FAILURE();
+    return;
+  }
+  versions_[id] = max_version;
+}
+
+int64 FakeInvalidationStateTracker::GetMaxVersion(
+    const invalidation::ObjectId& id) const {
+  InvalidationVersionMap::const_iterator it = versions_.find(id);
+  return (it == versions_.end()) ? kMinVersion : it->second;
+}
+
+void FakeInvalidationStateTracker::SetInvalidationState(
+    const std::string& state) {
+  state_ = state;
+}
+
+std::string FakeInvalidationStateTracker::GetInvalidationState() const {
+  return state_;
+}
+
+}  // namespace syncer
diff --git a/sync/notifier/fake_invalidation_state_tracker.h b/sync/notifier/fake_invalidation_state_tracker.h
new file mode 100644
index 0000000..4186f70
--- /dev/null
+++ b/sync/notifier/fake_invalidation_state_tracker.h
@@ -0,0 +1,39 @@
+// Copyright (c) 2012 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.
+
+#ifndef SYNC_NOTIFIER_FAKE_INVALIDATION_STATE_TRACKER_H_
+#define SYNC_NOTIFIER_FAKE_INVALIDATION_STATE_TRACKER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "sync/notifier/invalidation_state_tracker.h"
+
+namespace syncer {
+
+// InvalidationStateTracker implementation that simply keeps track of
+// the max versions and invalidation state in memory.
+class FakeInvalidationStateTracker
+    : public InvalidationStateTracker,
+      public base::SupportsWeakPtr<FakeInvalidationStateTracker> {
+ public:
+  FakeInvalidationStateTracker();
+  virtual ~FakeInvalidationStateTracker();
+
+  // InvalidationStateTracker implementation.
+  virtual InvalidationVersionMap GetAllMaxVersions() const OVERRIDE;
+  virtual void SetMaxVersion(const invalidation::ObjectId& id,
+                             int64 max_version) OVERRIDE;
+  int64 GetMaxVersion(const invalidation::ObjectId& id) const;
+  virtual void SetInvalidationState(const std::string& state) OVERRIDE;
+  virtual std::string GetInvalidationState() const OVERRIDE;
+
+  static const int64 kMinVersion;
+
+ private:
+  InvalidationVersionMap versions_;
+  std::string state_;
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_NOTIFIER_FAKE_INVALIDATION_STATE_TRACKER_H_
diff --git a/sync/notifier/invalidation_notifier.cc b/sync/notifier/invalidation_notifier.cc
index 3a72f32..4e67208 100644
--- a/sync/notifier/invalidation_notifier.cc
+++ b/sync/notifier/invalidation_notifier.cc
@@ -4,9 +4,11 @@
 
 #include "sync/notifier/invalidation_notifier.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "base/metrics/histogram.h"
+#include "google/cacheinvalidation/include/invalidation-client-factory.h"
 #include "jingle/notifier/listener/push_client.h"
 #include "net/url_request/url_request_context.h"
 #include "sync/internal_api/public/base/model_type_payload_map.h"
@@ -37,7 +39,9 @@ InvalidationNotifier::~InvalidationNotifier() {
 void InvalidationNotifier::UpdateRegisteredIds(SyncNotifierObserver* handler,
                                                const ObjectIdSet& ids) {
   DCHECK(CalledOnValidThread());
-  invalidation_client_.RegisterIds(helper_.UpdateRegisteredIds(handler, ids));
+  const ObjectIdSet& all_registered_ids =
+      helper_.UpdateRegisteredIds(handler, ids);
+  invalidation_client_.UpdateRegisteredIds(all_registered_ids);
 }
 
 void InvalidationNotifier::SetUniqueId(const std::string& unique_id) {
@@ -71,6 +75,7 @@ void InvalidationNotifier::UpdateCredentials(
     const std::string& email, const std::string& token) {
   if (state_ == STOPPED) {
     invalidation_client_.Start(
+        base::Bind(&invalidation::CreateInvalidationClient),
         invalidation_client_id_, client_info_, invalidation_state_,
         initial_max_invalidation_versions_,
         invalidation_state_tracker_,
diff --git a/sync/notifier/invalidation_notifier_unittest.cc b/sync/notifier/invalidation_notifier_unittest.cc
index 4a07678..ded6bab 100644
--- a/sync/notifier/invalidation_notifier_unittest.cc
+++ b/sync/notifier/invalidation_notifier_unittest.cc
@@ -13,8 +13,8 @@
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/base/model_type_payload_map.h"
 #include "sync/internal_api/public/util/weak_handle.h"
+#include "sync/notifier/fake_invalidation_state_tracker.h"
 #include "sync/notifier/invalidation_state_tracker.h"
-#include "sync/notifier/mock_invalidation_state_tracker.h"
 #include "sync/notifier/mock_sync_notifier_observer.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -48,7 +48,7 @@ class InvalidationNotifierTest : public testing::Test {
             scoped_ptr<notifier::PushClient>(new notifier::FakePushClient()),
             InvalidationVersionMap(),
             initial_invalidation_state,
-            MakeWeakHandle(mock_tracker_.AsWeakPtr()),
+            MakeWeakHandle(fake_tracker_.AsWeakPtr()),
             "fake_client_info"));
   }
 
@@ -63,11 +63,19 @@ class InvalidationNotifierTest : public testing::Test {
     invalidation_notifier_.reset();
   }
 
+  void SetStateDeprecated(const std::string& new_state) {
+    invalidation_notifier_->SetStateDeprecated(new_state);
+    message_loop_.RunAllPending();
+  }
+
+ private:
   MessageLoopForIO message_loop_;
+  notifier::FakeBaseTask fake_base_task_;
+
+ protected:
   scoped_ptr<InvalidationNotifier> invalidation_notifier_;
-  StrictMock<MockInvalidationStateTracker> mock_tracker_;
+  FakeInvalidationStateTracker fake_tracker_;
   StrictMock<MockSyncNotifierObserver> mock_observer_;
-  notifier::FakeBaseTask fake_base_task_;
 };
 
 TEST_F(InvalidationNotifierTest, Basic) {
@@ -88,12 +96,13 @@ TEST_F(InvalidationNotifierTest, Basic) {
               OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
   EXPECT_CALL(mock_observer_,
               OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
-  // Note no expectation on mock_tracker_, as we initialized with
-  // non-empty initial_invalidation_state above.
 
   // TODO(tim): This call should be a no-op, Remove once bug 124140 and
   // associated issues are fixed.
   invalidation_notifier_->SetStateDeprecated("fake_state");
+  // We don't expect |fake_tracker_|'s state to change, as we
+  // initialized with non-empty initial_invalidation_state above.
+  EXPECT_TRUE(fake_tracker_.GetInvalidationState().empty());
   invalidation_notifier_->SetUniqueId("fake_id");
   invalidation_notifier_->UpdateCredentials("foo@bar.com", "fake_token");
 
@@ -110,20 +119,21 @@ TEST_F(InvalidationNotifierTest, Basic) {
 
 TEST_F(InvalidationNotifierTest, MigrateState) {
   CreateAndObserveNotifier(std::string());
-  InSequence dummy;
 
-  EXPECT_CALL(mock_tracker_, SetInvalidationState("fake_state"));
-  invalidation_notifier_->SetStateDeprecated("fake_state");
+  SetStateDeprecated("fake_state");
+  EXPECT_EQ("fake_state", fake_tracker_.GetInvalidationState());
 
   // Should do nothing.
-  invalidation_notifier_->SetStateDeprecated("spurious_fake_state");
+  SetStateDeprecated("spurious_fake_state");
+  EXPECT_EQ("fake_state", fake_tracker_.GetInvalidationState());
 
   // Pretend Chrome shut down.
   ResetNotifier();
 
   CreateAndObserveNotifier("fake_state");
   // Should do nothing.
-  invalidation_notifier_->SetStateDeprecated("more_spurious_fake_state");
+  SetStateDeprecated("more_spurious_fake_state");
+  EXPECT_EQ("fake_state", fake_tracker_.GetInvalidationState());
 }
 
 }  // namespace
diff --git a/sync/notifier/mock_invalidation_state_tracker.cc b/sync/notifier/mock_invalidation_state_tracker.cc
deleted file mode 100644
index 37aabb3..0000000
--- a/sync/notifier/mock_invalidation_state_tracker.cc
+++ /dev/null
@@ -1,12 +0,0 @@
-// Copyright (c) 2012 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.
-
-#include "sync/notifier/mock_invalidation_state_tracker.h"
-
-namespace syncer {
-
-MockInvalidationStateTracker::MockInvalidationStateTracker() {}
-MockInvalidationStateTracker::~MockInvalidationStateTracker() {}
-
-}  // namespace syncer
diff --git a/sync/notifier/mock_invalidation_state_tracker.h b/sync/notifier/mock_invalidation_state_tracker.h
deleted file mode 100644
index e6d992d..0000000
--- a/sync/notifier/mock_invalidation_state_tracker.h
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (c) 2012 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.
-
-#ifndef SYNC_NOTIFIER_MOCK_INVALIDATION_STATE_TRACKER_H_
-#define SYNC_NOTIFIER_MOCK_INVALIDATION_STATE_TRACKER_H_
-
-#include "base/memory/weak_ptr.h"
-#include "sync/notifier/invalidation_state_tracker.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace syncer {
-
-class MockInvalidationStateTracker
-    : public InvalidationStateTracker,
-      public base::SupportsWeakPtr<MockInvalidationStateTracker> {
- public:
-  MockInvalidationStateTracker();
-  virtual ~MockInvalidationStateTracker();
-
-  MOCK_CONST_METHOD0(GetAllMaxVersions, InvalidationVersionMap());
-  MOCK_METHOD2(SetMaxVersion, void(const invalidation::ObjectId&, int64));
-  MOCK_CONST_METHOD0(GetInvalidationState, std::string());
-  MOCK_METHOD1(SetInvalidationState, void(const std::string&));
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_NOTIFIER_MOCK_INVALIDATION_STATE_TRACKER_H_
diff --git a/sync/notifier/registration_manager.cc b/sync/notifier/registration_manager.cc
index 3d20df0..1c94140 100644
--- a/sync/notifier/registration_manager.cc
+++ b/sync/notifier/registration_manager.cc
@@ -66,7 +66,7 @@ RegistrationManager::~RegistrationManager() {
   STLDeleteValues(&registration_statuses_);
 }
 
-void RegistrationManager::SetRegisteredIds(const ObjectIdSet& ids) {
+void RegistrationManager::UpdateRegisteredIds(const ObjectIdSet& ids) {
   DCHECK(CalledOnValidThread());
 
   const ObjectIdSet& old_ids = GetRegisteredIds();
diff --git a/sync/notifier/registration_manager.h b/sync/notifier/registration_manager.h
index 20c753b..9d00365 100644
--- a/sync/notifier/registration_manager.h
+++ b/sync/notifier/registration_manager.h
@@ -69,7 +69,7 @@ class RegistrationManager : public base::NonThreadSafe {
 
   // Registers all object IDs included in the given set (that are not
   // already disabled) and unregisters all other object IDs.
-  void SetRegisteredIds(const ObjectIdSet& ids);
+  void UpdateRegisteredIds(const ObjectIdSet& ids);
 
   // Marks the registration for the |id| lost and re-registers
   // it (unless it's disabled).
diff --git a/sync/notifier/registration_manager_unittest.cc b/sync/notifier/registration_manager_unittest.cc
index e2b9b91..4041501 100644
--- a/sync/notifier/registration_manager_unittest.cc
+++ b/sync/notifier/registration_manager_unittest.cc
@@ -172,7 +172,7 @@ class RegistrationManagerTest : public testing::Test {
   void RunBackoffTest(double jitter) {
     fake_registration_manager_.SetJitter(jitter);
     ObjectIdSet ids = GetSequenceOfIds(kObjectIdsCount);
-    fake_registration_manager_.SetRegisteredIds(ids);
+    fake_registration_manager_.UpdateRegisteredIds(ids);
 
     // Lose some ids.
     ObjectIdSet lost_ids = GetSequenceOfIds(2);
@@ -230,21 +230,21 @@ class RegistrationManagerTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(RegistrationManagerTest);
 };
 
-// Basic test of SetRegisteredIds to make sure we properly register new IDs and
-// unregister any IDs no longer in the set.
-TEST_F(RegistrationManagerTest, SetRegisteredIds) {
+// Basic test of UpdateRegisteredIds to make sure we properly register
+// new IDs and unregister any IDs no longer in the set.
+TEST_F(RegistrationManagerTest, UpdateRegisteredIds) {
   ObjectIdSet ids = GetSequenceOfIds(kObjectIdsCount - 1);
 
   EXPECT_TRUE(fake_registration_manager_.GetRegisteredIdsForTest().empty());
   EXPECT_TRUE(fake_invalidation_client_.GetRegisteredIdsForTest().empty());
 
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
   EXPECT_EQ(ids, fake_registration_manager_.GetRegisteredIdsForTest());
   EXPECT_EQ(ids, fake_invalidation_client_.GetRegisteredIdsForTest());
 
   ids.insert(GetIdForIndex(kObjectIdsCount - 1));
   ids.erase(GetIdForIndex(kObjectIdsCount - 2));
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
   EXPECT_EQ(ids, fake_registration_manager_.GetRegisteredIdsForTest());
   EXPECT_EQ(ids, fake_invalidation_client_.GetRegisteredIdsForTest());
 }
@@ -291,7 +291,7 @@ TEST_F(RegistrationManagerTest, CalculateBackoff) {
 TEST_F(RegistrationManagerTest, MarkRegistrationLost) {
   ObjectIdSet ids = GetSequenceOfIds(kObjectIdsCount);
 
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
   EXPECT_TRUE(
       fake_registration_manager_.GetPendingRegistrationsForTest().empty());
 
@@ -324,11 +324,11 @@ TEST_F(RegistrationManagerTest, MarkRegistrationLostBackoffHigh) {
 }
 
 // Exponential backoff on lost registrations should be reset to zero if
-// SetRegisteredIds is called.
+// UpdateRegisteredIds is called.
 TEST_F(RegistrationManagerTest, MarkRegistrationLostBackoffReset) {
   ObjectIdSet ids = GetSequenceOfIds(kObjectIdsCount);
 
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
 
   // Lose some ids.
   ObjectIdSet lost_ids = GetSequenceOfIds(2);
@@ -347,7 +347,7 @@ TEST_F(RegistrationManagerTest, MarkRegistrationLostBackoffReset) {
       fake_registration_manager_.GetPendingRegistrationsForTest());
 
   // Set ids again.
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
   ExpectPendingRegistrations(
       ObjectIdSet(),
       0.0,
@@ -357,7 +357,7 @@ TEST_F(RegistrationManagerTest, MarkRegistrationLostBackoffReset) {
 TEST_F(RegistrationManagerTest, MarkAllRegistrationsLost) {
   ObjectIdSet ids = GetSequenceOfIds(kObjectIdsCount);
 
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
 
   fake_invalidation_client_.LoseAllRegistrations();
   fake_registration_manager_.MarkAllRegistrationsLost();
@@ -385,12 +385,12 @@ TEST_F(RegistrationManagerTest, MarkAllRegistrationsLost) {
   EXPECT_EQ(ids, fake_invalidation_client_.GetRegisteredIdsForTest());
 }
 
-// IDs that are disabled should not be re-registered by SetRegisteredIds or
+// IDs that are disabled should not be re-registered by UpdateRegisteredIds or
 // automatic re-registration if that registration is lost.
 TEST_F(RegistrationManagerTest, DisableId) {
   ObjectIdSet ids = GetSequenceOfIds(kObjectIdsCount);
 
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
   EXPECT_TRUE(
       fake_registration_manager_.GetPendingRegistrationsForTest().empty());
 
@@ -405,7 +405,7 @@ TEST_F(RegistrationManagerTest, DisableId) {
   EXPECT_EQ(enabled_ids, fake_registration_manager_.GetRegisteredIdsForTest());
   EXPECT_EQ(enabled_ids, fake_invalidation_client_.GetRegisteredIdsForTest());
 
-  fake_registration_manager_.SetRegisteredIds(ids);
+  fake_registration_manager_.UpdateRegisteredIds(ids);
   EXPECT_EQ(enabled_ids, fake_registration_manager_.GetRegisteredIdsForTest());
 
   fake_registration_manager_.MarkRegistrationLost(
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 3583c87..99356d9 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -461,8 +461,8 @@
         'sync_notifier',
       ],
       'sources': [
-        'notifier/mock_invalidation_state_tracker.cc',
-        'notifier/mock_invalidation_state_tracker.h',
+        'notifier/fake_invalidation_state_tracker.cc',
+        'notifier/fake_invalidation_state_tracker.h',
         'notifier/mock_sync_notifier_observer.cc',
         'notifier/mock_sync_notifier_observer.h',
       ],