Add InvalidationStateTracker::Forget() to erase an entry from storage.

BUG=none


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

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

10 files changed, 61 insertions, 17 deletions

diff --git a/sync/notifier/chrome_invalidation_client.cc b/sync/notifier/chrome_invalidation_client.cc
index e2f406d..938f7e6 100644
--- a/sync/notifier/chrome_invalidation_client.cc
+++ b/sync/notifier/chrome_invalidation_client.cc
@@ -109,9 +109,8 @@ void ChromeInvalidationClient::UpdateRegisteredIds(const ObjectIdSet& ids) {
   // 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_->UpdateRegisteredIds(registered_ids_);
+    DoRegistrationUpdate();
   }
-  // TODO(akalin): Clear invalidation versions for unregistered types.
 }
 
 void ChromeInvalidationClient::Ready(
@@ -120,7 +119,7 @@ void ChromeInvalidationClient::Ready(
   DCHECK_EQ(client, invalidation_client_.get());
   ticl_state_ = NO_NOTIFICATION_ERROR;
   EmitStateChange();
-  registration_manager_->UpdateRegisteredIds(registered_ids_);
+  DoRegistrationUpdate();
 }
 
 void ChromeInvalidationClient::Invalidate(
@@ -244,10 +243,14 @@ void ChromeInvalidationClient::InformRegistrationFailure(
     // |registration_manager_| handle the registration backoff policy.
     registration_manager_->MarkRegistrationLost(object_id);
   } else {
-    // Non-transient failures are permanent, so block any future
-    // registration requests for |model_type|.  (This happens if the
-    // server doesn't recognize the data type, which could happen for
-    // brand-new data types.)
+    // Non-transient failures require an action to resolve. This could happen
+    // because:
+    // - the server doesn't yet recognize the data type, which could happen for
+    //   brand-new data types.
+    // - the user has changed his password and hasn't updated it yet locally.
+    // Either way, block future registration attempts for |object_id|. However,
+    // we don't forget any saved invalidation state since we may use it once the
+    // error is addressed.
     registration_manager_->DisableId(object_id);
   }
 }
@@ -285,6 +288,14 @@ void ChromeInvalidationClient::WriteState(const std::string& state) {
       FROM_HERE, &InvalidationStateTracker::SetInvalidationState, state);
 }
 
+void ChromeInvalidationClient::DoRegistrationUpdate() {
+  DCHECK(CalledOnValidThread());
+  const ObjectIdSet& unregistered_ids =
+      registration_manager_->UpdateRegisteredIds(registered_ids_);
+  invalidation_state_tracker_.Call(
+      FROM_HERE, &InvalidationStateTracker::Forget, unregistered_ids);
+}
+
 void ChromeInvalidationClient::StopForTest() {
   DCHECK(CalledOnValidThread());
   Stop();
diff --git a/sync/notifier/chrome_invalidation_client.h b/sync/notifier/chrome_invalidation_client.h
index fc6c43d..1d2d33d 100644
--- a/sync/notifier/chrome_invalidation_client.h
+++ b/sync/notifier/chrome_invalidation_client.h
@@ -130,6 +130,8 @@ class ChromeInvalidationClient
 
   void StopForTest();
 
+  void DoRegistrationUpdate();
+
  private:
   void Stop();
 
diff --git a/sync/notifier/fake_invalidation_state_tracker.cc b/sync/notifier/fake_invalidation_state_tracker.cc
index 426166b..b4d5e36 100644
--- a/sync/notifier/fake_invalidation_state_tracker.cc
+++ b/sync/notifier/fake_invalidation_state_tracker.cc
@@ -14,6 +14,12 @@ FakeInvalidationStateTracker::FakeInvalidationStateTracker() {}
 
 FakeInvalidationStateTracker::~FakeInvalidationStateTracker() {}
 
+int64 FakeInvalidationStateTracker::GetMaxVersion(
+    const invalidation::ObjectId& id) const {
+  InvalidationVersionMap::const_iterator it = versions_.find(id);
+  return (it == versions_.end()) ? kMinVersion : it->second;
+}
+
 InvalidationVersionMap
 FakeInvalidationStateTracker::GetAllMaxVersions() const {
   return versions_;
@@ -29,10 +35,10 @@ void FakeInvalidationStateTracker::SetMaxVersion(
   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::Forget(const ObjectIdSet& ids) {
+  for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
+    versions_.erase(*it);
+  }
 }
 
 void FakeInvalidationStateTracker::SetInvalidationState(
diff --git a/sync/notifier/fake_invalidation_state_tracker.h b/sync/notifier/fake_invalidation_state_tracker.h
index 4186f70..c1a9a42 100644
--- a/sync/notifier/fake_invalidation_state_tracker.h
+++ b/sync/notifier/fake_invalidation_state_tracker.h
@@ -19,11 +19,13 @@ class FakeInvalidationStateTracker
   FakeInvalidationStateTracker();
   virtual ~FakeInvalidationStateTracker();
 
+  int64 GetMaxVersion(const invalidation::ObjectId& id) const;
+
   // 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 Forget(const ObjectIdSet& ids) OVERRIDE;
   virtual void SetInvalidationState(const std::string& state) OVERRIDE;
   virtual std::string GetInvalidationState() const OVERRIDE;
 
diff --git a/sync/notifier/invalidation_state_tracker.h b/sync/notifier/invalidation_state_tracker.h
index 8f080fa..3105f3a 100644
--- a/sync/notifier/invalidation_state_tracker.h
+++ b/sync/notifier/invalidation_state_tracker.h
@@ -29,6 +29,8 @@ class InvalidationStateTracker {
   // version for |model_type|.
   virtual void SetMaxVersion(const invalidation::ObjectId& id,
                              int64 max_version) = 0;
+  // Removes all state tracked for |ids|.
+  virtual void Forget(const ObjectIdSet& ids) = 0;
 
   // Used by InvalidationClient for persistence. |state| is opaque data we can
   // present back to the client (e.g. after a restart) for it to bootstrap
diff --git a/sync/notifier/registration_manager.cc b/sync/notifier/registration_manager.cc
index 1c94140..4c59e01 100644
--- a/sync/notifier/registration_manager.cc
+++ b/sync/notifier/registration_manager.cc
@@ -66,7 +66,7 @@ RegistrationManager::~RegistrationManager() {
   STLDeleteValues(&registration_statuses_);
 }
 
-void RegistrationManager::UpdateRegisteredIds(const ObjectIdSet& ids) {
+ObjectIdSet RegistrationManager::UpdateRegisteredIds(const ObjectIdSet& ids) {
   DCHECK(CalledOnValidThread());
 
   const ObjectIdSet& old_ids = GetRegisteredIds();
@@ -92,6 +92,8 @@ void RegistrationManager::UpdateRegisteredIds(const ObjectIdSet& ids) {
       TryRegisterId(*it, false /* is-retry */);
     }
   }
+
+  return to_unregister;
 }
 
 void RegistrationManager::MarkRegistrationLost(
diff --git a/sync/notifier/registration_manager.h b/sync/notifier/registration_manager.h
index 9d00365..01ae7b9 100644
--- a/sync/notifier/registration_manager.h
+++ b/sync/notifier/registration_manager.h
@@ -68,8 +68,9 @@ class RegistrationManager : public base::NonThreadSafe {
   virtual ~RegistrationManager();
 
   // Registers all object IDs included in the given set (that are not
-  // already disabled) and unregisters all other object IDs.
-  void UpdateRegisteredIds(const ObjectIdSet& ids);
+  // already disabled) and unregisters all other object IDs. The return value is
+  // the set of IDs that was unregistered.
+  ObjectIdSet 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 4041501..1700000 100644
--- a/sync/notifier/registration_manager_unittest.cc
+++ b/sync/notifier/registration_manager_unittest.cc
@@ -238,13 +238,19 @@ TEST_F(RegistrationManagerTest, UpdateRegisteredIds) {
   EXPECT_TRUE(fake_registration_manager_.GetRegisteredIdsForTest().empty());
   EXPECT_TRUE(fake_invalidation_client_.GetRegisteredIdsForTest().empty());
 
-  fake_registration_manager_.UpdateRegisteredIds(ids);
+  ObjectIdSet expected_unregistered_ids;
+
+  ObjectIdSet unregistered_ids =
+      fake_registration_manager_.UpdateRegisteredIds(ids);
+  EXPECT_EQ(expected_unregistered_ids, unregistered_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_.UpdateRegisteredIds(ids);
+  unregistered_ids = fake_registration_manager_.UpdateRegisteredIds(ids);
+  expected_unregistered_ids.insert(GetIdForIndex(kObjectIdsCount - 2));
+  EXPECT_EQ(expected_unregistered_ids, unregistered_ids);
   EXPECT_EQ(ids, fake_registration_manager_.GetRegisteredIdsForTest());
   EXPECT_EQ(ids, fake_invalidation_client_.GetRegisteredIdsForTest());
 }
diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc
index 9047d48..5226fa9 100644
--- a/sync/tools/sync_client.cc
+++ b/sync/tools/sync_client.cc
@@ -83,6 +83,12 @@ class NullInvalidationStateTracker
             << ObjectIdToString(id) << " to " << max_invalidation_version;
   }
 
+  virtual void Forget(const ObjectIdSet& ids) OVERRIDE {
+    for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
+      VLOG(1) << "Forgetting invalidation state for " << ObjectIdToString(*it);
+    }
+  }
+
   virtual std::string GetInvalidationState() const OVERRIDE {
     return std::string();
   }
diff --git a/sync/tools/sync_listen_notifications.cc b/sync/tools/sync_listen_notifications.cc
index 3f3955c..24789fd7 100644
--- a/sync/tools/sync_listen_notifications.cc
+++ b/sync/tools/sync_listen_notifications.cc
@@ -100,6 +100,12 @@ class NullInvalidationStateTracker
               << ObjectIdToString(id) << " to " << max_invalidation_version;
   }
 
+  virtual void Forget(const ObjectIdSet& ids) OVERRIDE {
+    for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
+      LOG(INFO) << "Forgetting saved state for " << ObjectIdToString(*it);
+    }
+  }
+
   virtual std::string GetInvalidationState() const OVERRIDE {
     return std::string();
   }