[Sync] Avoid unregistering object IDs on shutdown

chrome/browser/sync/profile_sync_service.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.
 
 #ifndef CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_H_
 #define CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_H_
 
 #include <list>
 #include <string>
 
@@ skipping 20 matching lines @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_types.h"
 #include "googleurl/src/gurl.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/sync_manager_factory.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/util/unrecoverable_error_handler.h"
 #include "sync/js/sync_js_controller.h"
-#include "sync/notifier/sync_notifier_helper.h"
+#include "sync/notifier/sync_notifier_registrar.h"
 
 class Profile;
 class ProfileSyncComponentsFactory;
 class SigninManager;
 class SyncGlobalError;
 
 namespace browser_sync {
 class BackendMigrator;
 class ChangeProcessor;
 class DataTypeManager;
@@ skipping 493 matching lines @@
     return configure_status_;
   }
 
   // If true, the ProfileSyncService has detected that a new GAIA signin has
   // succeeded, and is waiting for initialization to complete. This is used by
   // the UI to differentiate between a new auth error (encountered as part of
   // the initialization process) and a pre-existing auth error that just hasn't
   // 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.
+  // Invalidation clients should follow the pattern below:

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

This is AWESOME, thank you!

   //
-  // The handler -> registered ids map is persisted across restarts of
-  // sync.
+  // When starting the client:
+  //
+  //   pss->RegisterInvalidationHandler(client_handler);
+  //
+  // When the set of IDs to register changes for the client during its lifetime
+  // (i.e., between calls to RegisterInvalidationHandler(client_handler) and
+  // UnregisterInvalidationHandler(client_handler):
+  //
+  //   pss->UpdateRegisteredInvalidationIds(client_handler, client_ids);
+  //
+  // When shutting down the client for browser shutdown:
+  //
+  //   pss->UnregisterInvalidationHandler(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:
+  //
+  //   pss->UpdateRegisteredInvalidationIds(client_handler, ObjectIdSet());
+  //   pss->UnregisterInvalidationHandler(client_handler)

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

nit: omg won't compile! needs trailing semicolon! :)

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

Done.

+
+  // 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.
+
+  // Starts sending notifications to |handler|.  |handler| must not be NULL,
+  // and it must not already have been added.
+  //
+  // Handler registrations are persisted across restarts of sync.
+  void RegisterInvalidationHandler(syncer::SyncNotifierObserver* handler);
+
+  // Updates the set of ObjectIds associated with |handler|.  |handler| must
+  // not be NULL, and must have already been added.  An ID must be registered

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

nit: s/added/registered/

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

done, also fixed other places

+  // for at most one handler.
+  //
+  // Registered IDs are persisted across restarts of sync.
   void UpdateRegisteredInvalidationIds(syncer::SyncNotifierObserver* handler,
                                        const syncer::ObjectIdSet& ids);
 
+  // Stops sending notifications to |handler|.  |handler| must not be NULL, and
+  // it must have been previously added.  Note that this doesn't unregister the
+  // IDs associated with |handler|.
+  //
+  // Handler registrations are persisted across restarts of sync.
+  void UnregisterInvalidationHandler(syncer::SyncNotifierObserver* handler);
+
   // ProfileKeyedService implementation.
   virtual void Shutdown() OVERRIDE;
 
  protected:
   // Used by test classes that derive from ProfileSyncService.
   virtual browser_sync::SyncBackendHost* GetBackendForTest();
 
   // Helper to install and configure a data type manager.
   void ConfigureDataTypeManager();
 
@@ skipping 254 matching lines @@
   // If |true|, there is setup UI visible so we should not start downloading
   // data types.
   bool setup_in_progress_;
 
   // The set of currently enabled sync experiments.
   syncer::Experiments current_experiments;
 
   // 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_;
+  syncer::SyncNotifierRegistrar notifier_registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileSyncService);
 };
 
 bool ShouldShowActionOnUI(
     const syncer::SyncProtocolError& error);
 
 
 #endif  // CHROME_BROWSER_SYNC_PROFILE_SYNC_SERVICE_H_