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

sync/notifier/registration_manager.h

 // 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.
 //
 // A class that manages the registration of types for server-issued
 // notifications.
 
 #ifndef SYNC_NOTIFIER_REGISTRATION_MANAGER_H_
 #define SYNC_NOTIFIER_REGISTRATION_MANAGER_H_
 
@@ skipping 50 matching lines @@
                    ObjectIdLessThan>
       PendingRegistrationMap;
 
   // Does not take ownership of |invalidation_client_|.
   explicit RegistrationManager(
       invalidation::InvalidationClient* invalidation_client);
 
   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);

[akalin, 2012/08/22 19:57:30]

what do you think of making GetRegisteredIdsForTest() not be just for test, and
moving the calculation of the types to forget to the caller?  I feel like the
return value of this function isn't very intuitive.

Also, I have another CL that needs GetRegisteredIds(), anyway.

[dcheng, 2012/08/24 01:35:25]

I'm not opposed to exposing it publicly.

However, making the caller makes the flow kind of funky:
Caller gets the registered ids and saves it somewhere.
Callee gets the registered ids, then diffs it against the set passed in, updates
registrations, etc.
Caller then does the exact same diff and gets the exact same result set.

I do agree with comment that it's not intuitive though. I'll think about it more
and see if I can come up with something that is better.

[akalin, 2012/08/29 17:33:54]

Yeah, you're right about the funky flow.  Actually, since this is just the
RegistrationManager, which is pretty much an internal class, I think keeping the
unregistered set as a return value is the least bad option.

 
   // Marks the registration for the |id| lost and re-registers
   // it (unless it's disabled).
   void MarkRegistrationLost(const invalidation::ObjectId& id);
 
   // Marks registrations lost for all enabled object IDs and re-registers them.
   void MarkAllRegistrationsLost();
 
   // Marks the registration for the |id| permanently lost and blocks any future
   // registration attempts.
   void DisableId(const invalidation::ObjectId& id);

[akalin, 2012/08/22 19:57:30]

should we also Forget the ID if we call this?

[dcheng, 2012/08/24 01:35:25]

I think we probably still want to keep it in this case. From the documentation
for InvalidationListener:
"For permanent failures, it must not automatically retry without fixing the
situation (e.g., by presenting a dialog box to the user)"

So it seems like permanent failures can still be addressed and fixed (maybe the
user changed his password or something like that)

[akalin, 2012/08/29 17:33:54]

Maybe add a comment to that effect when this is called in
chrome_invalidation_client.cc

[dcheng, 2012/08/31 05:56:47]

Done.

 
   // Calculate exponential backoff.  |jitter| must be Uniform[-1.0, 1.0].
   static double CalculateBackoff(double retry_interval,
                                  double initial_retry_interval,
                                  double min_retry_interval,
                                  double max_retry_interval,
                                  double backoff_exponent,
                                  double jitter,
                                  double max_jitter);
 
@@ skipping 82 matching lines @@
   RegistrationStatusMap registration_statuses_;
   // Weak pointer.
   invalidation::InvalidationClient* invalidation_client_;
 
   DISALLOW_COPY_AND_ASSIGN(RegistrationManager);
 };
 
 }  // namespace syncer
 
 #endif  // SYNC_NOTIFIER_REGISTRATION_MANAGER_H_