[Sync] Avoid unregistering object IDs on shutdown

chrome/browser/sync/profile_sync_service.h

Index: chrome/browser/sync/profile_sync_service.h
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index 3e3e14c44f89d32ea2b5753f4d6710119f068f1c..86a99e2a2c9e721bb25860828723666608f9feb5 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -552,13 +552,25 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // been cleared yet. Virtual for testing purposes.
   virtual bool waiting_for_auth() const;
 
-  // 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.
+  // NOTE(akalin): Invalidations that come in during browser shutdown may get
+  // dropped.  This won't matter once we have an Acknowledge API, though: see
+  // http://crbug.com/78462 and http://crbug.com/124149.
+
+  // Sets the invalidation handler for the given name.  Pass in NULL for
+  // |handler| if you want to remove the handler for the given name.  (This
+  // doesn't unregister the IDs for the given name, though.)  A handler must be
+  // set for at most one name.
   //
-  // The handler -> registered ids map is persisted across restarts of
+  // The handler name -> handler map is persisted across restarts of sync.
+  void SetInvalidationHandler(const std::string& handler_name,

[msw, 2012/08/07 20:09:30]

I'm wondering if using strings is a necessity to avoiding unregistering object
ids on shutdown. Wouldn't it make more sense to just add something like
InvalidationHandlerDestroying(syncer::SyncNotifierObserver* handler) called from
SyncNotifierObserver::~SyncNotifierObserver (or some new
SyncNotifierObserver::Disable/ShutDown function) that would disable the handler
without unregistering its ids?

That would seem to avoid your dangling object IDs concern, avoid the
string->handler map, and simplify client-side logic on destruction.

+                              syncer::SyncNotifierObserver* handler);
+
+  // Updates the set of ObjectIds associated with a given invalidation handler
+  // (via its name). An ID must be registered for at most one handler.
+  //
+  // The handler name -> registered ids map is persisted across restarts of
   // sync.
-  void UpdateRegisteredInvalidationIds(syncer::SyncNotifierObserver* handler,
+  void UpdateRegisteredInvalidationIds(const std::string& handler_name,
                                        const syncer::ObjectIdSet& ids);
 
   // ProfileKeyedService implementation.
@@ -835,9 +847,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   // Factory the backend will use to build the SyncManager.
   syncer::SyncManagerFactory sync_manager_factory_;
 
-  // The set of all registered IDs.
-  syncer::ObjectIdSet all_registered_ids_;
-
   // Dispatches invalidations to handlers.
   syncer::SyncNotifierHelper notifier_helper_;