diff options
22 files changed, 516 insertions, 76 deletions
diff --git a/chrome/browser/sync/DEPS b/chrome/browser/sync/DEPS index 7dba439..f9f6c1e 100644 --- a/chrome/browser/sync/DEPS +++ b/chrome/browser/sync/DEPS @@ -1,8 +1,11 @@ include_rules = [ - # For files not in a subdirectory (what a mess!). + # Used by tests. + "+google/cacheinvalidation", + + # For files not in a subdirectory. "+sync/internal_api/public", "+sync/js", - "+sync/notifier/invalidation_state_tracker.h", + "+sync/notifier", # TODO(tim): Remove everything below. Bug 131130. "+sync/util/cryptographer.h", diff --git a/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc b/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc index acf0898..991b774 100644 --- a/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc +++ b/chrome/browser/sync/glue/chrome_sync_notification_bridge_unittest.cc @@ -5,8 +5,8 @@ #include "chrome/browser/sync/glue/chrome_sync_notification_bridge.h" #include "base/compiler_specific.h" -#include "base/memory/scoped_ptr.h" #include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/message_loop.h" #include "base/sequenced_task_runner.h" @@ -53,12 +53,8 @@ class FakeSyncNotifierObserver : public syncer::SyncNotifierObserver { expected_payloads_(expected_payloads), expected_source_(expected_source) { DCHECK(sync_task_runner_->RunsTasksOnCurrentThread()); - // TODO(dcheng): We might want a function to go from ObjectIdPayloadMap -> - // ObjectIdSet to avoid this rather long incantation... - const syncer::ObjectIdSet& ids = syncer::ModelTypeSetToObjectIdSet( - syncer::ModelTypePayloadMapToEnumSet( - syncer::ObjectIdPayloadMapToModelTypePayloadMap( - expected_payloads))); + const syncer::ObjectIdSet& ids = + syncer::ObjectIdPayloadMapToSet(expected_payloads); bridge_->UpdateRegisteredIds(this, ids); } diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 54f0cd1..9b7d19d 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -43,8 +43,8 @@ #include "net/url_request/url_request_context_getter.h" #include "sync/internal_api/public/base_transaction.h" #include "sync/internal_api/public/engine/model_safe_worker.h" -#include "sync/internal_api/public/internal_components_factory_impl.h" #include "sync/internal_api/public/http_bridge.h" +#include "sync/internal_api/public/internal_components_factory_impl.h" #include "sync/internal_api/public/read_transaction.h" #include "sync/internal_api/public/sync_manager_factory.h" #include "sync/internal_api/public/util/experiments.h" @@ -78,7 +78,8 @@ using syncer::SyncCredentials; class SyncBackendHost::Core : public base::RefCountedThreadSafe<SyncBackendHost::Core>, - public syncer::SyncManager::Observer { + public syncer::SyncManager::Observer, + public syncer::SyncNotifierObserver { public: Core(const std::string& name, const FilePath& sync_data_folder_path, @@ -110,6 +111,14 @@ class SyncBackendHost::Core virtual void OnActionableError( const syncer::SyncProtocolError& sync_error) OVERRIDE; + // syncer::SyncNotifierObserver implementation. + virtual void OnNotificationsEnabled() OVERRIDE; + virtual void OnNotificationsDisabled( + syncer::NotificationsDisabledReason reason) OVERRIDE; + virtual void OnIncomingNotification( + const syncer::ObjectIdPayloadMap& id_payloads, + syncer::IncomingNotificationSource source) OVERRIDE; + // Note: // // The Do* methods are the various entry points from our @@ -122,9 +131,13 @@ class SyncBackendHost::Core void DoInitialize(const DoInitializeOptions& options); // Called to perform credential update on behalf of - // SyncBackendHost::UpdateCredentials + // SyncBackendHost::UpdateCredentials. void DoUpdateCredentials(const syncer::SyncCredentials& credentials); + // Called to update the given registered ids on behalf of + // SyncBackendHost::UpdateRegisteredInvalidationIds. + void DoUpdateRegisteredInvalidationIds(const syncer::ObjectIdSet& ids); + // Called to tell the syncapi to start syncing (generally after // initialization and authentication). void DoStartSyncing(const syncer::ModelSafeRoutingInfo& routing_info); @@ -423,6 +436,15 @@ void SyncBackendHost::UpdateCredentials(const SyncCredentials& credentials) { credentials)); } +void SyncBackendHost::UpdateRegisteredInvalidationIds( + const syncer::ObjectIdSet& ids) { + DCHECK_EQ(MessageLoop::current(), frontend_loop_); + DCHECK(sync_thread_.IsRunning()); + sync_thread_.message_loop()->PostTask(FROM_HERE, + base::Bind(&SyncBackendHost::Core::DoUpdateRegisteredInvalidationIds, + core_.get(), ids)); +} + void SyncBackendHost::StartSyncingWithServer() { SDVLOG(1) << "SyncBackendHost::StartSyncingWithServer called."; @@ -871,6 +893,8 @@ void SyncBackendHost::Core::OnInitializationComplete( if (!success) { sync_manager_->RemoveObserver(this); + sync_manager_->UpdateRegisteredInvalidationIds( + this, syncer::ObjectIdSet()); sync_manager_->ShutdownOnSyncThread(); sync_manager_.reset(); } @@ -980,6 +1004,35 @@ void SyncBackendHost::Core::OnActionableError( sync_error); } +void SyncBackendHost::Core::OnNotificationsEnabled() { + if (!sync_loop_) + return; + DCHECK_EQ(MessageLoop::current(), sync_loop_); + host_.Call(FROM_HERE, + &SyncBackendHost::HandleNotificationsEnabledOnFrontendLoop); +} + +void SyncBackendHost::Core::OnNotificationsDisabled( + syncer::NotificationsDisabledReason reason) { + if (!sync_loop_) + return; + DCHECK_EQ(MessageLoop::current(), sync_loop_); + host_.Call(FROM_HERE, + &SyncBackendHost::HandleNotificationsDisabledOnFrontendLoop, + reason); +} + +void SyncBackendHost::Core::OnIncomingNotification( + const syncer::ObjectIdPayloadMap& id_payloads, + syncer::IncomingNotificationSource source) { + if (!sync_loop_) + return; + DCHECK_EQ(MessageLoop::current(), sync_loop_); + host_.Call(FROM_HERE, + &SyncBackendHost::HandleIncomingNotificationOnFrontendLoop, + id_payloads, source); +} + void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) { DCHECK(!sync_loop_); sync_loop_ = options.sync_loop; @@ -1030,7 +1083,7 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) { &encryptor_, options.unrecoverable_error_handler, options.report_unrecoverable_error_function); - LOG_IF(ERROR, !success) << "Syncapi initialization failed!"; + LOG_IF(ERROR, !success) << "Sync manager initialization failed!"; // Now check the command line to see if we need to simulate an // unrecoverable error for testing purpose. Note the error is thrown @@ -1050,6 +1103,19 @@ void SyncBackendHost::Core::DoUpdateCredentials( sync_manager_->UpdateCredentials(credentials); } +void SyncBackendHost::Core::DoUpdateRegisteredInvalidationIds( + const syncer::ObjectIdSet& ids) { + DCHECK_EQ(MessageLoop::current(), sync_loop_); + // |sync_manager_| may end up being NULL here in tests (in + // synchronous initialization mode) since this is called during + // shutdown. + // + // TODO(akalin): Fix this behavior. + if (sync_manager_.get()) { + sync_manager_->UpdateRegisteredInvalidationIds(this, ids); + } +} + void SyncBackendHost::Core::DoStartSyncing( const syncer::ModelSafeRoutingInfo& routing_info) { DCHECK_EQ(MessageLoop::current(), sync_loop_); @@ -1095,6 +1161,8 @@ void SyncBackendHost::Core::DoShutdown(bool sync_disabled) { DCHECK_EQ(MessageLoop::current(), sync_loop_); if (sync_manager_.get()) { save_changes_timer_.reset(); + sync_manager_->UpdateRegisteredInvalidationIds( + this, syncer::ObjectIdSet()); sync_manager_->ShutdownOnSyncThread(); sync_manager_->RemoveObserver(this); sync_manager_.reset(); @@ -1313,6 +1381,30 @@ void SyncBackendHost::HandleActionableErrorEventOnFrontendLoop( frontend_->OnActionableError(sync_error); } +void SyncBackendHost::HandleNotificationsEnabledOnFrontendLoop() { + if (!frontend_) + return; + DCHECK_EQ(MessageLoop::current(), frontend_loop_); + frontend_->OnNotificationsEnabled(); +} + +void SyncBackendHost::HandleNotificationsDisabledOnFrontendLoop( + syncer::NotificationsDisabledReason reason) { + if (!frontend_) + return; + DCHECK_EQ(MessageLoop::current(), frontend_loop_); + frontend_->OnNotificationsDisabled(reason); +} + +void SyncBackendHost::HandleIncomingNotificationOnFrontendLoop( + const syncer::ObjectIdPayloadMap& id_payloads, + syncer::IncomingNotificationSource source) { + if (!frontend_) + return; + DCHECK_EQ(MessageLoop::current(), frontend_loop_); + frontend_->OnIncomingNotification(id_payloads, source); +} + bool SyncBackendHost::CheckPassphraseAgainstCachedPendingKeys( const std::string& passphrase) const { DCHECK(cached_pending_keys_.has_blob()); diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 0a6fde3..bafe140 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -27,6 +27,7 @@ #include "sync/internal_api/public/util/unrecoverable_error_handler.h" #include "sync/internal_api/public/util/weak_handle.h" #include "sync/notifier/sync_notifier_factory.h" +#include "sync/notifier/sync_notifier_observer.h" #include "sync/protocol/encryption.pb.h" #include "sync/protocol/sync_protocol_error.h" @@ -51,7 +52,7 @@ class SyncPrefs; // activity. // NOTE: All methods will be invoked by a SyncBackendHost on the same thread // used to create that SyncBackendHost. -class SyncFrontend { +class SyncFrontend : public syncer::SyncNotifierObserver { public: SyncFrontend() {} @@ -175,9 +176,13 @@ class SyncBackendHost : public BackendDataTypeConfigurer { syncer::ReportUnrecoverableErrorFunction report_unrecoverable_error_function); - // Called from |frontend_loop| to update SyncCredentials. + // Called on |frontend_loop| to update SyncCredentials. void UpdateCredentials(const syncer::SyncCredentials& credentials); + // Registers the underlying frontend for the given IDs to the + // underlying notifier. + void UpdateRegisteredInvalidationIds(const syncer::ObjectIdSet& ids); + // This starts the SyncerThread running a Syncer object to communicate with // sync servers. Until this is called, no changes will leave or enter this // browser from the cloud / sync servers. @@ -349,6 +354,8 @@ class SyncBackendHost : public BackendDataTypeConfigurer { const syncer::WeakHandle<syncer::JsBackend>& js_backend, bool success, syncer::ModelTypeSet restored_types); + SyncFrontend* frontend() { return frontend_; } + private: // The real guts of SyncBackendHost, to keep the public client API clean. class Core; @@ -460,6 +467,14 @@ class SyncBackendHost : public BackendDataTypeConfigurer { const syncer::WeakHandle<syncer::JsBackend>& js_backend, syncer::ModelTypeSet failed_configuration_types); + // syncer::SyncNotifierObserver-like functions. + void HandleNotificationsEnabledOnFrontendLoop(); + void HandleNotificationsDisabledOnFrontendLoop( + syncer::NotificationsDisabledReason reason); + void HandleIncomingNotificationOnFrontendLoop( + const syncer::ObjectIdPayloadMap& id_payloads, + syncer::IncomingNotificationSource source); + // Must be called on |frontend_loop_|. |done_callback| is called on // |frontend_loop_|. void RefreshNigori(const base::Closure& done_callback); diff --git a/chrome/browser/sync/glue/sync_backend_host_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_unittest.cc index dcaddef..2145da1 100644 --- a/chrome/browser/sync/glue/sync_backend_host_unittest.cc +++ b/chrome/browser/sync/glue/sync_backend_host_unittest.cc @@ -6,6 +6,7 @@ #include <cstddef> +#include "base/location.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" #include "base/synchronization/waitable_event.h" @@ -14,6 +15,7 @@ #include "chrome/browser/sync/sync_prefs.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread.h" +#include "google/cacheinvalidation/include/types.h" #include "googleurl/src/gurl.h" #include "net/url_request/test_url_fetcher_factory.h" #include "sync/internal_api/public/base/model_type.h" @@ -21,6 +23,8 @@ #include "sync/internal_api/public/sync_manager_factory.h" #include "sync/internal_api/public/test/fake_sync_manager.h" #include "sync/internal_api/public/util/experiments.h" +#include "sync/notifier/mock_sync_notifier_observer.h" +#include "sync/notifier/notifications_disabled_reason.h" #include "sync/protocol/encryption.pb.h" #include "sync/protocol/sync_protocol_error.h" #include "sync/util/test_unrecoverable_error_handler.h" @@ -31,6 +35,7 @@ using content::BrowserThread; using syncer::FakeSyncManager; using syncer::SyncManager; using ::testing::InvokeWithoutArgs; +using ::testing::StrictMock; using ::testing::_; namespace browser_sync { @@ -53,6 +58,12 @@ class MockSyncFrontend : public SyncFrontend { public: virtual ~MockSyncFrontend() {} + MOCK_METHOD0(OnNotificationsEnabled, void()); + MOCK_METHOD1(OnNotificationsDisabled, + void(syncer::NotificationsDisabledReason)); + MOCK_METHOD2(OnIncomingNotification, + void(const syncer::ObjectIdPayloadMap&, + syncer::IncomingNotificationSource)); MOCK_METHOD2(OnBackendInitialized, void(const syncer::WeakHandle<syncer::JsBackend>&, bool)); MOCK_METHOD0(OnSyncCycleCompleted, void()); @@ -140,7 +151,7 @@ class SyncBackendHostTest : public testing::Test { sync_prefs_.reset(); invalidator_storage_.reset(); profile_.reset(); - // Pump messages posted by the sync core thread (which may end up + // Pump messages posted by the sync thread (which may end up // posting on the IO thread). ui_loop_.RunAllPending(); io_thread_.Stop(); @@ -540,6 +551,64 @@ TEST_F(SyncBackendHostTest, NewlySupportedTypesWithPartialTypes) { enabled_types_).Empty()); } +// Register for some IDs and trigger an invalidation. This should +// propagate all the way to the frontend. +TEST_F(SyncBackendHostTest, Invalidate) { + InitializeBackend(); + + syncer::ObjectIdSet ids; + ids.insert(invalidation::ObjectId(1, "id1")); + ids.insert(invalidation::ObjectId(2, "id2")); + const syncer::ObjectIdPayloadMap& id_payloads = + syncer::ObjectIdSetToPayloadMap(ids, "payload"); + + EXPECT_CALL( + mock_frontend_, + OnIncomingNotification(id_payloads, syncer::REMOTE_NOTIFICATION)) + .WillOnce(InvokeWithoutArgs(QuitMessageLoop)); + + backend_->UpdateRegisteredInvalidationIds(ids); + fake_manager_->Invalidate(id_payloads, syncer::REMOTE_NOTIFICATION); + ui_loop_.PostDelayedTask( + FROM_HERE, ui_loop_.QuitClosure(), TestTimeouts::action_timeout()); + ui_loop_.Run(); +} + +// Register for some IDs and turn on notifications. This should +// propagate all the way to the frontend. +TEST_F(SyncBackendHostTest, EnableNotifications) { + InitializeBackend(); + + EXPECT_CALL(mock_frontend_, OnNotificationsEnabled()) + .WillOnce(InvokeWithoutArgs(QuitMessageLoop)); + + syncer::ObjectIdSet ids; + ids.insert(invalidation::ObjectId(3, "id3")); + backend_->UpdateRegisteredInvalidationIds(ids); + fake_manager_->EnableNotifications(); + ui_loop_.PostDelayedTask( + FROM_HERE, ui_loop_.QuitClosure(), TestTimeouts::action_timeout()); + ui_loop_.Run(); +} + +// Register for some IDs and turn off notifications. This should +// propagate all the way to the frontend. +TEST_F(SyncBackendHostTest, DisableNotifications) { + InitializeBackend(); + + EXPECT_CALL(mock_frontend_, + OnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR)) + .WillOnce(InvokeWithoutArgs(QuitMessageLoop)); + + syncer::ObjectIdSet ids; + ids.insert(invalidation::ObjectId(4, "id4")); + backend_->UpdateRegisteredInvalidationIds(ids); + fake_manager_->DisableNotifications(syncer::TRANSIENT_NOTIFICATION_ERROR); + ui_loop_.PostDelayedTask( + FROM_HERE, ui_loop_.QuitClosure(), TestTimeouts::action_timeout()); + ui_loop_.Run(); +} + } // namespace } // namespace browser_sync diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index af0299b..bc72dcc 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -422,6 +422,15 @@ void ProfileSyncService::StartUp() { // be there. InitializeBackend(!HasSyncSetupCompleted()); + // |backend_| may end up being NULL here in tests (in synchronous + // initialization mode). + // + // TODO(akalin): Fix this horribly non-intuitive behavior (see + // http://crbug.com/140354). + if (backend_.get()) { + backend_->UpdateRegisteredInvalidationIds(all_registered_ids_); + } + if (!sync_global_error_.get()) { #if !defined(OS_ANDROID) sync_global_error_.reset(new SyncGlobalError(this, signin())); @@ -432,6 +441,17 @@ void ProfileSyncService::StartUp() { } } +void ProfileSyncService::UpdateRegisteredInvalidationIds( + syncer::SyncNotifierObserver* handler, + const syncer::ObjectIdSet& ids) { + all_registered_ids_ = notifier_helper_.UpdateRegisteredIds(handler, ids); + // If |backend_| is NULL, its registered IDs will be updated when + // it's created and initialized. + if (backend_.get()) { + backend_->UpdateRegisteredInvalidationIds(all_registered_ids_); + } +} + void ProfileSyncService::Shutdown() { ShutdownImpl(false); } @@ -442,8 +462,10 @@ void ProfileSyncService::ShutdownImpl(bool sync_disabled) { // applying changes to the sync db that wouldn't get applied via // ChangeProcessors, leading to back-from-the-dead bugs. base::Time shutdown_start_time = base::Time::Now(); - if (backend_.get()) + if (backend_.get()) { backend_->StopSyncingForShutdown(); + backend_->UpdateRegisteredInvalidationIds(syncer::ObjectIdSet()); + } // Stop all data type controllers, if needed. Note that until Stop // completes, it is possible in theory to have a ChangeProcessor apply a @@ -636,6 +658,21 @@ void ProfileSyncService::DisableBrokenDatatype( weak_factory_.GetWeakPtr())); } +void ProfileSyncService::OnNotificationsEnabled() { + notifier_helper_.EmitOnNotificationsEnabled(); +} + +void ProfileSyncService::OnNotificationsDisabled( + syncer::NotificationsDisabledReason reason) { + notifier_helper_.EmitOnNotificationsDisabled(reason); +} + +void ProfileSyncService::OnIncomingNotification( + const syncer::ObjectIdPayloadMap& id_payloads, + syncer::IncomingNotificationSource source) { + notifier_helper_.DispatchInvalidationsToHandlers(id_payloads, source); +} + void ProfileSyncService::OnBackendInitialized( const syncer::WeakHandle<syncer::JsBackend>& js_backend, bool success) { is_first_time_sync_configure_ = !HasSyncSetupCompleted(); diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index ef5eff0..3dee74f 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -38,6 +38,7 @@ #include "sync/internal_api/public/util/experiments.h" #include "sync/internal_api/public/util/unrecoverable_error_handler.h" #include "sync/js/sync_js_controller.h" +#include "sync/notifier/sync_notifier_helper.h" class Profile; class ProfileSyncComponentsFactory; @@ -253,6 +254,14 @@ class ProfileSyncService : public browser_sync::SyncFrontend, virtual bool HasSyncSetupCompleted() const; virtual void SetSyncSetupCompleted(); + // syncer::SyncNotifier implementation (via SyncFrontend). + virtual void OnNotificationsEnabled() OVERRIDE; + virtual void OnNotificationsDisabled( + syncer::NotificationsDisabledReason reason) OVERRIDE; + virtual void OnIncomingNotification( + const syncer::ObjectIdPayloadMap& id_payloads, + syncer::IncomingNotificationSource source) OVERRIDE; + // SyncFrontend implementation. virtual void OnBackendInitialized( const syncer::WeakHandle<syncer::JsBackend>& js_backend, @@ -542,6 +551,16 @@ class ProfileSyncService : public browser_sync::SyncFrontend, // been cleared yet. Virtual for testing purposes. virtual bool waiting_for_auth() const; + // 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. + // + // The handler -> registered ids map is persisted across restarts of + // sync. + void UpdateRegisteredInvalidationIds(syncer::SyncNotifierObserver* handler, + const syncer::ObjectIdSet& ids); + // ProfileKeyedService implementation. virtual void Shutdown() OVERRIDE; @@ -816,6 +835,12 @@ class ProfileSyncService : public browser_sync::SyncFrontend, // Factory the backend will use to build the SyncManager. syncer::SyncManagerFactory sync_manager_factory_; + // The set of all registered IDs. + syncer::ObjectIdSet all_registered_ids_; + + // Dispatches invalidations to handlers. + syncer::SyncNotifierHelper notifier_helper_; + DISALLOW_COPY_AND_ASSIGN(ProfileSyncService); }; diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc index 1d589b3..e5c3b94 100644 --- a/chrome/browser/sync/profile_sync_service_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_unittest.cc @@ -21,9 +21,11 @@ #include "chrome/test/base/testing_profile.h" #include "content/public/common/content_client.h" #include "content/public/test/test_browser_thread.h" +#include "google/cacheinvalidation/include/types.h" #include "sync/js/js_arg_list.h" #include "sync/js/js_event_details.h" #include "sync/js/js_test_util.h" +#include "sync/notifier/mock_sync_notifier_observer.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "webkit/glue/webkit_glue.h" @@ -40,6 +42,7 @@ using content::BrowserThread; using testing::_; using testing::AtLeast; using testing::AtMost; +using testing::Mock; using testing::Return; using testing::StrictMock; @@ -82,7 +85,7 @@ class ProfileSyncServiceTest : public testing::Test { void StartSyncService() { StartSyncServiceAndSetInitialSyncEnded( - true, true, false, true, true, syncer::STORAGE_IN_MEMORY); + true, true, false, true, syncer::STORAGE_IN_MEMORY); } void StartSyncServiceAndSetInitialSyncEnded( @@ -90,7 +93,6 @@ class ProfileSyncServiceTest : public testing::Test { bool issue_auth_token, bool synchronous_sync_configuration, bool sync_setup_completed, - bool expect_create_dtm, syncer::StorageOption storage_option) { if (!service_.get()) { SigninManager* signin = @@ -113,14 +115,9 @@ class ProfileSyncServiceTest : public testing::Test { if (!sync_setup_completed) profile_->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false); - if (expect_create_dtm) { - // Register the bookmark data type. - EXPECT_CALL(*factory, CreateDataTypeManager(_, _)). - WillOnce(ReturnNewDataTypeManager()); - } else { - EXPECT_CALL(*factory, CreateDataTypeManager(_, _)). - Times(0); - } + // Register the bookmark data type. + ON_CALL(*factory, CreateDataTypeManager(_, _)). + WillByDefault(ReturnNewDataTypeManager()); if (issue_auth_token) { IssueTestTokens(); @@ -252,7 +249,7 @@ TEST_F(ProfileSyncServiceTest, JsControllerHandlersBasic) { TEST_F(ProfileSyncServiceTest, JsControllerHandlersDelayedBackendInitialization) { - StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, true, + StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, syncer::STORAGE_IN_MEMORY); StrictMock<syncer::MockJsEventHandler> event_handler; @@ -294,7 +291,7 @@ TEST_F(ProfileSyncServiceTest, JsControllerProcessJsMessageBasic) { TEST_F(ProfileSyncServiceTest, JsControllerProcessJsMessageBasicDelayedBackendInitialization) { - StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, true, + StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, syncer::STORAGE_IN_MEMORY); StrictMock<syncer::MockJsReplyHandler> reply_handler; @@ -337,7 +334,7 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) { ASSERT_NE(-1, file_util::WriteFile(sync_file3, nonsense3, strlen(nonsense3))); - StartSyncServiceAndSetInitialSyncEnded(false, false, true, false, true, + StartSyncServiceAndSetInitialSyncEnded(false, false, true, false, syncer::STORAGE_ON_DISK); EXPECT_FALSE(service_->HasSyncSetupCompleted()); EXPECT_FALSE(service_->sync_initialized()); @@ -365,12 +362,69 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) { // recreate it. This test is useful mainly when it is run under valgrind. Its // expectations are not very interesting. TEST_F(ProfileSyncServiceTest, FailToOpenDatabase) { - StartSyncServiceAndSetInitialSyncEnded(false, true, true, true, false, + StartSyncServiceAndSetInitialSyncEnded(false, true, true, true, syncer::STORAGE_INVALID); // The backend is not ready. Ensure the PSS knows this. EXPECT_FALSE(service_->sync_initialized()); } +// Register for some IDs with the ProfileSyncService and trigger some +// invalidation messages. They should be received by the observer. +// Then unregister and trigger the invalidation messages again. Those +// shouldn't be received by the observer. +TEST_F(ProfileSyncServiceTest, UpdateRegisteredInvalidationIds) { + StartSyncService(); + + syncer::ObjectIdSet ids; + ids.insert(invalidation::ObjectId(1, "id1")); + ids.insert(invalidation::ObjectId(2, "id2")); + const syncer::ObjectIdPayloadMap& payloads = + syncer::ObjectIdSetToPayloadMap(ids, "payload"); + + StrictMock<syncer::MockSyncNotifierObserver> observer; + EXPECT_CALL(observer, OnNotificationsEnabled()); + EXPECT_CALL(observer, OnNotificationsDisabled( + syncer::TRANSIENT_NOTIFICATION_ERROR)); + EXPECT_CALL(observer, OnIncomingNotification( + payloads, syncer::REMOTE_NOTIFICATION)); + + service_->UpdateRegisteredInvalidationIds(&observer, ids); + + SyncBackendHostForProfileSyncTest* const backend = + service_->GetBackendForTest(); + + backend->EmitOnNotificationsEnabled(); + backend->EmitOnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR); + backend->EmitOnIncomingNotification(payloads, syncer::REMOTE_NOTIFICATION); + + Mock::VerifyAndClearExpectations(&observer); + + service_->UpdateRegisteredInvalidationIds(&observer, syncer::ObjectIdSet()); + + backend->EmitOnNotificationsEnabled(); + backend->EmitOnNotificationsDisabled(syncer::TRANSIENT_NOTIFICATION_ERROR); + backend->EmitOnIncomingNotification(payloads, syncer::REMOTE_NOTIFICATION); +} + +// Register for some IDs with the ProfileSyncService, restart sync, +// and trigger some invalidation messages. They should still be +// received by the observer. +TEST_F(ProfileSyncServiceTest, UpdateRegisteredInvalidationIdsPersistence) { + StartSyncService(); + + StrictMock<syncer::MockSyncNotifierObserver> observer; + EXPECT_CALL(observer, OnNotificationsEnabled()); + + syncer::ObjectIdSet ids; + ids.insert(invalidation::ObjectId(3, "id3")); + service_->UpdateRegisteredInvalidationIds(&observer, ids); + + service_->StopAndSuppress(); + service_->UnsuppressAndStart(); + + service_->GetBackendForTest()->EmitOnNotificationsEnabled(); +} + } // namespace } // namespace browser_sync diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc index 8eb7365..f6528e7 100644 --- a/chrome/browser/sync/test_profile_sync_service.cc +++ b/chrome/browser/sync/test_profile_sync_service.cc @@ -74,7 +74,6 @@ void SyncBackendHostForProfileSyncTest::InitCore( test_options.internal_components_factory = new TestInternalComponentsFactory(storage); SyncBackendHost::InitCore(test_options); - // TODO(akalin): Figure out a better way to do this. if (synchronous_init_) { // The SyncBackend posts a task to the current loop when // initialization completes. @@ -142,6 +141,21 @@ void SyncBackendHostForProfileSyncTest::SetInitialSyncEndedForAllTypes() { } } +void SyncBackendHostForProfileSyncTest::EmitOnNotificationsEnabled() { + frontend()->OnNotificationsEnabled(); +} + +void SyncBackendHostForProfileSyncTest::EmitOnNotificationsDisabled( + syncer::NotificationsDisabledReason reason) { + frontend()->OnNotificationsDisabled(reason); +} + +void SyncBackendHostForProfileSyncTest::EmitOnIncomingNotification( + const syncer::ObjectIdPayloadMap& id_payloads, + const syncer::IncomingNotificationSource source) { + frontend()->OnIncomingNotification(id_payloads, source); +} + } // namespace browser_sync syncer::TestIdFactory* TestProfileSyncService::id_factory() { diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h index d24c0fd..bffe8a7 100644 --- a/chrome/browser/sync/test_profile_sync_service.h +++ b/chrome/browser/sync/test_profile_sync_service.h @@ -33,6 +33,7 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost { public: // |synchronous_init| causes initialization to block until the syncapi has // completed setting itself up and called us back. + // TOOD(akalin): Remove |synchronous_init| (http://crbug.com/140354). SyncBackendHostForProfileSyncTest( Profile* profile, const base::WeakPtr<SyncPrefs>& sync_prefs, @@ -61,7 +62,13 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost { static void SetHistoryServiceExpectations(ProfileMock* profile); void SetInitialSyncEndedForAllTypes(); - void dont_set_initial_sync_ended_on_init(); + + void EmitOnNotificationsEnabled(); + void EmitOnNotificationsDisabled( + syncer::NotificationsDisabledReason reason); + void EmitOnIncomingNotification( + const syncer::ObjectIdPayloadMap& id_payloads, + const syncer::IncomingNotificationSource source); protected: virtual void InitCore(const DoInitializeOptions& options) OVERRIDE; diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 44fe3b5..de9a73e 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -958,6 +958,7 @@ # 3) anything tests directly depend on '../skia/skia.gyp:skia', '../third_party/bzip2/bzip2.gyp:bzip2', + '../third_party/cacheinvalidation/cacheinvalidation.gyp:cacheinvalidation', '../third_party/cld/cld.gyp:cld', '../third_party/icu/icu.gyp:icui18n', '../third_party/icu/icu.gyp:icuuc', 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', ], }, }, |