Refactor sync-specific parts out of SyncNotifier/SyncNotifierObserver

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 107ed606e743b2bdbd91d550541e84f238a5b35f..153466dfcad7d032f7c9d3cd8ecffe0d48fd23fe 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -46,6 +46,7 @@
 #include "sync/js/js_event_details.h"
 #include "sync/js/js_event_handler.h"
 #include "sync/js/js_reply_handler.h"
+#include "sync/notifier/invalidation_util.h"
 #include "sync/notifier/notifications_disabled_reason.h"
 #include "sync/notifier/sync_notifier.h"
 #include "sync/notifier/sync_notifier_observer.h"
@@ -308,7 +309,7 @@ class SyncManagerImpl::SyncInternal
   virtual void OnNotificationsDisabled(
       syncer::NotificationsDisabledReason reason) OVERRIDE;
   virtual void OnIncomingNotification(
-      const syncer::ModelTypePayloadMap& type_payloads,
+      const ObjectIdPayloadMap& id_payloads,
       syncer::IncomingNotificationSource source) OVERRIDE;
 
   void AddObserver(SyncManager::Observer* observer);
@@ -993,8 +994,6 @@ bool SyncManagerImpl::SyncInternal::Init(
   if (!success && testing_mode_ == NON_TEST)
     return false;
 
-  sync_notifier_->AddObserver(this);
-
   return success;
 }
 
@@ -1234,7 +1233,8 @@ void SyncManagerImpl::SyncInternal::UpdateCredentials(
 void SyncManagerImpl::SyncInternal::UpdateEnabledTypes(
     const ModelTypeSet& enabled_types) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  sync_notifier_->UpdateEnabledTypes(enabled_types);
+  sync_notifier_->UpdateRegisteredIds(this,
+                                      ModelTypeSetToObjectIdSet(enabled_types));
 }
 
 void SyncManagerImpl::SyncInternal::SetEncryptionPassphrase(
@@ -1722,7 +1722,7 @@ void SyncManagerImpl::SyncInternal::ShutdownOnSyncThread() {
   RemoveObserver(&debug_info_event_listener_);
 
   if (sync_notifier_.get()) {
-    sync_notifier_->RemoveObserver(this);
+    sync_notifier_->UpdateRegisteredIds(this, ObjectIdSet());
   }
   sync_notifier_.reset();
 
@@ -2347,9 +2347,11 @@ void SyncManagerImpl::SyncInternal::OnNotificationsDisabled(
 }
 
 void SyncManagerImpl::SyncInternal::OnIncomingNotification(
-    const syncer::ModelTypePayloadMap& type_payloads,
+    const ObjectIdPayloadMap& id_payloads,
     syncer::IncomingNotificationSource source) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  ModelTypePayloadMap type_payloads =

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

const ref

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

Done.

As an aside, do you know if the style guide has some preference for binding
returned temporaries to const refs, or if this is something that is preferred in
sync code? AFAIK, it shouldn't actually make a difference in generated code due
to RVO/NRVO.

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

I don't think so -- I just prefer it myself.  I think it always avoids a copy,
even if RVO/NRVO doesn't apply.

+      ObjectIdPayloadMapToModelTypePayloadMap(id_payloads);
   if (source == syncer::LOCAL_NOTIFICATION) {
     if (scheduler()) {
       scheduler()->ScheduleNudgeWithPayloadsAsync(
@@ -2481,12 +2483,11 @@ void SyncManagerImpl::SimulateDisableNotificationsForTest(int reason) {
 void SyncManagerImpl::TriggerOnIncomingNotificationForTest(
     ModelTypeSet model_types) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  syncer::ModelTypePayloadMap model_types_with_payloads =
-      syncer::ModelTypePayloadMapFromEnumSet(model_types,
-          std::string());
-
-  data_->OnIncomingNotification(model_types_with_payloads,
-                                syncer::REMOTE_NOTIFICATION);
+  ModelTypePayloadMap type_payloads =

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

const ref

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

Done.

+      ModelTypePayloadMapFromEnumSet(model_types, std::string());
+  data_->OnIncomingNotification(
+      ModelTypePayloadMapToObjectIdPayloadMap(type_payloads),
+      REMOTE_NOTIFICATION);
 }
 
 // static.