Refactor sync-specific parts out of SyncNotifier/SyncNotifierObserver

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 9936fa08f020e93b0eff6fe86a1dcbecc3f81db8..35e2781d571c2c0f32fa605014a9ca3c1050191f 100644
--- a/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
+++ b/chrome/browser/sync/glue/chrome_sync_notification_bridge.cc
@@ -14,9 +14,7 @@ using content::BrowserThread;
 namespace browser_sync {
 
 ChromeSyncNotificationBridge::ChromeSyncNotificationBridge(
-    const Profile* profile)
-    : observers_(
-        new ObserverListThreadSafe<syncer::SyncNotifierObserver>()) {
+    const Profile* profile) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(profile);
   registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH_LOCAL,
@@ -33,14 +31,11 @@ void ChromeSyncNotificationBridge::UpdateEnabledTypes(
   enabled_types_ = types;
 }
 
-void ChromeSyncNotificationBridge::AddObserver(
-    syncer::SyncNotifierObserver* observer) {
-  observers_->AddObserver(observer);
-}
-
-void ChromeSyncNotificationBridge::RemoveObserver(
-    syncer::SyncNotifierObserver* observer) {
-  observers_->RemoveObserver(observer);
+void ChromeSyncNotificationBridge::UpdateRegisteredIds(
+    syncer::SyncNotifierObserver* handler,
+    const syncer::ObjectIdSet& ids) {
+  base::AutoLock locker_(helper_lock_);
+  helper_.UpdateRegisteredIds(handler, ids);
 }
 
 void ChromeSyncNotificationBridge::Observe(
@@ -69,9 +64,10 @@ void ChromeSyncNotificationBridge::Observe(
         syncer::ModelTypePayloadMapFromEnumSet(enabled_types_, std::string());
   }
 
-  observers_->Notify(
-      &syncer::SyncNotifierObserver::OnIncomingNotification,
-      payload_map, notification_source);
+  base::AutoLock locker_(helper_lock_);
+  helper_.DispatchInvalidationsToHandlers(

[akalin, 2012/07/19 00:42:06]

this won't work.  the invalidations must happen on the same thread as called by
UpdateRegisteredIds.  Ugh, this sucks, I forgot about this. :( Let me try to
think of a solution.

I may try to fix this in a separate patch in parallel.

[dcheng, 2012/07/19 18:31:05]

I don't quite understand the issue. Why isn't it a problem in the current code?

[akalin, 2012/07/19 23:07:26]

ObserverListThreadSafe guarantees that notifications are sent only on the same
thread as AddObserver, and only if RemoveObserver (which has to happen on the
same thread) hasn't been called yet.  Without using OLTS, you'd need to
guarantee that yourself (which is non-trivial).

FYI, my patch to fix this is https://chromiumcodereview.appspot.com/10808040/

[dcheng, 2012/07/20 00:57:04]

Ah. My fault for not looking at the implementation of OLTS. I assumed it was
pretty much just a bunch of locks =)

I'll wait for that patch to land then.

+      ModelTypePayloadMapToObjectIdPayloadMap(payload_map),
+      notification_source);
 }
 
 }  // namespace browser_sync