[Sync] Avoid unregistering object IDs on shutdown

sync/internal_api/sync_manager_impl.cc

 // 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/internal_api/sync_manager_impl.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 378 matching lines @@
   DCHECK(!credentials.sync_token.empty());
   DVLOG(1) << "SyncManager starting Init...";
 
   weak_handle_this_ = MakeWeakHandle(weak_ptr_factory_.GetWeakPtr());
 
   blocking_task_runner_ = blocking_task_runner;
 
   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);
 
   AddObserver(&debug_info_event_listener_);
 
   database_path_ = database_location.Append(
       syncable::Directory::kSyncDatabaseFilename);
   encryptor_ = encryptor;
   unrecoverable_error_handler_ = unrecoverable_error_handler;
@@ skipping 317 matching lines @@
   if (!connection_manager_->set_auth_token(credentials.sync_token))
     return;  // Auth token is known to be invalid, so exit early.
 
   sync_notifier_->UpdateCredentials(credentials.email, credentials.sync_token);
   scheduler_->OnCredentialsUpdated();
 }
 
 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) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // We do not accept empty passphrases.
   if (passphrase.empty()) {
     NOTREACHED() << "Cannot encrypt with an empty passphrase.";
     return;
   }
 
@@ skipping 451 matching lines @@
   js_mutation_event_observer_.InvalidateWeakPtrs();
 
   scheduler_.reset();
   session_context_.reset();
 
   SetJsEventHandler(WeakHandle<JsEventHandler>());
   RemoveObserver(&js_sync_manager_observer_);
 
   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();
 
   if (connection_manager_.get()) {
     connection_manager_->RemoveListener(this);
   }
   connection_manager_.reset();
 
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
   observing_ip_address_changes_ = false;
@@ skipping 666 matching lines @@
 int SyncManagerImpl::GetDefaultNudgeDelay() {
   return kDefaultNudgeDelayMilliseconds;
 }
 
 // static.
 int SyncManagerImpl::GetPreferencesNudgeDelay() {
   return kPreferencesNudgeDelayMilliseconds;
 }
 
 }  // namespace syncer