Refactor sync-specific parts out of SyncNotifier/SyncNotifierObserver

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 28 matching lines @@
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
 #include "sync/internal_api/syncapi_internal.h"
 #include "sync/internal_api/syncapi_server_connection_manager.h"
 #include "sync/js/js_arg_list.h"
 #include "sync/js/js_backend.h"
 #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"
 #include "sync/protocol/encryption.pb.h"
 #include "sync/protocol/proto_value_conversions.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/directory_change_delegate.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/in_memory_directory_backing_store.h"
@@ skipping 242 matching lines @@
   // Cryptographer::Observer implementation.
   virtual void OnEncryptedTypesChanged(
       syncer::ModelTypeSet encrypted_types,
       bool encrypt_everything) OVERRIDE;
 
   // SyncNotifierObserver implementation.
   virtual void OnNotificationsEnabled() OVERRIDE;
   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);
   void RemoveObserver(SyncManager::Observer* observer);
 
   // Accessors for the private members.
   syncable::Directory* directory() { return share_.directory.get(); }
   SyncAPIServerConnectionManager* connection_manager() {
     return connection_manager_.get();
   }
@@ skipping 664 matching lines @@
   // on the UI thread and if shutdown wins then that tasks would execute on
   // a freed pointer. This is because UI thread is not shut down.
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                     OnInitializationComplete(
                         MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()),
                         success));
 
   if (!success && testing_mode_ == NON_TEST)
     return false;
 
-  sync_notifier_->AddObserver(this);
-
   return success;
 }
 
 void SyncManagerImpl::SyncInternal::UpdateCryptographerAndNigori(
     const std::string& chrome_version,
     const base::Closure& done_callback) {
   DCHECK(initialized_);
   syncer::GetSessionName(
       blocking_task_runner_,
       base::Bind(
@@ skipping 219 matching lines @@
         credentials.email, credentials.sync_token);
     if (initialized_ && scheduler()) {
       scheduler()->OnCredentialsUpdated();
     }
   }
 }
 
 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(
     const std::string& passphrase,
     bool is_explicit) {
   // We do not accept empty passphrases.
   if (passphrase.empty()) {
     NOTREACHED() << "Cannot encrypt with an empty passphrase.";
     return;
   }
@@ skipping 467 matching lines @@
 
   scheduler_.reset();
   session_context_.reset();
 
   SetJsEventHandler(WeakHandle<JsEventHandler>());
   RemoveObserver(&js_sync_manager_observer_);
 
   RemoveObserver(&debug_info_event_listener_);
 
   if (sync_notifier_.get()) {
-    sync_notifier_->RemoveObserver(this);
+    sync_notifier_->UpdateRegisteredIds(this, ObjectIdSet());
   }
   sync_notifier_.reset();
 
   if (connection_manager_.get()) {
     connection_manager_->RemoveListener(this);
   }
   connection_manager_.reset();
 
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
   observing_ip_address_changes_ = false;
@@ skipping 604 matching lines @@
     js_event_handler_.Call(FROM_HERE,
                            &JsEventHandler::HandleJsEvent,
                            "onNotificationStateChange",
                            JsEventDetails(&details));
   }
   // TODO(akalin): Treat a CREDENTIALS_REJECTED state as an auth
   // error.
 }
 
 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(
           TimeDelta::FromMilliseconds(kSyncRefreshDelayMsec),
           syncer::NUDGE_SOURCE_LOCAL_REFRESH,
           type_payloads, FROM_HERE);
     }
   } else if (!type_payloads.empty()) {
     if (scheduler()) {
       scheduler()->ScheduleNudgeWithPayloadsAsync(
@@ skipping 111 matching lines @@
 
 void SyncManagerImpl::SimulateDisableNotificationsForTest(int reason) {
   DCHECK(thread_checker_.CalledOnValidThread());
   data_->OnNotificationsDisabled(
       static_cast<syncer::NotificationsDisabledReason>(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.
 int SyncManagerImpl::GetDefaultNudgeDelay() {
   return kDefaultNudgeDelayMilliseconds;
 }
 
 // static.
 int SyncManagerImpl::GetPreferencesNudgeDelay() {
   return kPreferencesNudgeDelayMilliseconds;
 }
 
 }  // namespace syncer