[Sync] Avoid unregistering object IDs on shutdown

Add RegisterHandler() and UnregisterHandler(), which should be called before and after calls to UpdateRegisteredIds().  Use UnregisterHandler() on
shutdown instead of UpdateRegisteredIds(_, ObjectIdSet()).

Make SyncNotifierHelper non-thread-safe.  Fix test breakages that this revealed.  Also add GetAllRegisteredIds() instead of making it the return value of UpdateRegisteredIds().

Propagate UpdateRegisteredIds()/RegisterHandler()/UnregisterHandler() all
the way up to ProfileSyncService.

Make FakeSyncManager be created on the sync thread.

Clean up SyncBackendHost startup/shutdown behavior a bit.

BUG=140325


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

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

25 files changed, 733 insertions, 417 deletions

diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h
index 0cb6a4c..01d919c 100644
--- a/sync/internal_api/public/sync_manager.h
+++ b/sync/internal_api/public/sync_manager.h
@@ -407,12 +407,19 @@ class SyncManager {
   virtual void UpdateEnabledTypes(
       const ModelTypeSet& enabled_types) = 0;
 
-  // Forwards to the underlying notifier (see
-  // SyncNotifier::UpdateRegisteredIds()).
+  // Forwards to the underlying notifier (see comments in sync_notifier.h).
+  virtual void RegisterInvalidationHandler(
+      SyncNotifierObserver* handler) = 0;
+
+  // Forwards to the underlying notifier (see comments in sync_notifier.h).
   virtual void UpdateRegisteredInvalidationIds(
       SyncNotifierObserver* handler,
       const ObjectIdSet& ids) = 0;
 
+  // Forwards to the underlying notifier (see comments in sync_notifier.h).
+  virtual void UnregisterInvalidationHandler(
+      SyncNotifierObserver* handler) = 0;
+
   // Put the syncer in normal mode ready to perform nudges and polls.
   virtual void StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) = 0;
diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h
index 12f3f70..71fe35e 100644
--- a/sync/internal_api/public/test/fake_sync_manager.h
+++ b/sync/internal_api/public/test/fake_sync_manager.h
@@ -10,7 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "sync/internal_api/public/sync_manager.h"
-#include "sync/notifier/sync_notifier_helper.h"
+#include "sync/notifier/sync_notifier_registrar.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -20,25 +20,23 @@ namespace syncer {
 
 class FakeSyncManager : public SyncManager {
  public:
-  explicit FakeSyncManager();
+  // |initial_sync_ended_types|: The set of types that have initial_sync_ended
+  // set to true. This value will be used by InitialSyncEndedTypes() until the
+  // next configuration is performed.
+  //
+  // |progress_marker_types|: The set of types that have valid progress
+  // markers. This will be used by GetTypesWithEmptyProgressMarkerToken() until
+  // the next configuration is performed.
+  //
+  // |configure_fail_types|: The set of types that will fail
+  // configuration. Once ConfigureSyncer is called, the
+  // |initial_sync_ended_types_| and |progress_marker_types_| will be updated
+  // to include those types that didn't fail.
+  FakeSyncManager(ModelTypeSet initial_sync_ended_types,
+                  ModelTypeSet progress_marker_types,
+                  ModelTypeSet configure_fail_types);
   virtual ~FakeSyncManager();
 
-  // The set of types that have initial_sync_ended set to true. This value will
-  // be used by InitialSyncEndedTypes() until the next configuration is
-  // performed.
-  void set_initial_sync_ended_types(ModelTypeSet types);
-
-  // The set of types that have valid progress markers. This will be used by
-  // GetTypesWithEmptyProgressMarkerToken() until the next configuration is
-  // performed.
-  void set_progress_marker_types(ModelTypeSet types);
-
-  // The set of types that will fail configuration. Once ConfigureSyncer is
-  // called, the |initial_sync_ended_types_| and
-  // |progress_marker_types_| will be updated to include those types
-  // that didn't fail.
-  void set_configure_fail_types(ModelTypeSet types);
-
   // Returns those types that have been cleaned (purged from the directory)
   // since the last call to GetAndResetCleanedTypes(), or since startup if never
   // called.
@@ -63,6 +61,9 @@ class FakeSyncManager : public SyncManager {
   // Posts a method to disable notifications on the sync thread.
   void DisableNotifications(NotificationsDisabledReason reason);
 
+  // Block until the sync thread has finished processing any pending messages.
+  void WaitForSyncThread();
+
   // SyncManager implementation.
   // Note: we treat whatever message loop this is called from as the sync
   // loop for purposes of callbacks.
@@ -94,9 +95,13 @@ class FakeSyncManager : public SyncManager {
   virtual bool PurgePartiallySyncedTypes() OVERRIDE;
   virtual void UpdateCredentials(const SyncCredentials& credentials) OVERRIDE;
   virtual void UpdateEnabledTypes(const ModelTypeSet& types) OVERRIDE;
+  virtual void RegisterInvalidationHandler(
+      SyncNotifierObserver* handler) OVERRIDE;
   virtual void UpdateRegisteredInvalidationIds(
       SyncNotifierObserver* handler,
       const ObjectIdSet& ids) OVERRIDE;
+  virtual void UnregisterInvalidationHandler(
+      SyncNotifierObserver* handler) OVERRIDE;
   virtual void StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) OVERRIDE;
   virtual void SetEncryptionPassphrase(const std::string& passphrase,
@@ -150,7 +155,7 @@ class FakeSyncManager : public SyncManager {
   ModelTypeSet enabled_types_;
 
   // Faked notifier state.
-  SyncNotifierHelper notifier_helper_;
+  SyncNotifierRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeSyncManager);
 };
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 16232f4..28d6b3c 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -396,6 +396,7 @@ void SyncManagerImpl::Init(
   change_delegate_ = change_delegate;
 
   sync_notifier_ = sync_notifier.Pass();
+  sync_notifier_->RegisterHandler(this);
 
   AddObserver(&js_sync_manager_observer_);
   SetJsEventHandler(event_handler);
@@ -733,18 +734,34 @@ void SyncManagerImpl::UpdateCredentials(
 void SyncManagerImpl::UpdateEnabledTypes(
     const ModelTypeSet& enabled_types) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(initialized_);
   sync_notifier_->UpdateRegisteredIds(
       this,
       ModelTypeSetToObjectIdSet(enabled_types));
 }
 
+void SyncManagerImpl::RegisterInvalidationHandler(
+    SyncNotifierObserver* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(initialized_);
+  sync_notifier_->RegisterHandler(handler);
+}
+
 void SyncManagerImpl::UpdateRegisteredInvalidationIds(
     SyncNotifierObserver* handler,
     const ObjectIdSet& ids) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(initialized_);
   sync_notifier_->UpdateRegisteredIds(handler, ids);
 }
 
+void SyncManagerImpl::UnregisterInvalidationHandler(
+    SyncNotifierObserver* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(initialized_);
+  sync_notifier_->UnregisterHandler(handler);
+}
+
 void SyncManagerImpl::SetEncryptionPassphrase(
     const std::string& passphrase,
     bool is_explicit) {
@@ -1216,14 +1233,17 @@ void SyncManagerImpl::ShutdownOnSyncThread() {
 
   RemoveObserver(&debug_info_event_listener_);
 
-  if (sync_notifier_.get()) {
-    sync_notifier_->UpdateRegisteredIds(this, ObjectIdSet());
-  }
+  // |sync_notifier_| and |connection_manager_| may end up being NULL here in
+  // tests (in synchronous initialization mode).
+  //
+  // TODO(akalin): Fix this behavior.
+
+  if (sync_notifier_.get())
+    sync_notifier_->UnregisterHandler(this);
   sync_notifier_.reset();
 
-  if (connection_manager_.get()) {
+  if (connection_manager_.get())
     connection_manager_->RemoveListener(this);
-  }
   connection_manager_.reset();
 
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index d9bd8ab..1c4541f 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -87,9 +87,13 @@ class SyncManagerImpl : public SyncManager,
   virtual void UpdateCredentials(const SyncCredentials& credentials) OVERRIDE;
   virtual void UpdateEnabledTypes(
       const ModelTypeSet& enabled_types) OVERRIDE;
+  virtual void RegisterInvalidationHandler(
+      SyncNotifierObserver* handler) OVERRIDE;
   virtual void UpdateRegisteredInvalidationIds(
       SyncNotifierObserver* handler,
       const ObjectIdSet& ids) OVERRIDE;
+  virtual void UnregisterInvalidationHandler(
+      SyncNotifierObserver* handler) OVERRIDE;
   virtual void StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) OVERRIDE;
   virtual void SetEncryptionPassphrase(const std::string& passphrase,
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index bd2ffef..841ffd0 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -696,8 +696,10 @@ class SyncManagerObserverMock : public SyncManager::Observer {
 
 class SyncNotifierMock : public SyncNotifier {
  public:
+  MOCK_METHOD1(RegisterHandler, void(SyncNotifierObserver*));
   MOCK_METHOD2(UpdateRegisteredIds,
                void(SyncNotifierObserver*, const ObjectIdSet&));
+  MOCK_METHOD1(UnregisterHandler, void(SyncNotifierObserver*));
   MOCK_METHOD1(SetUniqueId, void(const std::string&));
   MOCK_METHOD1(SetStateDeprecated, void(const std::string&));
   MOCK_METHOD2(UpdateCredentials,
@@ -742,6 +744,10 @@ class SyncManagerTest : public testing::Test,
     EXPECT_CALL(*sync_notifier_mock_, SetStateDeprecated(""));
     EXPECT_CALL(*sync_notifier_mock_,
                 UpdateCredentials(credentials.email, credentials.sync_token));
+    EXPECT_CALL(*sync_notifier_mock_, RegisterHandler(_));
+
+    // Called by ShutdownOnSyncThread().
+    EXPECT_CALL(*sync_notifier_mock_, UnregisterHandler(_));
 
     sync_manager_.AddObserver(&observer_);
     EXPECT_CALL(observer_, OnInitializationComplete(_, _, _)).
@@ -782,7 +788,8 @@ class SyncManagerTest : public testing::Test,
 
   void TearDown() {
     sync_manager_.RemoveObserver(&observer_);
-    EXPECT_CALL(*sync_notifier_mock_, UpdateRegisteredIds(_, ObjectIdSet()));
+    // |sync_notifier_mock_| is strict, which ensures we don't do anything but
+    // unregister |sync_manager_| as a handler on shutdown.
     sync_manager_.ShutdownOnSyncThread();
     sync_notifier_mock_ = NULL;
     PumpLoop();
@@ -956,18 +963,28 @@ TEST_F(SyncManagerTest, UpdateEnabledTypes) {
   ModelSafeRoutingInfo routes;
   GetModelSafeRoutingInfo(&routes);
   const ModelTypeSet enabled_types = GetRoutingInfoTypes(routes);
-
   EXPECT_CALL(*sync_notifier_mock_,
               UpdateRegisteredIds(
                   _, ModelTypeSetToObjectIdSet(enabled_types)));
+
   sync_manager_.UpdateEnabledTypes(enabled_types);
 }
 
+TEST_F(SyncManagerTest, RegisterInvalidationHandler) {
+  EXPECT_CALL(*sync_notifier_mock_, RegisterHandler(NULL));
+  sync_manager_.RegisterInvalidationHandler(NULL);
+}
+
 TEST_F(SyncManagerTest, UpdateRegisteredInvalidationIds) {
   EXPECT_CALL(*sync_notifier_mock_, UpdateRegisteredIds(NULL, ObjectIdSet()));
   sync_manager_.UpdateRegisteredInvalidationIds(NULL, ObjectIdSet());
 }
 
+TEST_F(SyncManagerTest, UnregisterInvalidationHandler) {
+  EXPECT_CALL(*sync_notifier_mock_, UnregisterHandler(NULL));
+  sync_manager_.UnregisterInvalidationHandler(NULL);
+}
+
 TEST_F(SyncManagerTest, ProcessJsMessage) {
   const JsArgList kNoArgs;
 
diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc
index 696746d..e6b415a 100644
--- a/sync/internal_api/test/fake_sync_manager.cc
+++ b/sync/internal_api/test/fake_sync_manager.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
@@ -21,22 +22,15 @@
 
 namespace syncer {
 
-FakeSyncManager::FakeSyncManager() {}
+FakeSyncManager::FakeSyncManager(ModelTypeSet initial_sync_ended_types,
+                                 ModelTypeSet progress_marker_types,
+                                 ModelTypeSet configure_fail_types) :
+    initial_sync_ended_types_(initial_sync_ended_types),
+    progress_marker_types_(progress_marker_types),
+    configure_fail_types_(configure_fail_types) {}
 
 FakeSyncManager::~FakeSyncManager() {}
 
-void FakeSyncManager::set_initial_sync_ended_types(ModelTypeSet types) {
-  initial_sync_ended_types_ = types;
-}
-
-void FakeSyncManager::set_progress_marker_types(ModelTypeSet types) {
-  progress_marker_types_ = types;
-}
-
-void FakeSyncManager::set_configure_fail_types(ModelTypeSet types) {
-  configure_fail_types_ = types;
-}
-
 ModelTypeSet FakeSyncManager::GetAndResetCleanedTypes() {
   ModelTypeSet cleaned_types = cleaned_types_;
   cleaned_types_.Clear();
@@ -84,6 +78,24 @@ void FakeSyncManager::DisableNotifications(
   }
 }
 
+namespace {
+
+void DoNothing() {}
+
+}  // namespace
+
+void FakeSyncManager::WaitForSyncThread() {
+  // Post a task to |sync_task_runner_| and block until it runs.
+  base::RunLoop run_loop;
+  if (!sync_task_runner_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&DoNothing),
+      run_loop.QuitClosure())) {
+    NOTREACHED();
+  }
+  run_loop.Run();
+}
+
 void FakeSyncManager::Init(
     const FilePath& database_location,
     const WeakHandle<JsEventHandler>& event_handler,
@@ -148,10 +160,20 @@ void FakeSyncManager::UpdateEnabledTypes(const ModelTypeSet& types) {
   enabled_types_ = types;
 }
 
+void FakeSyncManager::RegisterInvalidationHandler(
+    SyncNotifierObserver* handler) {
+  registrar_.RegisterHandler(handler);
+}
+
 void FakeSyncManager::UpdateRegisteredInvalidationIds(
     SyncNotifierObserver* handler,
     const ObjectIdSet& ids) {
-  notifier_helper_.UpdateRegisteredIds(handler, ids);
+  registrar_.UpdateRegisteredIds(handler, ids);
+}
+
+void FakeSyncManager::UnregisterInvalidationHandler(
+    SyncNotifierObserver* handler) {
+  registrar_.UnregisterHandler(handler);
 }
 
 void FakeSyncManager::StartSyncingNormally(
@@ -264,18 +286,18 @@ void FakeSyncManager::InvalidateOnSyncThread(
     const ObjectIdPayloadMap& id_payloads,
     IncomingNotificationSource source) {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
-  notifier_helper_.DispatchInvalidationsToHandlers(id_payloads, source);
+  registrar_.DispatchInvalidationsToHandlers(id_payloads, source);
 }
 
 void FakeSyncManager::EnableNotificationsOnSyncThread() {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
-  notifier_helper_.EmitOnNotificationsEnabled();
+  registrar_.EmitOnNotificationsEnabled();
 }
 
 void FakeSyncManager::DisableNotificationsOnSyncThread(
     NotificationsDisabledReason reason) {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
-  notifier_helper_.EmitOnNotificationsDisabled(reason);
+  registrar_.EmitOnNotificationsDisabled(reason);
 }
 
 }  // namespace syncer
diff --git a/sync/notifier/invalidation_notifier.cc b/sync/notifier/invalidation_notifier.cc
index 4e67208..1b7ac86 100644
--- a/sync/notifier/invalidation_notifier.cc
+++ b/sync/notifier/invalidation_notifier.cc
@@ -36,12 +36,21 @@ InvalidationNotifier::~InvalidationNotifier() {
   DCHECK(CalledOnValidThread());
 }
 
+void InvalidationNotifier::RegisterHandler(SyncNotifierObserver* handler) {
+  DCHECK(CalledOnValidThread());
+  registrar_.RegisterHandler(handler);
+}
+
 void InvalidationNotifier::UpdateRegisteredIds(SyncNotifierObserver* handler,
                                                const ObjectIdSet& ids) {
   DCHECK(CalledOnValidThread());
-  const ObjectIdSet& all_registered_ids =
-      helper_.UpdateRegisteredIds(handler, ids);
-  invalidation_client_.UpdateRegisteredIds(all_registered_ids);
+  registrar_.UpdateRegisteredIds(handler, ids);
+  invalidation_client_.UpdateRegisteredIds(registrar_.GetAllRegisteredIds());
+}
+
+void InvalidationNotifier::UnregisterHandler(SyncNotifierObserver* handler) {
+  DCHECK(CalledOnValidThread());
+  registrar_.UnregisterHandler(handler);
 }
 
 void InvalidationNotifier::SetUniqueId(const std::string& unique_id) {
@@ -93,18 +102,18 @@ void InvalidationNotifier::SendNotification(ModelTypeSet changed_types) {
 
 void InvalidationNotifier::OnInvalidate(const ObjectIdPayloadMap& id_payloads) {
   DCHECK(CalledOnValidThread());
-  helper_.DispatchInvalidationsToHandlers(id_payloads, REMOTE_NOTIFICATION);
+  registrar_.DispatchInvalidationsToHandlers(id_payloads, REMOTE_NOTIFICATION);
 }
 
 void InvalidationNotifier::OnNotificationsEnabled() {
   DCHECK(CalledOnValidThread());
-  helper_.EmitOnNotificationsEnabled();
+  registrar_.EmitOnNotificationsEnabled();
 }
 
 void InvalidationNotifier::OnNotificationsDisabled(
     NotificationsDisabledReason reason) {
   DCHECK(CalledOnValidThread());
-  helper_.EmitOnNotificationsDisabled(reason);
+  registrar_.EmitOnNotificationsDisabled(reason);
 }
 
 }  // namespace syncer
diff --git a/sync/notifier/invalidation_notifier.h b/sync/notifier/invalidation_notifier.h
index 8d74552..6cbe44d 100644
--- a/sync/notifier/invalidation_notifier.h
+++ b/sync/notifier/invalidation_notifier.h
@@ -23,7 +23,7 @@
 #include "sync/notifier/chrome_invalidation_client.h"
 #include "sync/notifier/invalidation_state_tracker.h"
 #include "sync/notifier/sync_notifier.h"
-#include "sync/notifier/sync_notifier_helper.h"
+#include "sync/notifier/sync_notifier_registrar.h"
 
 namespace notifier {
 class PushClient;
@@ -49,8 +49,10 @@ class InvalidationNotifier
   virtual ~InvalidationNotifier();
 
   // SyncNotifier implementation.
+  virtual void RegisterHandler(SyncNotifierObserver* handler) OVERRIDE;
   virtual void UpdateRegisteredIds(SyncNotifierObserver* handler,
                                    const ObjectIdSet& ids) OVERRIDE;
+  virtual void UnregisterHandler(SyncNotifierObserver* handler) OVERRIDE;
   virtual void SetUniqueId(const std::string& unique_id) OVERRIDE;
   virtual void SetStateDeprecated(const std::string& state) OVERRIDE;
   virtual void UpdateCredentials(
@@ -75,7 +77,7 @@ class InvalidationNotifier
   };
   State state_;
 
-  SyncNotifierHelper helper_;
+  SyncNotifierRegistrar registrar_;
 
   // Passed to |invalidation_client_|.
   const InvalidationVersionMap initial_max_invalidation_versions_;
diff --git a/sync/notifier/invalidation_notifier_unittest.cc b/sync/notifier/invalidation_notifier_unittest.cc
index ded6bab..973c26f 100644
--- a/sync/notifier/invalidation_notifier_unittest.cc
+++ b/sync/notifier/invalidation_notifier_unittest.cc
@@ -33,11 +33,10 @@ class InvalidationNotifierTest : public testing::Test {
       ResetNotifier();
   }
 
-  // Constructs an InvalidationNotifier, places it in
-  // |invalidation_notifier_|, and adds |mock_observer_| as an observer. This
-  // remains in place until either TearDown (automatic) or ResetNotifier
-  // (manual) is called.
-  void CreateAndObserveNotifier(
+  // Constructs an InvalidationNotifier, places it in |invalidation_notifier_|,
+  // and registers |mock_observer_| as a handler. This remains in place until
+  // either TearDown (automatic) or ResetNotifier (manual) is called.
+  void CreateNotifier(
       const std::string& initial_invalidation_state) {
     notifier::NotifierOptions notifier_options;
     // Note: URLRequestContextGetters are ref-counted.
@@ -50,10 +49,11 @@ class InvalidationNotifierTest : public testing::Test {
             initial_invalidation_state,
             MakeWeakHandle(fake_tracker_.AsWeakPtr()),
             "fake_client_info"));
+    invalidation_notifier_->RegisterHandler(&mock_observer_);
   }
 
   void ResetNotifier() {
-    invalidation_notifier_->UpdateRegisteredIds(&mock_observer_, ObjectIdSet());
+    invalidation_notifier_->UnregisterHandler(&mock_observer_);
     // Stopping the invalidation notifier stops its scheduler, which deletes any
     // pending tasks without running them.  Some tasks "run and delete" another
     // task, so they must be run in order to avoid leaking the inner task.
@@ -79,13 +79,11 @@ class InvalidationNotifierTest : public testing::Test {
 };
 
 TEST_F(InvalidationNotifierTest, Basic) {
-  CreateAndObserveNotifier("fake_state");
   InSequence dummy;
 
-  ModelTypeSet models(PREFERENCES, BOOKMARKS, AUTOFILL);
-  invalidation_notifier_->UpdateRegisteredIds(
-      &mock_observer_, ModelTypeSetToObjectIdSet(models));
+  CreateNotifier("fake_state");
 
+  const ModelTypeSet models(PREFERENCES, BOOKMARKS, AUTOFILL);
   const ModelTypePayloadMap& type_payloads =
       ModelTypePayloadMapFromEnumSet(models, "payload");
   EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
@@ -97,6 +95,9 @@ TEST_F(InvalidationNotifierTest, Basic) {
   EXPECT_CALL(mock_observer_,
               OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
 
+  invalidation_notifier_->UpdateRegisteredIds(
+      &mock_observer_, ModelTypeSetToObjectIdSet(models));
+
   // TODO(tim): This call should be a no-op, Remove once bug 124140 and
   // associated issues are fixed.
   invalidation_notifier_->SetStateDeprecated("fake_state");
@@ -118,7 +119,7 @@ TEST_F(InvalidationNotifierTest, Basic) {
 }
 
 TEST_F(InvalidationNotifierTest, MigrateState) {
-  CreateAndObserveNotifier(std::string());
+  CreateNotifier(std::string());
 
   SetStateDeprecated("fake_state");
   EXPECT_EQ("fake_state", fake_tracker_.GetInvalidationState());
@@ -130,7 +131,7 @@ TEST_F(InvalidationNotifierTest, MigrateState) {
   // Pretend Chrome shut down.
   ResetNotifier();
 
-  CreateAndObserveNotifier("fake_state");
+  CreateNotifier("fake_state");
   // Should do nothing.
   SetStateDeprecated("more_spurious_fake_state");
   EXPECT_EQ("fake_state", fake_tracker_.GetInvalidationState());
diff --git a/sync/notifier/non_blocking_invalidation_notifier.cc b/sync/notifier/non_blocking_invalidation_notifier.cc
index ac0f76a..c561c35 100644
--- a/sync/notifier/non_blocking_invalidation_notifier.cc
+++ b/sync/notifier/non_blocking_invalidation_notifier.cc
@@ -4,6 +4,8 @@
 
 #include "sync/notifier/non_blocking_invalidation_notifier.h"
 
+#include <cstddef>
+
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ -89,12 +91,12 @@ void NonBlockingInvalidationNotifier::Core::Initialize(
           initial_invalidation_state,
           invalidation_state_tracker,
           client_info));
+  invalidation_notifier_->RegisterHandler(this);
 }
 
-
 void NonBlockingInvalidationNotifier::Core::Teardown() {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
-  invalidation_notifier_->UpdateRegisteredIds(this, ObjectIdSet());
+  invalidation_notifier_->UnregisterHandler(this);
   invalidation_notifier_.reset();
   network_task_runner_ = NULL;
 }
@@ -183,21 +185,33 @@ NonBlockingInvalidationNotifier::~NonBlockingInvalidationNotifier() {
   }
 }
 
+void NonBlockingInvalidationNotifier::RegisterHandler(
+    SyncNotifierObserver* handler) {
+  DCHECK(parent_task_runner_->BelongsToCurrentThread());
+  registrar_.RegisterHandler(handler);
+}
+
 void NonBlockingInvalidationNotifier::UpdateRegisteredIds(
-    SyncNotifierObserver* handler, const ObjectIdSet& ids) {
+    SyncNotifierObserver* handler,
+    const ObjectIdSet& ids) {
   DCHECK(parent_task_runner_->BelongsToCurrentThread());
-  const ObjectIdSet& all_registered_ids =
-      helper_.UpdateRegisteredIds(handler, ids);
+  registrar_.UpdateRegisteredIds(handler, ids);
   if (!network_task_runner_->PostTask(
           FROM_HERE,
           base::Bind(
               &NonBlockingInvalidationNotifier::Core::UpdateRegisteredIds,
               core_.get(),
-              all_registered_ids))) {
+              registrar_.GetAllRegisteredIds()))) {
     NOTREACHED();
   }
 }
 
+void NonBlockingInvalidationNotifier::UnregisterHandler(
+    SyncNotifierObserver* handler) {
+  DCHECK(parent_task_runner_->BelongsToCurrentThread());
+  registrar_.UnregisterHandler(handler);
+}
+
 void NonBlockingInvalidationNotifier::SetUniqueId(
     const std::string& unique_id) {
   DCHECK(parent_task_runner_->BelongsToCurrentThread());
@@ -241,20 +255,20 @@ void NonBlockingInvalidationNotifier::SendNotification(
 
 void NonBlockingInvalidationNotifier::OnNotificationsEnabled() {
   DCHECK(parent_task_runner_->BelongsToCurrentThread());
-  helper_.EmitOnNotificationsEnabled();
+  registrar_.EmitOnNotificationsEnabled();
 }
 
 void NonBlockingInvalidationNotifier::OnNotificationsDisabled(
     NotificationsDisabledReason reason) {
   DCHECK(parent_task_runner_->BelongsToCurrentThread());
-  helper_.EmitOnNotificationsDisabled(reason);
+  registrar_.EmitOnNotificationsDisabled(reason);
 }
 
 void NonBlockingInvalidationNotifier::OnIncomingNotification(
         const ObjectIdPayloadMap& id_payloads,
         IncomingNotificationSource source) {
   DCHECK(parent_task_runner_->BelongsToCurrentThread());
-  helper_.DispatchInvalidationsToHandlers(id_payloads, source);
+  registrar_.DispatchInvalidationsToHandlers(id_payloads, source);
 }
 
 }  // namespace syncer
diff --git a/sync/notifier/non_blocking_invalidation_notifier.h b/sync/notifier/non_blocking_invalidation_notifier.h
index f76b37b..9577297 100644
--- a/sync/notifier/non_blocking_invalidation_notifier.h
+++ b/sync/notifier/non_blocking_invalidation_notifier.h
@@ -18,8 +18,8 @@
 #include "sync/internal_api/public/util/weak_handle.h"
 #include "sync/notifier/invalidation_state_tracker.h"
 #include "sync/notifier/sync_notifier.h"
-#include "sync/notifier/sync_notifier_helper.h"
 #include "sync/notifier/sync_notifier_observer.h"
+#include "sync/notifier/sync_notifier_registrar.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -27,6 +27,8 @@ class SingleThreadTaskRunner;
 
 namespace syncer {
 
+// TODO(akalin): Generalize to NonBlockingSyncNotifier
+// (http://crbug.com/140409).
 class NonBlockingInvalidationNotifier
     : public SyncNotifier,
       // SyncNotifierObserver to "observe" our Core via WeakHandle.
@@ -44,8 +46,10 @@ class NonBlockingInvalidationNotifier
   virtual ~NonBlockingInvalidationNotifier();
 
   // SyncNotifier implementation.
+  virtual void RegisterHandler(SyncNotifierObserver* handler) OVERRIDE;
   virtual void UpdateRegisteredIds(SyncNotifierObserver* handler,
                                    const ObjectIdSet& ids) OVERRIDE;
+  virtual void UnregisterHandler(SyncNotifierObserver* handler) OVERRIDE;
   virtual void SetUniqueId(const std::string& unique_id) OVERRIDE;
   virtual void SetStateDeprecated(const std::string& state) OVERRIDE;
   virtual void UpdateCredentials(
@@ -65,7 +69,7 @@ class NonBlockingInvalidationNotifier
 
   base::WeakPtrFactory<NonBlockingInvalidationNotifier> weak_ptr_factory_;
 
-  SyncNotifierHelper helper_;
+  SyncNotifierRegistrar registrar_;
 
   // The real guts of NonBlockingInvalidationNotifier, which allows
   // this class to live completely on the parent thread.
diff --git a/sync/notifier/non_blocking_invalidation_notifier_unittest.cc b/sync/notifier/non_blocking_invalidation_notifier_unittest.cc
index 0cadd4a..f237cdb 100644
--- a/sync/notifier/non_blocking_invalidation_notifier_unittest.cc
+++ b/sync/notifier/non_blocking_invalidation_notifier_unittest.cc
@@ -46,10 +46,11 @@ class NonBlockingInvalidationNotifierTest : public testing::Test {
             std::string(),  // initial_invalidation_state
             MakeWeakHandle(base::WeakPtr<InvalidationStateTracker>()),
             "fake_client_info"));
+    invalidation_notifier_->RegisterHandler(&mock_observer_);
   }
 
   virtual void TearDown() {
-    invalidation_notifier_->UpdateRegisteredIds(&mock_observer_, ObjectIdSet());
+    invalidation_notifier_->UnregisterHandler(&mock_observer_);
     invalidation_notifier_.reset();
     request_context_getter_ = NULL;
     io_thread_.Stop();
@@ -64,13 +65,12 @@ class NonBlockingInvalidationNotifierTest : public testing::Test {
   notifier::FakeBaseTask fake_base_task_;
 };
 
+// TODO(akalin): Add real unit tests (http://crbug.com/140410).
+
 TEST_F(NonBlockingInvalidationNotifierTest, Basic) {
   InSequence dummy;
 
-  ModelTypeSet models(PREFERENCES, BOOKMARKS, AUTOFILL);
-  invalidation_notifier_->UpdateRegisteredIds(
-      &mock_observer_, ModelTypeSetToObjectIdSet(models));
-
+  const ModelTypeSet models(PREFERENCES, BOOKMARKS, AUTOFILL);
   const ModelTypePayloadMap& type_payloads =
       ModelTypePayloadMapFromEnumSet(models, "payload");
   EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
@@ -82,6 +82,9 @@ TEST_F(NonBlockingInvalidationNotifierTest, Basic) {
   EXPECT_CALL(mock_observer_,
               OnNotificationsDisabled(NOTIFICATION_CREDENTIALS_REJECTED));
 
+  invalidation_notifier_->UpdateRegisteredIds(
+      &mock_observer_, ModelTypeSetToObjectIdSet(models));
+
   invalidation_notifier_->SetStateDeprecated("fake_state");
   invalidation_notifier_->SetUniqueId("fake_id");
   invalidation_notifier_->UpdateCredentials("foo@bar.com", "fake_token");
diff --git a/sync/notifier/p2p_notifier.cc b/sync/notifier/p2p_notifier.cc
index 5d5aeff..dab7221 100644
--- a/sync/notifier/p2p_notifier.cc
+++ b/sync/notifier/p2p_notifier.cc
@@ -157,10 +157,18 @@ P2PNotifier::~P2PNotifier() {
   push_client_->RemoveObserver(this);
 }
 
+void P2PNotifier::RegisterHandler(SyncNotifierObserver* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  registrar_.RegisterHandler(handler);
+}
+
 void P2PNotifier::UpdateRegisteredIds(SyncNotifierObserver* handler,
                                       const ObjectIdSet& ids) {
-  const ModelTypeSet enabled_types = ObjectIdSetToModelTypeSet(
-      helper_.UpdateRegisteredIds(handler, ids));
+  // TODO(akalin): Handle arbitrary object IDs (http://crbug.com/140411).
+  DCHECK(thread_checker_.CalledOnValidThread());
+  registrar_.UpdateRegisteredIds(handler, ids);
+  const ModelTypeSet enabled_types =
+      ObjectIdSetToModelTypeSet(registrar_.GetAllRegisteredIds());
   const ModelTypeSet new_enabled_types =
       Difference(enabled_types, enabled_types_);
   const P2PNotificationData notification_data(
@@ -169,6 +177,11 @@ void P2PNotifier::UpdateRegisteredIds(SyncNotifierObserver* handler,
   enabled_types_ = enabled_types;
 }
 
+void P2PNotifier::UnregisterHandler(SyncNotifierObserver* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  registrar_.UnregisterHandler(handler);
+}
+
 void P2PNotifier::SetUniqueId(const std::string& unique_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   unique_id_ = unique_id;
@@ -207,7 +220,7 @@ void P2PNotifier::OnNotificationsEnabled() {
   DCHECK(thread_checker_.CalledOnValidThread());
   bool just_turned_on = (notifications_enabled_ == false);
   notifications_enabled_ = true;
-  helper_.EmitOnNotificationsEnabled();
+  registrar_.EmitOnNotificationsEnabled();
   if (just_turned_on) {
     const P2PNotificationData notification_data(
         unique_id_, NOTIFY_SELF, enabled_types_);
@@ -218,7 +231,7 @@ void P2PNotifier::OnNotificationsEnabled() {
 void P2PNotifier::OnNotificationsDisabled(
     notifier::NotificationsDisabledReason reason) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  helper_.EmitOnNotificationsDisabled(FromNotifierReason(reason));
+  registrar_.EmitOnNotificationsDisabled(FromNotifierReason(reason));
 }
 
 void P2PNotifier::OnIncomingNotification(
@@ -257,7 +270,7 @@ void P2PNotifier::OnIncomingNotification(
   }
   const ModelTypePayloadMap& type_payloads = ModelTypePayloadMapFromEnumSet(
       notification_data.GetChangedTypes(), std::string());
-  helper_.DispatchInvalidationsToHandlers(
+  registrar_.DispatchInvalidationsToHandlers(
       ModelTypePayloadMapToObjectIdPayloadMap(type_payloads),
       REMOTE_NOTIFICATION);
 }
diff --git a/sync/notifier/p2p_notifier.h b/sync/notifier/p2p_notifier.h
index d2de89d..4457c6f 100644
--- a/sync/notifier/p2p_notifier.h
+++ b/sync/notifier/p2p_notifier.h
@@ -20,7 +20,7 @@
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/notifier/notifications_disabled_reason.h"
 #include "sync/notifier/sync_notifier.h"
-#include "sync/notifier/sync_notifier_helper.h"
+#include "sync/notifier/sync_notifier_registrar.h"
 
 namespace notifier {
 class PushClient;
@@ -96,8 +96,10 @@ class P2PNotifier : public SyncNotifier,
   virtual ~P2PNotifier();
 
   // SyncNotifier implementation
+  virtual void RegisterHandler(SyncNotifierObserver* handler) OVERRIDE;
   virtual void UpdateRegisteredIds(SyncNotifierObserver* handler,
                                    const ObjectIdSet& ids) OVERRIDE;
+  virtual void UnregisterHandler(SyncNotifierObserver* handler) OVERRIDE;
   virtual void SetUniqueId(const std::string& unique_id) OVERRIDE;
   virtual void SetStateDeprecated(const std::string& state) OVERRIDE;
   virtual void UpdateCredentials(
@@ -119,7 +121,7 @@ class P2PNotifier : public SyncNotifier,
 
   base::ThreadChecker thread_checker_;
 
-  SyncNotifierHelper helper_;
+  SyncNotifierRegistrar registrar_;
 
   // The push client.
   scoped_ptr<notifier::PushClient> push_client_;
diff --git a/sync/notifier/p2p_notifier_unittest.cc b/sync/notifier/p2p_notifier_unittest.cc
index 28a73c9..97033ea 100644
--- a/sync/notifier/p2p_notifier_unittest.cc
+++ b/sync/notifier/p2p_notifier_unittest.cc
@@ -28,10 +28,11 @@ class P2PNotifierTest : public testing::Test {
             scoped_ptr<notifier::PushClient>(fake_push_client_),
             NOTIFY_OTHERS),
         next_sent_notification_to_reflect_(0) {
+    p2p_notifier_.RegisterHandler(&mock_observer_);
   }
 
   virtual ~P2PNotifierTest() {
-    p2p_notifier_.UpdateRegisteredIds(&mock_observer_, ObjectIdSet());
+    p2p_notifier_.UnregisterHandler(&mock_observer_);
   }
 
   ModelTypePayloadMap MakePayloadMap(ModelTypeSet types) {
@@ -140,16 +141,16 @@ TEST_F(P2PNotifierTest, P2PNotificationDataNonDefault) {
 // observer should receive only a notification from the call to
 // UpdateEnabledTypes().
 TEST_F(P2PNotifierTest, NotificationsBasic) {
-  ModelTypeSet enabled_types(BOOKMARKS, PREFERENCES);
-
-  p2p_notifier_.UpdateRegisteredIds(&mock_observer_,
-                                    ModelTypeSetToObjectIdSet(enabled_types));
+  const ModelTypeSet enabled_types(BOOKMARKS, PREFERENCES);
 
   EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
   EXPECT_CALL(mock_observer_, OnIncomingNotification(
       ModelTypePayloadMapToObjectIdPayloadMap(MakePayloadMap(enabled_types)),
       REMOTE_NOTIFICATION));
 
+  p2p_notifier_.UpdateRegisteredIds(&mock_observer_,
+                                    ModelTypeSetToObjectIdSet(enabled_types));
+
   p2p_notifier_.SetUniqueId("sender");
 
   const char kEmail[] = "foo@bar.com";
@@ -183,15 +184,9 @@ TEST_F(P2PNotifierTest, NotificationsBasic) {
 // target settings.  The notifications received by the observer should
 // be consistent with the target settings.
 TEST_F(P2PNotifierTest, SendNotificationData) {
-  ModelTypeSet enabled_types(BOOKMARKS, PREFERENCES, THEMES);
-  ModelTypeSet changed_types(THEMES, APPS);
-  ModelTypeSet expected_types(THEMES);
-
-  p2p_notifier_.UpdateRegisteredIds(&mock_observer_,
-                                    ModelTypeSetToObjectIdSet(enabled_types));
-
-  const ModelTypePayloadMap& expected_payload_map =
-      MakePayloadMap(expected_types);
+  const ModelTypeSet enabled_types(BOOKMARKS, PREFERENCES, THEMES);
+  const ModelTypeSet changed_types(THEMES, APPS);
+  const ModelTypeSet expected_types(THEMES);
 
   EXPECT_CALL(mock_observer_, OnNotificationsEnabled());
   EXPECT_CALL(mock_observer_,
@@ -200,6 +195,12 @@ TEST_F(P2PNotifierTest, SendNotificationData) {
                       MakePayloadMap(enabled_types)),
                   REMOTE_NOTIFICATION));
 
+  p2p_notifier_.UpdateRegisteredIds(&mock_observer_,
+                                    ModelTypeSetToObjectIdSet(enabled_types));
+
+  const ModelTypePayloadMap& expected_payload_map =
+      MakePayloadMap(expected_types);
+
   p2p_notifier_.SetUniqueId("sender");
   p2p_notifier_.UpdateCredentials("foo@bar.com", "fake_token");
 
diff --git a/sync/notifier/sync_notifier.h b/sync/notifier/sync_notifier.h
index 80b97b8..9191b90 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:
+  //
+  // 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);
+
+  // Starts sending notifications to |handler|.  |handler| must not be NULL,
+  // and it must already be registered.
+  virtual void RegisterHandler(SyncNotifierObserver* handler) = 0;
+
+  // Updates the set of ObjectIds associated with |handler|.  |handler| must
+  // not be NULL, and must already be registered.  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 already be registered.  Note that this doesn't unregister the 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.
diff --git a/sync/notifier/sync_notifier_factory_unittest.cc b/sync/notifier/sync_notifier_factory_unittest.cc
index 8036c1a..50d923b 100644
--- a/sync/notifier/sync_notifier_factory_unittest.cc
+++ b/sync/notifier/sync_notifier_factory_unittest.cc
@@ -61,8 +61,9 @@ TEST_F(SyncNotifierFactoryTest, Basic) {
 #else
   ASSERT_TRUE(notifier.get());
   ObjectIdSet ids = ModelTypeSetToObjectIdSet(ModelTypeSet(syncer::BOOKMARKS));
+  notifier->RegisterHandler(&mock_observer_);
   notifier->UpdateRegisteredIds(&mock_observer_, ids);
-  notifier->UpdateRegisteredIds(&mock_observer_, ObjectIdSet());
+  notifier->UnregisterHandler(&mock_observer_);
 #endif
 }
 
@@ -79,8 +80,9 @@ TEST_F(SyncNotifierFactoryTest, Basic_P2P) {
 #else
   ASSERT_TRUE(notifier.get());
   ObjectIdSet ids = ModelTypeSetToObjectIdSet(ModelTypeSet(syncer::BOOKMARKS));
+  notifier->RegisterHandler(&mock_observer_);
   notifier->UpdateRegisteredIds(&mock_observer_, ids);
-  notifier->UpdateRegisteredIds(&mock_observer_, ObjectIdSet());
+  notifier->UnregisterHandler(&mock_observer_);
 #endif
 }
 
diff --git a/sync/notifier/sync_notifier_helper.cc b/sync/notifier/sync_notifier_helper.cc
deleted file mode 100644
index af3b2ea..0000000
--- a/sync/notifier/sync_notifier_helper.cc
+++ /dev/null
@@ -1,95 +0,0 @@
-// 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.
-
-#include "sync/notifier/sync_notifier_helper.h"
-
-#include "base/logging.h"
-
-namespace syncer {
-
-SyncNotifierHelper::SyncNotifierHelper() {}
-SyncNotifierHelper::~SyncNotifierHelper() {}
-
-ObjectIdSet SyncNotifierHelper::UpdateRegisteredIds(
-    SyncNotifierObserver* handler, const ObjectIdSet& ids) {
-  if (ids.empty()) {
-    handlers_.RemoveObserver(handler);
-  } else if (!handlers_.HasObserver(handler)) {
-    handlers_.AddObserver(handler);
-  }
-
-  ObjectIdSet registered_ids(ids);
-  // Remove all existing entries for |handler| and fill |registered_ids| with
-  // the rest.
-  for (ObjectIdHandlerMap::iterator it = id_to_handler_map_.begin();
-       it != id_to_handler_map_.end(); ) {
-    if (it->second == handler) {
-      ObjectIdHandlerMap::iterator erase_it = it;
-      ++it;
-      id_to_handler_map_.erase(erase_it);
-    } else {
-      registered_ids.insert(it->first);
-      ++it;
-    }
-  }
-
-  // Now add the entries for |handler|. We keep track of the last insertion
-  // point so we only traverse the map once to insert all the new entries.
-  ObjectIdHandlerMap::iterator insert_it = id_to_handler_map_.begin();
-  for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
-    insert_it = id_to_handler_map_.insert(insert_it,
-                                          std::make_pair(*it, handler));
-    CHECK_EQ(handler, insert_it->second) << "Duplicate registration for "
-                                         << ObjectIdToString(insert_it->first);
-  }
-  if (logging::DEBUG_MODE) {
-    // The mapping shouldn't contain any handlers that aren't in |handlers_|.
-    for (ObjectIdHandlerMap::const_iterator it = id_to_handler_map_.begin();
-         it != id_to_handler_map_.end(); ++it) {
-      CHECK(handlers_.HasObserver(it->second));
-    }
-  }
-  return registered_ids;
-}
-
-void SyncNotifierHelper::DispatchInvalidationsToHandlers(
-    const ObjectIdPayloadMap& id_payloads,
-    IncomingNotificationSource source) {
-  typedef std::map<SyncNotifierObserver*, ObjectIdPayloadMap> DispatchMap;
-  DispatchMap dispatch_map;
-  for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin();
-       it != id_payloads.end(); ++it) {
-    ObjectIdHandlerMap::const_iterator find_it =
-        id_to_handler_map_.find(it->first);
-    // If we get an invalidation with a source type that we can't map to an
-    // observer, just drop it--the observer was unregistered while the
-    // invalidation was in flight.
-    if (find_it == id_to_handler_map_.end())
-      continue;
-    dispatch_map[find_it->second].insert(*it);
-  }
-
-  if (handlers_.might_have_observers()) {
-    ObserverListBase<SyncNotifierObserver>::Iterator it(handlers_);
-    SyncNotifierObserver* handler = NULL;
-    while ((handler = it.GetNext()) != NULL) {
-      DispatchMap::const_iterator dispatch_it = dispatch_map.find(handler);
-      if (dispatch_it != dispatch_map.end()) {
-        handler->OnIncomingNotification(dispatch_it->second, source);
-      }
-    }
-  }
-}
-
-void SyncNotifierHelper::EmitOnNotificationsEnabled() {
-  FOR_EACH_OBSERVER(SyncNotifierObserver, handlers_, OnNotificationsEnabled());
-}
-
-void SyncNotifierHelper::EmitOnNotificationsDisabled(
-    NotificationsDisabledReason reason) {
-  FOR_EACH_OBSERVER(SyncNotifierObserver, handlers_,
-                    OnNotificationsDisabled(reason));
-}
-
-}  // namespace syncer
diff --git a/sync/notifier/sync_notifier_helper.h b/sync/notifier/sync_notifier_helper.h
deleted file mode 100644
index 28ff187..0000000
--- a/sync/notifier/sync_notifier_helper.h
+++ /dev/null
@@ -1,55 +0,0 @@
-// 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 SYNC_NOTIFIER_SYNC_NOTIFIER_HELPER_H_
-#define SYNC_NOTIFIER_SYNC_NOTIFIER_HELPER_H_
-
-#include <map>
-
-#include "base/basictypes.h"
-#include "base/observer_list.h"
-#include "sync/notifier/invalidation_util.h"
-#include "sync/notifier/object_id_payload_map.h"
-#include "sync/notifier/sync_notifier_observer.h"
-
-namespace syncer {
-
-// A helper class for classes that want to implement the SyncNotifier interface.
-// It helps keep track of registered handlers and which object ID registrations
-// are associated with which handlers, so implementors can just reuse the logic
-// here to dispatch invalidations and other interesting notifications.
-class SyncNotifierHelper {
- public:
-  SyncNotifierHelper();
-  ~SyncNotifierHelper();
-
-  // Updates the set of ObjectIds associated with a given |handler|. Passing an
-  // empty ObjectIdSet will unregister |handler|. The return value is an
-  // ObjectIdSet containing values for all existing handlers.
-  ObjectIdSet UpdateRegisteredIds(SyncNotifierObserver* handler,
-                                  const ObjectIdSet& ids);
-
-  // Helper that sorts incoming invalidations into a bucket for each handler
-  // and then dispatches the batched invalidations to the corresponding handler.
-  void DispatchInvalidationsToHandlers(const ObjectIdPayloadMap& id_payloads,
-                                       IncomingNotificationSource source);
-
-  // Calls the given handler method for each handler that has registered IDs.
-  void EmitOnNotificationsEnabled();
-  void EmitOnNotificationsDisabled(NotificationsDisabledReason reason);
-
- private:
-  typedef std::map<invalidation::ObjectId,
-                   SyncNotifierObserver*,
-                   ObjectIdLessThan> ObjectIdHandlerMap;
-
-  ObserverList<SyncNotifierObserver> handlers_;
-  ObjectIdHandlerMap id_to_handler_map_;
-
-  DISALLOW_COPY_AND_ASSIGN(SyncNotifierHelper);
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_NOTIFIER_SYNC_NOTIFIER_HELPER_H_
diff --git a/sync/notifier/sync_notifier_helper_unittest.cc b/sync/notifier/sync_notifier_helper_unittest.cc
deleted file mode 100644
index 24d8871..0000000
--- a/sync/notifier/sync_notifier_helper_unittest.cc
+++ /dev/null
@@ -1,156 +0,0 @@
-// 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.
-
-#include "google/cacheinvalidation/types.pb.h"
-#include "sync/notifier/sync_notifier_helper.h"
-#include "sync/notifier/mock_sync_notifier_observer.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using testing::StrictMock;
-
-namespace syncer {
-
-class SyncNotifierHelperTest : public testing::Test {
- protected:
-  SyncNotifierHelperTest()
-      : kObjectId1(ipc::invalidation::ObjectSource::TEST, "a"),
-        kObjectId2(ipc::invalidation::ObjectSource::TEST, "b"),
-        kObjectId3(ipc::invalidation::ObjectSource::TEST, "c") {
-  }
-
-  invalidation::ObjectId kObjectId1;
-  invalidation::ObjectId kObjectId2;
-  invalidation::ObjectId kObjectId3;
-};
-
-// Basic check that registrations are correctly updated for one handler.
-TEST_F(SyncNotifierHelperTest, Basic) {
-  SyncNotifierHelper helper;
-  StrictMock<MockSyncNotifierObserver> observer;
-  ObjectIdSet ids;
-  ids.insert(kObjectId1);
-  ids.insert(kObjectId2);
-  helper.UpdateRegisteredIds(&observer, ids);
-
-  ObjectIdPayloadMap dispatched_payloads;
-  dispatched_payloads[kObjectId1] = "1";
-  dispatched_payloads[kObjectId2] = "2";
-  dispatched_payloads[kObjectId3] = "3";
-
-  // A object ID with no registration should be ignored.
-  ObjectIdPayloadMap expected_payload1;
-  expected_payload1[kObjectId1] = "1";
-  expected_payload1[kObjectId2] = "2";
-  EXPECT_CALL(observer, OnIncomingNotification(expected_payload1,
-                                               REMOTE_NOTIFICATION));
-  helper.DispatchInvalidationsToHandlers(dispatched_payloads,
-                                         REMOTE_NOTIFICATION);
-
-  // Removed object IDs should not be notified, newly-added ones should.
-  ids.erase(kObjectId1);
-  ids.insert(kObjectId3);
-  helper.UpdateRegisteredIds(&observer, ids);
-
-  ObjectIdPayloadMap expected_payload2;
-  expected_payload2[kObjectId2] = "2";
-  expected_payload2[kObjectId3] = "3";
-  EXPECT_CALL(observer, OnIncomingNotification(expected_payload2,
-                                               REMOTE_NOTIFICATION));
-  helper.DispatchInvalidationsToHandlers(dispatched_payloads,
-                                         REMOTE_NOTIFICATION);
-}
-
-// Tests that we correctly bucket and dispatch invalidations on multiple objects
-// to the corresponding handlers.
-TEST_F(SyncNotifierHelperTest, MultipleHandlers) {
-  SyncNotifierHelper helper;
-  StrictMock<MockSyncNotifierObserver> observer;
-  ObjectIdSet ids;
-  ids.insert(kObjectId1);
-  ids.insert(kObjectId2);
-  helper.UpdateRegisteredIds(&observer, ids);
-  StrictMock<MockSyncNotifierObserver> observer2;
-  ObjectIdSet ids2;
-  ids2.insert(kObjectId3);
-  helper.UpdateRegisteredIds(&observer2, ids2);
-
-  ObjectIdPayloadMap expected_payload1;
-  expected_payload1[kObjectId1] = "1";
-  expected_payload1[kObjectId2] = "2";
-  EXPECT_CALL(observer, OnIncomingNotification(expected_payload1,
-                                               REMOTE_NOTIFICATION));
-  ObjectIdPayloadMap expected_payload2;
-  expected_payload2[kObjectId3] = "3";
-  EXPECT_CALL(observer2, OnIncomingNotification(expected_payload2,
-                                                REMOTE_NOTIFICATION));
-
-  ObjectIdPayloadMap dispatched_payloads;
-  dispatched_payloads[kObjectId1] = "1";
-  dispatched_payloads[kObjectId2] = "2";
-  dispatched_payloads[kObjectId3] = "3";
-  helper.DispatchInvalidationsToHandlers(dispatched_payloads,
-                                         REMOTE_NOTIFICATION);
-
-  // Also verify that the callbacks for OnNotificationsEnabled/Disabled work.
-  EXPECT_CALL(observer, OnNotificationsEnabled());
-  EXPECT_CALL(observer2, OnNotificationsEnabled());
-  EXPECT_CALL(observer,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-  EXPECT_CALL(observer2,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-  helper.EmitOnNotificationsEnabled();
-  helper.EmitOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR);
-}
-
-// Multiple registrations by different handlers on the same object ID should
-// cause a CHECK.
-TEST_F(SyncNotifierHelperTest, MultipleRegistration) {
-  SyncNotifierHelper helper;
-  StrictMock<MockSyncNotifierObserver> observer;
-  ObjectIdSet ids;
-  ids.insert(kObjectId1);
-  ids.insert(kObjectId2);
-  helper.UpdateRegisteredIds(&observer, ids);
-
-  StrictMock<MockSyncNotifierObserver> observer2;
-  EXPECT_DEATH({ helper.UpdateRegisteredIds(&observer2, ids); },
-               "Duplicate registration for .*");
-}
-
-// Make sure that passing an empty set to UpdateRegisteredIds clears the
-// corresponding entries for the handler.
-TEST_F(SyncNotifierHelperTest, EmptySetUnregisters) {
-  SyncNotifierHelper helper;
-  StrictMock<MockSyncNotifierObserver> observer;
-  ObjectIdSet ids;
-  ids.insert(kObjectId1);
-  ids.insert(kObjectId2);
-  helper.UpdateRegisteredIds(&observer, ids);
-  // Control observer.
-  StrictMock<MockSyncNotifierObserver> observer2;
-  ObjectIdSet ids2;
-  ids2.insert(kObjectId3);
-  helper.UpdateRegisteredIds(&observer2, ids2);
-  // Unregister the first observer. It should not receive any further callbacks.
-  helper.UpdateRegisteredIds(&observer, ObjectIdSet());
-
-  ObjectIdPayloadMap expected_payload2;
-  expected_payload2[kObjectId3] = "3";
-  EXPECT_CALL(observer2, OnIncomingNotification(expected_payload2,
-                                                REMOTE_NOTIFICATION));
-  EXPECT_CALL(observer2, OnNotificationsEnabled());
-  EXPECT_CALL(observer2,
-              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
-
-  ObjectIdPayloadMap dispatched_payloads;
-  dispatched_payloads[kObjectId1] = "1";
-  dispatched_payloads[kObjectId2] = "2";
-  dispatched_payloads[kObjectId3] = "3";
-  helper.DispatchInvalidationsToHandlers(dispatched_payloads,
-                                         REMOTE_NOTIFICATION);
-  helper.EmitOnNotificationsEnabled();
-  helper.EmitOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR);
-}
-
-}  // namespace syncer
diff --git a/sync/notifier/sync_notifier_registrar.cc b/sync/notifier/sync_notifier_registrar.cc
new file mode 100644
index 0000000..02c00b6
--- /dev/null
+++ b/sync/notifier/sync_notifier_registrar.cc
@@ -0,0 +1,129 @@
+// 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.
+
+#include "sync/notifier/sync_notifier_registrar.h"
+
+#include <cstddef>
+#include <utility>
+
+#include "base/logging.h"
+
+namespace syncer {
+
+SyncNotifierRegistrar::SyncNotifierRegistrar() {}
+
+SyncNotifierRegistrar::~SyncNotifierRegistrar() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void SyncNotifierRegistrar::RegisterHandler(SyncNotifierObserver* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CHECK(handler);
+  CHECK(!handlers_.HasObserver(handler));
+  handlers_.AddObserver(handler);
+}
+
+void SyncNotifierRegistrar::UpdateRegisteredIds(
+    SyncNotifierObserver* handler,
+    const ObjectIdSet& ids) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CHECK(handler);
+  CHECK(handlers_.HasObserver(handler));
+  // Remove all existing entries for |handler|.
+  for (IdHandlerMap::iterator it = id_to_handler_map_.begin();
+       it != id_to_handler_map_.end(); ) {
+    if (it->second == handler) {
+      IdHandlerMap::iterator erase_it = it;
+      ++it;
+      id_to_handler_map_.erase(erase_it);
+    } else {
+      ++it;
+    }
+  }
+
+  // Now add the entries for |handler|. We keep track of the last insertion
+  // point so we only traverse the map once to insert all the new entries.
+  IdHandlerMap::iterator insert_it = id_to_handler_map_.begin();
+  for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
+    insert_it =
+        id_to_handler_map_.insert(insert_it, std::make_pair(*it, handler));
+    CHECK_EQ(handler, insert_it->second)
+        << "Duplicate registration: trying to register "
+        << ObjectIdToString(insert_it->first) << " for "
+        << handler << " when it's already registered for "
+        << insert_it->second;
+  }
+}
+
+void SyncNotifierRegistrar::UnregisterHandler(SyncNotifierObserver* handler) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CHECK(handler);
+  CHECK(handlers_.HasObserver(handler));
+  handlers_.RemoveObserver(handler);
+}
+
+ObjectIdSet SyncNotifierRegistrar::GetAllRegisteredIds() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  ObjectIdSet registered_ids;
+  for (IdHandlerMap::const_iterator it = id_to_handler_map_.begin();
+       it != id_to_handler_map_.end(); ++it) {
+    registered_ids.insert(it->first);
+  }
+  return registered_ids;
+}
+
+void SyncNotifierRegistrar::DispatchInvalidationsToHandlers(
+    const ObjectIdPayloadMap& id_payloads,
+    IncomingNotificationSource source) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // If we have no handlers, there's nothing to do.
+  if (!handlers_.might_have_observers()) {
+    return;
+  }
+
+  typedef std::map<SyncNotifierObserver*, ObjectIdPayloadMap> DispatchMap;
+  DispatchMap dispatch_map;
+  for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin();
+       it != id_payloads.end(); ++it) {
+    SyncNotifierObserver* const handler = ObjectIdToHandler(it->first);
+    // Filter out invalidations for IDs with no handler.
+    if (handler)
+      dispatch_map[handler].insert(*it);
+  }
+
+  // Emit invalidations only for handlers in |handlers_|.
+  ObserverListBase<SyncNotifierObserver>::Iterator it(handlers_);
+  SyncNotifierObserver* handler = NULL;
+  while ((handler = it.GetNext()) != NULL) {
+    DispatchMap::const_iterator dispatch_it = dispatch_map.find(handler);
+    if (dispatch_it != dispatch_map.end())
+      handler->OnIncomingNotification(dispatch_it->second, source);
+  }
+}
+
+void SyncNotifierRegistrar::EmitOnNotificationsEnabled() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  FOR_EACH_OBSERVER(SyncNotifierObserver, handlers_, OnNotificationsEnabled());
+}
+
+void SyncNotifierRegistrar::EmitOnNotificationsDisabled(
+    NotificationsDisabledReason reason) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  FOR_EACH_OBSERVER(SyncNotifierObserver, handlers_,
+                    OnNotificationsDisabled(reason));
+}
+
+void SyncNotifierRegistrar::DetachFromThreadForTest() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  thread_checker_.DetachFromThread();
+}
+
+SyncNotifierObserver* SyncNotifierRegistrar::ObjectIdToHandler(
+    const invalidation::ObjectId& id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  IdHandlerMap::const_iterator it = id_to_handler_map_.find(id);
+  return (it == id_to_handler_map_.end()) ? NULL : it->second;
+}
+
+}  // namespace syncer
diff --git a/sync/notifier/sync_notifier_registrar.h b/sync/notifier/sync_notifier_registrar.h
new file mode 100644
index 0000000..949e3ca
--- /dev/null
+++ b/sync/notifier/sync_notifier_registrar.h
@@ -0,0 +1,82 @@
+// 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 SYNC_NOTIFIER_SYNC_NOTIFIER_REGISTRAR_H_
+#define SYNC_NOTIFIER_SYNC_NOTIFIER_REGISTRAR_H_
+
+#include <map>
+
+#include "base/basictypes.h"
+#include "base/observer_list.h"
+#include "base/threading/thread_checker.h"
+#include "sync/notifier/invalidation_util.h"
+#include "sync/notifier/object_id_payload_map.h"
+#include "sync/notifier/sync_notifier_observer.h"
+
+namespace invalidation {
+class ObjectId;
+}  // namespace invalidation
+
+namespace syncer {
+
+// A helper class for implementations of the SyncNotifier interface.  It helps
+// keep track of registered handlers and which object ID registrations are
+// associated with which handlers, so implementors can just reuse the logic
+// here to dispatch invalidations and other interesting notifications.
+class SyncNotifierRegistrar {
+ public:
+  SyncNotifierRegistrar();
+  ~SyncNotifierRegistrar();
+
+  // Starts sending notifications to |handler|.  |handler| must not be NULL,
+  // and it must already be registered.
+  void RegisterHandler(SyncNotifierObserver* handler);
+
+
+  // Updates the set of ObjectIds associated with |handler|.  |handler| must
+  // not be NULL, and must already be registered.  An ID must be registered for
+  // at most one handler.
+  void UpdateRegisteredIds(SyncNotifierObserver* handler,
+                           const ObjectIdSet& ids);
+
+  // Stops sending notifications to |handler|.  |handler| must not be NULL, and
+  // it must already be registered.  Note that this doesn't unregister the IDs
+  // associated with |handler|.
+  void UnregisterHandler(SyncNotifierObserver* handler);
+
+  // Returns the set of all IDs that are registered to some handler (even
+  // handlers that have been unregistered).
+  ObjectIdSet GetAllRegisteredIds() const;
+
+  // Sorts incoming invalidations into a bucket for each handler and then
+  // dispatches the batched invalidations to the corresponding handler.
+  // Invalidations for IDs with no corresponding handler are dropped, as are
+  // invalidations for handlers that are not added.
+  void DispatchInvalidationsToHandlers(const ObjectIdPayloadMap& id_payloads,
+                                       IncomingNotificationSource source);
+
+  // Calls the given handler method for each handler that has registered IDs.
+  void EmitOnNotificationsEnabled();
+  void EmitOnNotificationsDisabled(NotificationsDisabledReason reason);
+
+  // Needed for death tests.
+  void DetachFromThreadForTest();
+
+ private:
+  typedef std::map<invalidation::ObjectId, SyncNotifierObserver*,
+                   ObjectIdLessThan>
+      IdHandlerMap;
+
+  SyncNotifierObserver* ObjectIdToHandler(const invalidation::ObjectId& id);
+
+  base::ThreadChecker thread_checker_;
+  ObserverList<SyncNotifierObserver> handlers_;
+  IdHandlerMap id_to_handler_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(SyncNotifierRegistrar);
+};
+
+}  // namespace syncer
+
+#endif  // SYNC_NOTIFIER_SYNC_NOTIFIER_REGISTRAR_H_
diff --git a/sync/notifier/sync_notifier_registrar_unittest.cc b/sync/notifier/sync_notifier_registrar_unittest.cc
new file mode 100644
index 0000000..b30252b
--- /dev/null
+++ b/sync/notifier/sync_notifier_registrar_unittest.cc
@@ -0,0 +1,248 @@
+// 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.
+
+#include "google/cacheinvalidation/types.pb.h"
+#include "sync/notifier/mock_sync_notifier_observer.h"
+#include "sync/notifier/sync_notifier_registrar.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+namespace {
+
+using testing::InSequence;
+using testing::Mock;
+using testing::StrictMock;
+
+class SyncNotifierRegistrarTest : public testing::Test {
+ protected:
+  SyncNotifierRegistrarTest()
+      : kObjectId1(ipc::invalidation::ObjectSource::TEST, "a"),
+        kObjectId2(ipc::invalidation::ObjectSource::TEST, "b"),
+        kObjectId3(ipc::invalidation::ObjectSource::TEST, "c"),
+        kObjectId4(ipc::invalidation::ObjectSource::TEST, "d") {
+  }
+
+  const invalidation::ObjectId kObjectId1;
+  const invalidation::ObjectId kObjectId2;
+  const invalidation::ObjectId kObjectId3;
+  const invalidation::ObjectId kObjectId4;
+};
+
+// Register a handler, register some IDs for that handler, and then unregister
+// the handler, dispatching invalidations in between.  The handler should only
+// see invalidations when its registered and its IDs are registered.
+TEST_F(SyncNotifierRegistrarTest, Basic) {
+  StrictMock<MockSyncNotifierObserver> handler;
+
+  SyncNotifierRegistrar registrar;
+
+  registrar.RegisterHandler(&handler);
+
+  ObjectIdPayloadMap payloads;
+  payloads[kObjectId1] = "1";
+  payloads[kObjectId2] = "2";
+  payloads[kObjectId3] = "3";
+
+  // Should be ignored since no IDs are registered to |handler|.
+  registrar.DispatchInvalidationsToHandlers(payloads, REMOTE_NOTIFICATION);
+
+  Mock::VerifyAndClearExpectations(&handler);
+
+  ObjectIdSet ids;
+  ids.insert(kObjectId1);
+  ids.insert(kObjectId2);
+  registrar.UpdateRegisteredIds(&handler, ids);
+
+  {
+    ObjectIdPayloadMap expected_payloads;
+    expected_payloads[kObjectId1] = "1";
+    expected_payloads[kObjectId2] = "2";
+    EXPECT_CALL(handler, OnIncomingNotification(expected_payloads,
+                                                REMOTE_NOTIFICATION));
+  }
+
+  registrar.DispatchInvalidationsToHandlers(payloads, REMOTE_NOTIFICATION);
+
+  Mock::VerifyAndClearExpectations(&handler);
+
+  ids.erase(kObjectId1);
+  ids.insert(kObjectId3);
+  registrar.UpdateRegisteredIds(&handler, ids);
+
+  {
+    ObjectIdPayloadMap expected_payloads;
+    expected_payloads[kObjectId2] = "2";
+    expected_payloads[kObjectId3] = "3";
+    EXPECT_CALL(handler, OnIncomingNotification(expected_payloads,
+                                                REMOTE_NOTIFICATION));
+  }
+
+  // Removed object IDs should not be notified, newly-added ones should.
+  registrar.DispatchInvalidationsToHandlers(payloads, REMOTE_NOTIFICATION);
+
+  Mock::VerifyAndClearExpectations(&handler);
+
+  registrar.UnregisterHandler(&handler);
+
+  // Should be ignored since |handler| isn't registered anymore.
+  registrar.DispatchInvalidationsToHandlers(payloads, REMOTE_NOTIFICATION);
+}
+
+// Register handlers and some IDs for those handlers, register a handler with
+// no IDs, and register a handler with some IDs but unregister it.  Then,
+// dispatch some notifications and invalidations.  Handlers that are registered
+// should get notifications, and the ones that have registered IDs should
+// receive invalidations for those IDs.
+TEST_F(SyncNotifierRegistrarTest, MultipleHandlers) {
+  StrictMock<MockSyncNotifierObserver> handler1;
+  EXPECT_CALL(handler1, OnNotificationsEnabled());
+  {
+    ObjectIdPayloadMap expected_payloads;
+    expected_payloads[kObjectId1] = "1";
+    expected_payloads[kObjectId2] = "2";
+    EXPECT_CALL(handler1, OnIncomingNotification(expected_payloads,
+                                                 REMOTE_NOTIFICATION));
+  }
+  EXPECT_CALL(handler1,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+
+  StrictMock<MockSyncNotifierObserver> handler2;
+  EXPECT_CALL(handler2, OnNotificationsEnabled());
+  {
+    ObjectIdPayloadMap expected_payloads;
+    expected_payloads[kObjectId3] = "3";
+    EXPECT_CALL(handler2, OnIncomingNotification(expected_payloads,
+                                                 REMOTE_NOTIFICATION));
+  }
+  EXPECT_CALL(handler2,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+
+  StrictMock<MockSyncNotifierObserver> handler3;
+  EXPECT_CALL(handler3, OnNotificationsEnabled());
+  EXPECT_CALL(handler3,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+
+  StrictMock<MockSyncNotifierObserver> handler4;
+
+  SyncNotifierRegistrar registrar;
+
+  registrar.RegisterHandler(&handler1);
+  registrar.RegisterHandler(&handler2);
+  registrar.RegisterHandler(&handler3);
+  registrar.RegisterHandler(&handler4);
+
+  {
+    ObjectIdSet ids;
+    ids.insert(kObjectId1);
+    ids.insert(kObjectId2);
+    registrar.UpdateRegisteredIds(&handler1, ids);
+  }
+
+  {
+    ObjectIdSet ids;
+    ids.insert(kObjectId3);
+    registrar.UpdateRegisteredIds(&handler2, ids);
+  }
+
+  // Don't register any IDs for handler3.
+
+  {
+    ObjectIdSet ids;
+    ids.insert(kObjectId4);
+    registrar.UpdateRegisteredIds(&handler4, ids);
+  }
+
+  registrar.UnregisterHandler(&handler4);
+
+  registrar.EmitOnNotificationsEnabled();
+  {
+    ObjectIdPayloadMap payloads;
+    payloads[kObjectId1] = "1";
+    payloads[kObjectId2] = "2";
+    payloads[kObjectId3] = "3";
+    payloads[kObjectId4] = "4";
+    registrar.DispatchInvalidationsToHandlers(payloads, REMOTE_NOTIFICATION);
+  }
+  registrar.EmitOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR);
+}
+
+// Multiple registrations by different handlers on the same object ID should
+// cause a CHECK.
+TEST_F(SyncNotifierRegistrarTest, MultipleRegistration) {
+  SyncNotifierRegistrar registrar;
+
+  StrictMock<MockSyncNotifierObserver> handler1;
+  registrar.RegisterHandler(&handler1);
+
+  MockSyncNotifierObserver handler2;
+  registrar.RegisterHandler(&handler2);
+
+  ObjectIdSet ids;
+  ids.insert(kObjectId1);
+  ids.insert(kObjectId2);
+  registrar.UpdateRegisteredIds(&handler1, ids);
+
+  registrar.DetachFromThreadForTest();
+  EXPECT_DEATH({ registrar.UpdateRegisteredIds(&handler2, ids); },
+               "Duplicate registration: .*");
+}
+
+// Make sure that passing an empty set to UpdateRegisteredIds clears the
+// corresponding entries for the handler.
+TEST_F(SyncNotifierRegistrarTest, EmptySetUnregisters) {
+  StrictMock<MockSyncNotifierObserver> handler1;
+  EXPECT_CALL(handler1, OnNotificationsEnabled());
+  EXPECT_CALL(handler1,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+
+  // Control observer.
+  StrictMock<MockSyncNotifierObserver> handler2;
+  EXPECT_CALL(handler2, OnNotificationsEnabled());
+  {
+    ObjectIdPayloadMap expected_payloads;
+    expected_payloads[kObjectId3] = "3";
+    EXPECT_CALL(handler2, OnIncomingNotification(expected_payloads,
+                                                 REMOTE_NOTIFICATION));
+  }
+  EXPECT_CALL(handler2,
+              OnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR));
+
+  SyncNotifierRegistrar registrar;
+
+  registrar.RegisterHandler(&handler1);
+  registrar.RegisterHandler(&handler2);
+
+  {
+    ObjectIdSet ids;
+    ids.insert(kObjectId1);
+    ids.insert(kObjectId2);
+    registrar.UpdateRegisteredIds(&handler1, ids);
+  }
+
+  {
+    ObjectIdSet ids;
+    ids.insert(kObjectId3);
+    registrar.UpdateRegisteredIds(&handler2, ids);
+  }
+
+  // Unregister the IDs for the first observer. It should not receive any
+  // further invalidations.
+  registrar.UpdateRegisteredIds(&handler1, ObjectIdSet());
+
+  registrar.EmitOnNotificationsEnabled();
+  {
+    ObjectIdPayloadMap payloads;
+    payloads[kObjectId1] = "1";
+    payloads[kObjectId2] = "2";
+    payloads[kObjectId3] = "3";
+    registrar.DispatchInvalidationsToHandlers(payloads,
+                                              REMOTE_NOTIFICATION);
+  }
+  registrar.EmitOnNotificationsDisabled(TRANSIENT_NOTIFICATION_ERROR);
+}
+
+}  // namespace
+
+}  // namespace syncer
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 3143b6d..0851271 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -257,9 +257,9 @@
         'notifier/sync_notifier.h',
         'notifier/sync_notifier_factory.cc',
         'notifier/sync_notifier_factory.h',
-        'notifier/sync_notifier_helper.cc',
-        'notifier/sync_notifier_helper.h',
         'notifier/sync_notifier_observer.h',
+        'notifier/sync_notifier_registrar.cc',
+        'notifier/sync_notifier_registrar.h',
       ],
       'conditions': [
         ['OS != "android"', {
@@ -654,7 +654,7 @@
               'notifier/p2p_notifier_unittest.cc',
               'notifier/push_client_channel_unittest.cc',
               'notifier/registration_manager_unittest.cc',
-              'notifier/sync_notifier_helper_unittest.cc',
+              'notifier/sync_notifier_registrar_unittest.cc',
             ],
           }],
         ],
diff --git a/sync/tools/sync_listen_notifications.cc b/sync/tools/sync_listen_notifications.cc
index 5eb05b6..6d8adb0 100644
--- a/sync/tools/sync_listen_notifications.cc
+++ b/sync/tools/sync_listen_notifications.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <cstddef>
 #include <cstdio>
 #include <string>
 
@@ -242,13 +243,15 @@ int SyncListenNotificationsMain(int argc, char* argv[]) {
   const char kUniqueId[] = "fake_unique_id";
   sync_notifier->SetUniqueId(kUniqueId);
   sync_notifier->UpdateCredentials(email, token);
+
   // Listen for notifications for all known types.
+  sync_notifier->RegisterHandler(&notification_printer);
   sync_notifier->UpdateRegisteredIds(
       &notification_printer, ModelTypeSetToObjectIdSet(ModelTypeSet::All()));
 
   ui_loop.Run();
 
-  sync_notifier->UpdateRegisteredIds(&notification_printer, ObjectIdSet());
+  sync_notifier->UnregisterHandler(&notification_printer);
   io_thread.Stop();
   return 0;
 }