[Sync] Enable adding notifier observers from ProfileSyncService

Plumb the registrations from the UI thread to the sync thread, and plumb the invalidations back.

Add ObjectIdSet <-> ObjectIdPayloadMap conversions.

Rename syncapi_unittest.cc to sync_manager_impl_unittest.cc.

BUG=137087
TEST=


Review URL: https://chromiumcodereview.appspot.com/10805002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149747 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 170 insertions, 43 deletions

diff --git a/sync/internal_api/public/DEPS b/sync/internal_api/public/DEPS
index 58ef64a..877a88d 100644
--- a/sync/internal_api/public/DEPS
+++ b/sync/internal_api/public/DEPS
@@ -2,6 +2,7 @@ include_rules = [
   "-sync",
   "+sync/base",
   "+sync/internal_api/public",
+  "+sync/notifier",
   "+sync/protocol",
 
   # TODO(tim): Remove. Bug 131130
diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h
index f1b23ce..f36b8ec 100644
--- a/sync/internal_api/public/sync_manager.h
+++ b/sync/internal_api/public/sync_manager.h
@@ -22,36 +22,31 @@
 #include "sync/internal_api/public/engine/sync_status.h"
 #include "sync/internal_api/public/util/report_unrecoverable_error_function.h"
 #include "sync/internal_api/public/util/weak_handle.h"
+#include "sync/notifier/invalidation_util.h"
 #include "sync/protocol/sync_protocol_error.h"
 
+namespace sync_pb {
+class EncryptedData;
+}  // namespace sync_pb
+
 namespace syncer {
+class BaseTransaction;
 class Encryptor;
 struct Experiments;
 class ExtensionsActivityMonitor;
+class HttpPostProviderFactory;
 class InternalComponentsFactory;
 class JsBackend;
 class JsEventHandler;
+class SyncNotifier;
+class SyncNotifierObserver;
 class SyncScheduler;
 class UnrecoverableErrorHandler;
+struct UserShare;
 
 namespace sessions {
 class SyncSessionSnapshot;
 }  // namespace sessions
-}  // namespace syncer
-
-namespace syncer {
-class SyncNotifier;
-}  // namespace syncer
-
-namespace sync_pb {
-class EncryptedData;
-}  // namespace sync_pb
-
-namespace syncer {
-
-class BaseTransaction;
-class HttpPostProviderFactory;
-struct UserShare;
 
 // Used by SyncManager::OnConnectionStatusChange().
 enum ConnectionStatus {
@@ -411,6 +406,11 @@ class SyncManager {
   virtual void UpdateEnabledTypes(
       const ModelTypeSet& enabled_types) = 0;
 
+  // Forwards to the underlying notifier (see
+  // SyncNotifier::UpdateRegisteredIds()).
+  virtual void UpdateRegisteredInvalidationIds(
+      SyncNotifierObserver* handler, const ObjectIdSet& ids) = 0;
+
   // Put the syncer in normal mode ready to perform nudges and polls.
   virtual void StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) = 0;
diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h
index 531785e..b8e5471 100644
--- a/sync/internal_api/public/test/fake_sync_manager.h
+++ b/sync/internal_api/public/test/fake_sync_manager.h
@@ -7,11 +7,14 @@
 
 #include <string>
 
-#include "sync/internal_api/public/sync_manager.h"
-
+#include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
+#include "sync/internal_api/public/sync_manager.h"
+#include "sync/notifier/sync_notifier_helper.h"
 
-class MessageLoop;
+namespace base {
+class SequencedTaskRunner;
+}
 
 namespace syncer {
 
@@ -50,6 +53,17 @@ class FakeSyncManager : public SyncManager {
   // called.
   ModelTypeSet GetAndResetEnabledTypes();
 
+  // Posts a method to invalidate the given IDs on the sync thread.
+  void Invalidate(
+    const ObjectIdPayloadMap& id_payloads,
+    IncomingNotificationSource source);
+
+  // Posts a method to enable notifications on the sync thread.
+  void EnableNotifications();
+
+  // Posts a method to disable notifications on the sync thread.
+  void DisableNotifications(NotificationsDisabledReason reason);
+
   // SyncManager implementation.
   // Note: we treat whatever message loop this is called from as the sync
   // loop for purposes of callbacks.
@@ -81,6 +95,8 @@ class FakeSyncManager : public SyncManager {
   virtual bool PurgePartiallySyncedTypes() OVERRIDE;
   virtual void UpdateCredentials(const SyncCredentials& credentials) OVERRIDE;
   virtual void UpdateEnabledTypes(const ModelTypeSet& types) OVERRIDE;
+  virtual void UpdateRegisteredInvalidationIds(
+      SyncNotifierObserver* handler, const ObjectIdSet& ids) OVERRIDE;
   virtual void StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) OVERRIDE;
   virtual void SetEncryptionPassphrase(const std::string& passphrase,
@@ -108,6 +124,14 @@ class FakeSyncManager : public SyncManager {
   virtual bool HasUnsyncedItems() OVERRIDE;
 
  private:
+  void InvalidateOnSyncThread(
+      const ObjectIdPayloadMap& id_payloads,
+      IncomingNotificationSource source);
+  void EnableNotificationsOnSyncThread();
+  void DisableNotificationsOnSyncThread(NotificationsDisabledReason reason);
+
+  scoped_refptr<base::SequencedTaskRunner> sync_task_runner_;
+
   ObserverList<SyncManager::Observer> observers_;
 
   // Faked directory state.
@@ -125,8 +149,8 @@ class FakeSyncManager : public SyncManager {
   // The set of types that have been enabled.
   ModelTypeSet enabled_types_;
 
-  // For StopSyncingForShutdown's callback.
-  MessageLoop* sync_loop_;
+  // Faked notifier state.
+  SyncNotifierHelper notifier_helper_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeSyncManager);
 };
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index b400c51..950e280 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -733,8 +733,15 @@ void SyncManagerImpl::UpdateCredentials(
 void SyncManagerImpl::UpdateEnabledTypes(
     const ModelTypeSet& enabled_types) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  sync_notifier_->UpdateRegisteredIds(this,
-                                      ModelTypeSetToObjectIdSet(enabled_types));
+  sync_notifier_->UpdateRegisteredIds(
+      this,
+      ModelTypeSetToObjectIdSet(enabled_types));
+}
+
+void SyncManagerImpl::UpdateRegisteredInvalidationIds(
+    SyncNotifierObserver* handler, const ObjectIdSet& ids) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  sync_notifier_->UpdateRegisteredIds(handler, ids);
 }
 
 void SyncManagerImpl::SetEncryptionPassphrase(
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index 3c68939..bdbc223 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -86,6 +86,8 @@ class SyncManagerImpl : public SyncManager,
   virtual void UpdateCredentials(const SyncCredentials& credentials) OVERRIDE;
   virtual void UpdateEnabledTypes(
       const ModelTypeSet& enabled_types) OVERRIDE;
+  virtual void UpdateRegisteredInvalidationIds(
+      SyncNotifierObserver* handler, const ObjectIdSet& ids) OVERRIDE;
   virtual void StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) OVERRIDE;
   virtual void SetEncryptionPassphrase(const std::string& passphrase,
diff --git a/sync/internal_api/syncapi_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 58c83c2..6cf652e 100644
--- a/sync/internal_api/syncapi_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -23,8 +23,8 @@
 #include "base/test/values_test_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
-#include "sync/internal_api/public/base/model_type_test_util.h"
 #include "sync/engine/sync_scheduler.h"
+#include "sync/internal_api/public/base/model_type_test_util.h"
 #include "sync/internal_api/public/change_record.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/engine/polling_constants.h"
@@ -36,8 +36,8 @@
 #include "sync/internal_api/public/test/test_user_share.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/sync_manager_impl.h"
+#include "sync/internal_api/syncapi_internal.h"
 #include "sync/js/js_arg_list.h"
 #include "sync/js/js_backend.h"
 #include "sync/js/js_event_handler.h"
@@ -60,9 +60,9 @@
 #include "sync/syncable/syncable_id.h"
 #include "sync/syncable/write_transaction.h"
 #include "sync/test/callback_counter.h"
+#include "sync/test/engine/fake_sync_scheduler.h"
 #include "sync/test/fake_encryptor.h"
 #include "sync/test/fake_extensions_activity_monitor.h"
-#include "sync/test/engine/fake_sync_scheduler.h"
 #include "sync/util/cryptographer.h"
 #include "sync/util/extensions_activity_monitor.h"
 #include "sync/util/test_unrecoverable_error_handler.h"
@@ -696,8 +696,8 @@ class SyncManagerObserverMock : public SyncManager::Observer {
 
 class SyncNotifierMock : public SyncNotifier {
  public:
-  MOCK_METHOD2(UpdateRegisteredIds, void(SyncNotifierObserver*,
-                                         const ObjectIdSet&));
+  MOCK_METHOD2(UpdateRegisteredIds,
+               void(SyncNotifierObserver*, const ObjectIdSet&));
   MOCK_METHOD1(SetUniqueId, void(const std::string&));
   MOCK_METHOD1(SetStateDeprecated, void(const std::string&));
   MOCK_METHOD2(UpdateCredentials,
@@ -782,8 +782,7 @@ class SyncManagerTest : public testing::Test,
 
   void TearDown() {
     sync_manager_.RemoveObserver(&observer_);
-    EXPECT_CALL(*sync_notifier_mock_,
-                UpdateRegisteredIds(_, ObjectIdSet()));
+    EXPECT_CALL(*sync_notifier_mock_, UpdateRegisteredIds(_, ObjectIdSet()));
     sync_manager_.ShutdownOnSyncThread();
     sync_notifier_mock_ = NULL;
     PumpLoop();
@@ -959,10 +958,16 @@ TEST_F(SyncManagerTest, UpdateEnabledTypes) {
   const ModelTypeSet enabled_types = GetRoutingInfoTypes(routes);
 
   EXPECT_CALL(*sync_notifier_mock_,
-              UpdateRegisteredIds(_, ModelTypeSetToObjectIdSet(enabled_types)));
+              UpdateRegisteredIds(
+                  _, ModelTypeSetToObjectIdSet(enabled_types)));
   sync_manager_.UpdateEnabledTypes(enabled_types);
 }
 
+TEST_F(SyncManagerTest, UpdateRegisteredInvalidationIds) {
+  EXPECT_CALL(*sync_notifier_mock_, UpdateRegisteredIds(NULL, ObjectIdSet()));
+  sync_manager_.UpdateRegisteredInvalidationIds(NULL, ObjectIdSet());
+}
+
 TEST_F(SyncManagerTest, ProcessJsMessage) {
   const JsArgList kNoArgs;
 
diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc
index b0b9ac2..894a953 100644
--- a/sync/internal_api/test/fake_sync_manager.cc
+++ b/sync/internal_api/test/fake_sync_manager.cc
@@ -4,19 +4,26 @@
 
 #include "sync/internal_api/public/test/fake_sync_manager.h"
 
-#include "base/message_loop.h"
+#include <cstddef>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/sequenced_task_runner.h"
+#include "base/single_thread_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "sync/internal_api/public/http_post_provider_factory.h"
 #include "sync/internal_api/public/internal_components_factory.h"
 #include "sync/internal_api/public/util/weak_handle.h"
+#include "sync/notifier/notifications_disabled_reason.h"
+#include "sync/notifier/object_id_payload_map.h"
 #include "sync/notifier/sync_notifier.h"
 
 namespace syncer {
 
-FakeSyncManager::FakeSyncManager() {
-}
+FakeSyncManager::FakeSyncManager() {}
 
-FakeSyncManager::~FakeSyncManager() {
-}
+FakeSyncManager::~FakeSyncManager() {}
 
 void FakeSyncManager::set_initial_sync_ended_types(ModelTypeSet types) {
   initial_sync_ended_types_ = types;
@@ -48,6 +55,36 @@ ModelTypeSet FakeSyncManager::GetAndResetEnabledTypes() {
   return enabled_types;
 }
 
+void FakeSyncManager::Invalidate(
+    const ObjectIdPayloadMap& id_payloads,
+    IncomingNotificationSource source) {
+  if (!sync_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&FakeSyncManager::InvalidateOnSyncThread,
+                 base::Unretained(this), id_payloads, source))) {
+    NOTREACHED();
+  }
+}
+
+void FakeSyncManager::EnableNotifications() {
+  if (!sync_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&FakeSyncManager::EnableNotificationsOnSyncThread,
+                 base::Unretained(this)))) {
+    NOTREACHED();
+  }
+}
+
+void FakeSyncManager::DisableNotifications(
+    NotificationsDisabledReason reason) {
+  if (!sync_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&FakeSyncManager::DisableNotificationsOnSyncThread,
+                 base::Unretained(this), reason))) {
+    NOTREACHED();
+  }
+}
+
 bool FakeSyncManager::Init(
     const FilePath& database_location,
     const WeakHandle<JsEventHandler>& event_handler,
@@ -69,7 +106,7 @@ bool FakeSyncManager::Init(
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction
         report_unrecoverable_error_function) {
-  sync_loop_ = MessageLoop::current();
+  sync_task_runner_ = base::ThreadTaskRunnerHandle::Get();
   PurgePartiallySyncedTypes();
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                     OnInitializationComplete(
@@ -113,6 +150,11 @@ void FakeSyncManager::UpdateEnabledTypes(const ModelTypeSet& types) {
   enabled_types_ = types;
 }
 
+void FakeSyncManager::UpdateRegisteredInvalidationIds(
+    SyncNotifierObserver* handler, const ObjectIdSet& ids) {
+  notifier_helper_.UpdateRegisteredIds(handler, ids);
+}
+
 void FakeSyncManager::StartSyncingNormally(
       const ModelSafeRoutingInfo& routing_info) {
   // Do nothing.
@@ -187,11 +229,13 @@ void FakeSyncManager::SaveChanges() {
 }
 
 void FakeSyncManager::StopSyncingForShutdown(const base::Closure& callback) {
-  sync_loop_->PostTask(FROM_HERE, callback);
+  if (!sync_task_runner_->PostTask(FROM_HERE, callback)) {
+    NOTREACHED();
+  }
 }
 
 void FakeSyncManager::ShutdownOnSyncThread() {
-  // Do nothing.
+  DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
 }
 
 UserShare* FakeSyncManager::GetUserShare() {
@@ -217,5 +261,22 @@ bool FakeSyncManager::HasUnsyncedItems() {
   return false;
 }
 
-}  // namespace syncer
+void FakeSyncManager::InvalidateOnSyncThread(
+    const ObjectIdPayloadMap& id_payloads,
+    IncomingNotificationSource source) {
+  DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+  notifier_helper_.DispatchInvalidationsToHandlers(id_payloads, source);
+}
 
+void FakeSyncManager::EnableNotificationsOnSyncThread() {
+  DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+  notifier_helper_.EmitOnNotificationsEnabled();
+}
+
+void FakeSyncManager::DisableNotificationsOnSyncThread(
+    NotificationsDisabledReason reason) {
+  DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+  notifier_helper_.EmitOnNotificationsDisabled(reason);
+}
+
+}  // namespace syncer
diff --git a/sync/notifier/object_id_payload_map.cc b/sync/notifier/object_id_payload_map.cc
index 179f8be..19eed0c 100644
--- a/sync/notifier/object_id_payload_map.cc
+++ b/sync/notifier/object_id_payload_map.cc
@@ -6,6 +6,25 @@
 
 namespace syncer {
 
+ObjectIdSet ObjectIdPayloadMapToSet(
+    const ObjectIdPayloadMap& id_payloads) {
+  ObjectIdSet ids;
+  for (ObjectIdPayloadMap::const_iterator it = id_payloads.begin();
+       it != id_payloads.end(); ++it) {
+    ids.insert(it->first);
+  }
+  return ids;
+}
+
+ObjectIdPayloadMap ObjectIdSetToPayloadMap(
+    ObjectIdSet ids, const std::string& payload) {
+  ObjectIdPayloadMap id_payloads;
+  for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) {
+    id_payloads[*it] = payload;
+  }
+  return id_payloads;
+}
+
 ModelTypePayloadMap ObjectIdPayloadMapToModelTypePayloadMap(
     const ObjectIdPayloadMap& id_payloads) {
   ModelTypePayloadMap types_with_payloads;
diff --git a/sync/notifier/object_id_payload_map.h b/sync/notifier/object_id_payload_map.h
index 9205326..91a1f15 100644
--- a/sync/notifier/object_id_payload_map.h
+++ b/sync/notifier/object_id_payload_map.h
@@ -18,6 +18,11 @@ typedef std::map<invalidation::ObjectId,
                  std::string,
                  ObjectIdLessThan> ObjectIdPayloadMap;
 
+// Converts between ObjectIdPayloadMaps and ObjectIdSets.
+ObjectIdSet ObjectIdPayloadMapToSet(const ObjectIdPayloadMap& id_payloads);
+ObjectIdPayloadMap ObjectIdSetToPayloadMap(
+    ObjectIdSet ids, const std::string& payload);
+
 // Converts between ObjectIdPayloadMaps and ModelTypePayloadMaps.
 ModelTypePayloadMap ObjectIdPayloadMapToModelTypePayloadMap(
     const ObjectIdPayloadMap& id_payloads);
diff --git a/sync/notifier/sync_notifier.h b/sync/notifier/sync_notifier.h
index 369ced1..48bc1f1 100644
--- a/sync/notifier/sync_notifier.h
+++ b/sync/notifier/sync_notifier.h
@@ -22,9 +22,10 @@ class SyncNotifier {
   SyncNotifier() {}
   virtual ~SyncNotifier() {}
 
-  // Updates the set of ObjectIds associated with a given |handler|. Passing an
-  // empty ObjectIdSet will unregister |handler|. If two different handlers
-  // attempt to register for the same object ID, the first registration wins.
+  // Updates the set of ObjectIds associated with a given
+  // |handler|. Passing an empty ObjectIdSet will unregister
+  // |handler|. There should be at most one handler registered per
+  // object id.
   virtual void UpdateRegisteredIds(SyncNotifierObserver* handler,
                                    const ObjectIdSet& ids) = 0;
 
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 6a38127..3583c87 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -480,11 +480,13 @@
         '../base/base.gyp:base',
         '../testing/gtest.gyp:gtest',
         'syncapi_core',
+        'sync_notifier',
         'test_support_sync',
       ],
       'export_dependent_settings': [
         '../testing/gtest.gyp:gtest',
         'syncapi_core',
+        'sync_notifier',
         'test_support_sync',
       ],
       'sources': [
@@ -706,7 +708,7 @@
           'internal_api/js_mutation_event_observer_unittest.cc',
           'internal_api/js_sync_manager_observer_unittest.cc',
           'internal_api/syncapi_server_connection_manager_unittest.cc',
-          'internal_api/syncapi_unittest.cc',
+          'internal_api/sync_manager_impl_unittest.cc',
         ],
       },
     },