[Sync] Avoid unregistering object IDs on shutdown

chrome/browser/sync/glue/chrome_sync_notification_bridge.cc

Index: chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
diff --git a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
index 31db8e2da4443d733b4e32e7b0ea16017d908219..615174244ca8eb7139ab578aefa36b563224df72 100644
--- a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
+++ b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
@@ -25,8 +27,13 @@ class ChromeSyncNotificationBridge::Core
 
   // All member functions below must be called on the sync task runner.
 
+  void InitializeOnSyncThread();
+  void DestroyOnSyncThread();

[tim (not reviewing), 2012/08/06 05:55:32]

Is 'Destroy' the right name?  This won't necessarily result in calling the
destructor. 

[akalin, 2012/08/07 07:25:19]

Done, use Cleanup

+
   void UpdateEnabledTypes(syncer::ModelTypeSet enabled_types);
-  void UpdateRegisteredIds(syncer::SyncNotifierObserver* handler,
+  void SetHandler(const std::string& handler_name,
+                  syncer::SyncNotifierObserver* handler);
+  void UpdateRegisteredIds(const std::string& handler_name,
                            const syncer::ObjectIdSet& ids);
 
   void EmitNotification(
@@ -43,12 +50,13 @@ class ChromeSyncNotificationBridge::Core
 
   // Used only on |sync_task_runner_|.
   syncer::ModelTypeSet enabled_types_;
-  syncer::SyncNotifierHelper helper_;
+  scoped_ptr<syncer::SyncNotifierHelper> helper_;
 };
 
 ChromeSyncNotificationBridge::Core::Core(
     const scoped_refptr<base::SequencedTaskRunner>& sync_task_runner)
-    : sync_task_runner_(sync_task_runner) {
+    : sync_task_runner_(sync_task_runner),
+      helper_(NULL) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(sync_task_runner_.get());
 }
@@ -56,6 +64,15 @@ ChromeSyncNotificationBridge::Core::Core(
 ChromeSyncNotificationBridge::Core::~Core() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          sync_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(!helper_.get());
+}
+
+void ChromeSyncNotificationBridge::Core::InitializeOnSyncThread() {
+  helper_.reset(new syncer::SyncNotifierHelper());
+}
+
+void ChromeSyncNotificationBridge::Core::DestroyOnSyncThread() {
+  helper_.reset();
 }
 
 void ChromeSyncNotificationBridge::Core::UpdateEnabledTypes(
@@ -64,11 +81,18 @@ void ChromeSyncNotificationBridge::Core::UpdateEnabledTypes(
   enabled_types_ = types;
 }
 
+void ChromeSyncNotificationBridge::Core::SetHandler(
+    const std::string& handler_name,
+    syncer::SyncNotifierObserver* handler) {
+  DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+  helper_->SetHandler(handler_name, handler);
+}
+
 void ChromeSyncNotificationBridge::Core::UpdateRegisteredIds(
-    syncer::SyncNotifierObserver* handler,
+    const std::string& handler_name,
     const syncer::ObjectIdSet& ids) {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
-  helper_.UpdateRegisteredIds(handler, ids);
+  helper_->UpdateRegisteredIds(handler_name, ids);
 }
 
 void ChromeSyncNotificationBridge::Core::EmitNotification(
@@ -80,7 +104,7 @@ void ChromeSyncNotificationBridge::Core::EmitNotification(
       syncer::ModelTypePayloadMapFromEnumSet(enabled_types_, std::string()) :
       payload_map;
 
-  helper_.DispatchInvalidationsToHandlers(
+  helper_->DispatchInvalidationsToHandlers(
       ModelTypePayloadMapToObjectIdPayloadMap(effective_payload_map),
       notification_source);
 }
@@ -96,21 +120,40 @@ ChromeSyncNotificationBridge::ChromeSyncNotificationBridge(
                  content::Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH_REMOTE,
                  content::Source<Profile>(profile));
+
+  if (!sync_task_runner_->PostTask(
+          FROM_HERE, base::Bind(&Core::InitializeOnSyncThread, core_))) {
+    NOTREACHED();
+  }
 }
 
 ChromeSyncNotificationBridge::~ChromeSyncNotificationBridge() {}
 
+void ChromeSyncNotificationBridge::StopForShutdown() {
+  if (!sync_task_runner_->PostTask(
+          FROM_HERE, base::Bind(&Core::DestroyOnSyncThread, core_))) {
+    NOTREACHED();
+  }
+}
+
 void ChromeSyncNotificationBridge::UpdateEnabledTypes(
     syncer::ModelTypeSet types) {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
   core_->UpdateEnabledTypes(types);
 }
 
+void ChromeSyncNotificationBridge::SetHandler(
+    const std::string& handler_name,
+    syncer::SyncNotifierObserver* handler) {
+  DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+  core_->SetHandler(handler_name, handler);
+}
+
 void ChromeSyncNotificationBridge::UpdateRegisteredIds(
-    syncer::SyncNotifierObserver* handler,
+    const std::string& handler_name,
     const syncer::ObjectIdSet& ids) {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
-  core_->UpdateRegisteredIds(handler, ids);
+  core_->UpdateRegisteredIds(handler_name, ids);
 }
 
 void ChromeSyncNotificationBridge::Observe(
@@ -132,10 +175,12 @@ void ChromeSyncNotificationBridge::Observe(
   content::Details<const syncer::ModelTypePayloadMap>
       payload_details(details);
   const syncer::ModelTypePayloadMap& payload_map = *(payload_details.ptr());
-  sync_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&Core::EmitNotification,
-                 core_, payload_map, notification_source));
+  if (!sync_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&Core::EmitNotification,
+                     core_, payload_map, notification_source))) {
+    NOTREACHED();
+  }
 }
 
 }  // namespace browser_sync