[Sync] Avoid unregistering object IDs on shutdown

sync/internal_api/sync_manager_impl.cc

Index: sync/internal_api/sync_manager_impl.cc
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 72f936f3d23a27c2d34c0c17f6cf4598b04998c2..de3a1a8f8d29a0a2b993794b3741a21d53f39ac3 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -396,6 +396,8 @@ bool SyncManagerImpl::Init(
   change_delegate_ = change_delegate;
 
   sync_notifier_ = sync_notifier.Pass();
+  if (sync_notifier_.get())

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

Can this legitimately be NULL? It's not NULL checked in some other places.
If it's just NULL in tests, perhaps add a comment to that effect?

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

good catch.  this shouldn't be null.

+    sync_notifier_->RegisterHandler(this);
 
   AddObserver(&js_sync_manager_observer_);
   SetJsEventHandler(event_handler);
@@ -733,18 +735,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) {
@@ -1217,7 +1235,7 @@ void SyncManagerImpl::ShutdownOnSyncThread() {
   RemoveObserver(&debug_info_event_listener_);
 
   if (sync_notifier_.get()) {

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

ditto on NULL question; and nit: remove {}

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

needed for tests.  added comment.

-    sync_notifier_->UpdateRegisteredIds(this, ObjectIdSet());
+    sync_notifier_->UnregisterHandler(this);
   }
   sync_notifier_.reset();