From 51e6efd247934b09cd531428411f6ed377e5665d Mon Sep 17 00:00:00 2001 From: "rlarocque@chromium.org" Date: Tue, 8 Jul 2014 23:38:21 +0000 Subject: sync: Inject sync/'s dependency on invalidations Defines an InvalidationInterface class and uses it to break any direct dependencies from sync code on syncer::Invalidation. Despite its name, syncer::Invalidation should belong solely to the invalidations component, which code in the sync/ directory should not depend on. Changes the interface in the sync engine from copying syncer::Invalidation to managing scoped_ptr. This change in memory management was required to support the use of an abstract interface. Removes the DroppedInvaldiationTracker. This class was previously only used by sync. The small benefit provided by this class is outweighed by the amount of glue code it would take to maintain it. Changes tests to conform to the new interface. Adds some test-only implementations of InvalidationInterface and some associated classes. BUG=259559 Review URL: https://codereview.chromium.org/322333004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@281884 0039d316-1c4b-4281-b951-d872f2087c98 --- sync/engine/get_updates_processor_unittest.cc | 18 +++++++++++++++--- sync/engine/sync_scheduler.h | 5 +++-- sync/engine/sync_scheduler_impl.cc | 9 ++++----- sync/engine/sync_scheduler_impl.h | 3 ++- sync/engine/sync_scheduler_unittest.cc | 23 ++++++++++++++--------- 5 files changed, 38 insertions(+), 20 deletions(-) (limited to 'sync/engine') diff --git a/sync/engine/get_updates_processor_unittest.cc b/sync/engine/get_updates_processor_unittest.cc index 7c87fed..54ce503 100644 --- a/sync/engine/get_updates_processor_unittest.cc +++ b/sync/engine/get_updates_processor_unittest.cc @@ -15,11 +15,23 @@ #include "sync/sessions/status_controller.h" #include "sync/test/engine/fake_model_worker.h" #include "sync/test/engine/mock_update_handler.h" +#include "sync/test/mock_invalidation.h" #include "sync/test/sessions/mock_debug_info_getter.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { +namespace { + +scoped_ptr BuildInvalidation( + int64 version, + const std::string& payload) { + return MockInvalidation::Build(version, payload) + .PassAs(); +} + +} // namespace + using sessions::MockDebugInfoGetter; // A test fixture for tests exercising download updates functions. @@ -133,11 +145,11 @@ TEST_F(GetUpdatesProcessorTest, BookmarkNudge) { TEST_F(GetUpdatesProcessorTest, NotifyMany) { sessions::NudgeTracker nudge_tracker; nudge_tracker.RecordRemoteInvalidation( - BuildInvalidationMap(AUTOFILL, 1, "autofill_payload")); + AUTOFILL, BuildInvalidation(1, "autofill_payload")); nudge_tracker.RecordRemoteInvalidation( - BuildInvalidationMap(BOOKMARKS, 1, "bookmark_payload")); + BOOKMARKS, BuildInvalidation(1, "bookmark_payload")); nudge_tracker.RecordRemoteInvalidation( - BuildInvalidationMap(PREFERENCES, 1, "preferences_payload")); + PREFERENCES, BuildInvalidation(1, "preferences_payload")); ModelTypeSet notified_types; notified_types.Put(AUTOFILL); notified_types.Put(BOOKMARKS); diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h index aef0118..b512042 100644 --- a/sync/engine/sync_scheduler.h +++ b/sync/engine/sync_scheduler.h @@ -13,6 +13,7 @@ #include "base/time/time.h" #include "sync/base/sync_export.h" #include "sync/engine/nudge_source.h" +#include "sync/internal_api/public/base/invalidation_interface.h" #include "sync/sessions/sync_session.h" namespace tracked_objects { @@ -21,7 +22,6 @@ class Location; namespace syncer { -class ObjectIdInvalidationMap; struct ServerConnectionEvent; struct SYNC_EXPORT_PRIVATE ConfigurationParams { @@ -115,7 +115,8 @@ class SYNC_EXPORT_PRIVATE SyncScheduler // order to fetch the update. virtual void ScheduleInvalidationNudge( const base::TimeDelta& desired_delay, - const ObjectIdInvalidationMap& invalidations, + syncer::ModelType type, + scoped_ptr invalidation, const tracked_objects::Location& nudge_location) = 0; // Change status of notifications in the SyncSessionContext. diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 01a054a..c5629ba 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -387,16 +387,15 @@ void SyncSchedulerImpl::ScheduleLocalRefreshRequest( void SyncSchedulerImpl::ScheduleInvalidationNudge( const TimeDelta& desired_delay, - const ObjectIdInvalidationMap& invalidation_map, + syncer::ModelType model_type, + scoped_ptr invalidation, const tracked_objects::Location& nudge_location) { DCHECK(CalledOnValidThread()); - DCHECK(!invalidation_map.Empty()); SDVLOG_LOC(nudge_location, 2) << "Scheduling sync because we received invalidation for " - << ModelTypeSetToString( - ObjectIdSetToModelTypeSet(invalidation_map.GetObjectIds())); - nudge_tracker_.RecordRemoteInvalidation(invalidation_map); + << ModelTypeToString(model_type); + nudge_tracker_.RecordRemoteInvalidation(model_type, invalidation.Pass()); ScheduleNudgeImpl(desired_delay, nudge_location); } diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h index 75cad3e..d78b5d6 100644 --- a/sync/engine/sync_scheduler_impl.h +++ b/sync/engine/sync_scheduler_impl.h @@ -65,7 +65,8 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl const tracked_objects::Location& nudge_location) OVERRIDE; virtual void ScheduleInvalidationNudge( const base::TimeDelta& desired_delay, - const ObjectIdInvalidationMap& invalidation_map, + syncer::ModelType type, + scoped_ptr invalidation, const tracked_objects::Location& nudge_location) OVERRIDE; virtual void SetNotificationsEnabled(bool notifications_enabled) OVERRIDE; diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index c231087..8348eab 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -20,6 +20,7 @@ #include "sync/test/engine/fake_model_worker.h" #include "sync/test/engine/mock_connection_manager.h" #include "sync/test/engine/test_directory_setter_upper.h" +#include "sync/test/mock_invalidation.h" #include "sync/util/extensions_activity.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -220,6 +221,13 @@ class SyncSchedulerTest : public testing::Test { return scheduler_->retry_timer_.GetCurrentDelay(); } + static scoped_ptr BuildInvalidation( + int64 version, + const std::string& payload) { + return MockInvalidation::Build(version, payload) + .PassAs(); + } + private: syncable::Directory* directory() { return dir_maker_.directory(); @@ -523,25 +531,23 @@ TEST_F(SyncSchedulerTest, NudgeWithStates) { StartSyncScheduler(SyncScheduler::NORMAL_MODE); SyncShareTimes times1; - ObjectIdInvalidationMap invalidations1 = - BuildInvalidationMap(BOOKMARKS, 10, "test"); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), RecordSyncShare(×1))) .RetiresOnSaturation(); - scheduler()->ScheduleInvalidationNudge(zero(), invalidations1, FROM_HERE); + scheduler()->ScheduleInvalidationNudge( + zero(), BOOKMARKS, BuildInvalidation(10, "test"), FROM_HERE); RunLoop(); Mock::VerifyAndClearExpectations(syncer()); // Make sure a second, later, nudge is unaffected by first (no coalescing). SyncShareTimes times2; - ObjectIdInvalidationMap invalidations2 = - BuildInvalidationMap(AUTOFILL, 10, "test2"); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), RecordSyncShare(×2))); - scheduler()->ScheduleInvalidationNudge(zero(), invalidations2, FROM_HERE); + scheduler()->ScheduleInvalidationNudge( + zero(), AUTOFILL, BuildInvalidation(10, "test2"), FROM_HERE); RunLoop(); } @@ -834,9 +840,8 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) { EXPECT_TRUE(GetThrottledTypes().HasAll(throttled_types)); // Ignore invalidations for throttled types. - ObjectIdInvalidationMap invalidations = - BuildInvalidationMap(BOOKMARKS, 10, "test"); - scheduler()->ScheduleInvalidationNudge(zero(), invalidations, FROM_HERE); + scheduler()->ScheduleInvalidationNudge( + zero(), BOOKMARKS, BuildInvalidation(10, "test"), FROM_HERE); PumpLoop(); // Ignore refresh requests for throttled types. -- cgit v1.1