[Sync] Avoid unregistering object IDs on shutdown

sync/notifier/sync_notifier.h

Index: sync/notifier/sync_notifier.h
diff --git a/sync/notifier/sync_notifier.h b/sync/notifier/sync_notifier.h
index 80b97b892741fedfa11df17a0c645389d27a5aa9..3df67d1bcee8680907c10ac386c8af633ce6e96e 100644
--- a/sync/notifier/sync_notifier.h
+++ b/sync/notifier/sync_notifier.h
@@ -22,12 +22,46 @@ class SyncNotifier {
   SyncNotifier() {}
   virtual ~SyncNotifier() {}
 
-  // Updates the set of ObjectIds associated with a given |handler|.
-  // Passing an empty ObjectIdSet will unregister |handler|.
-  // There should be at most one handler registered per object id.
+  // Clients should follow the pattern below:

[msw, 2012/08/09 05:20:26]

nit: I wonder if it's good to have this comment in both places, perhaps so.

[akalin, 2012/08/10 01:28:08]

I think it's good to have it in both places for now.  Eventually, we might have
a common interface, which would make a convenient single place to put the
comments.

+  //
+  // When starting the client:
+  //
+  //   notifier->RegisterHandler(client_handler);
+  //
+  // When the set of IDs to register changes for the client during its lifetime
+  // (i.e., between calls to RegisterHandler(client_handler) and
+  // UnregisterHandler(client_handler):
+  //
+  //   notifier->UpdateRegisteredIds(client_handler, client_ids);
+  //
+  // When shutting down the client for browser shutdown:
+  //
+  //   notifier->UnregisterHandler(client_handler);
+  //
+  // Note that there's no call to UpdateRegisteredIds() -- this is because the
+  // invalidation API persists registrations across browser restarts.
+  //
+  // When permanently shutting down the client, e.g. when disabling the related
+  // feature:
+  //
+  //   notifier->UpdateRegisteredIds(client_handler, ObjectIdSet());
+  //   notifier->UnregisterHandler(client_handler)

[msw, 2012/08/09 05:20:26]

ditto: add trailing semicolon.

[akalin, 2012/08/10 01:28:08]

Done.

+
+  // Starts sending notifications to |handler|.  |handler| must not be NULL,
+  // and it must not already have been added.
+  virtual void RegisterHandler(SyncNotifierObserver* handler) = 0;
+
+  // Updates the set of ObjectIds associated with |handler|.  |handler| must
+  // not be NULL, and must have already been added.  An ID must be registered
+  // for at most one handler.
   virtual void UpdateRegisteredIds(SyncNotifierObserver* handler,
                                    const ObjectIdSet& ids) = 0;
 
+  // Stops sending notifications to |handler|.  |handler| must not be NULL, and
+  // it must have been previously added.  Note that this doesn't unregister the

[msw, 2012/08/09 05:20:26]

ditto: s/added/registered/

[akalin, 2012/08/10 01:28:08]

Done.

+  // IDs associated with |handler|.
+  virtual void UnregisterHandler(SyncNotifierObserver* handler) = 0;
+
   // SetUniqueId must be called once, before any call to
   // UpdateCredentials.  |unique_id| should be a non-empty globally
   // unique string.