diff options
36 files changed, 343 insertions, 575 deletions
diff --git a/chrome/browser/sync/about_sync_util.cc b/chrome/browser/sync/about_sync_util.cc index 1b9d149..bdf1411 100644 --- a/chrome/browser/sync/about_sync_util.cc +++ b/chrome/browser/sync/about_sync_util.cc @@ -335,8 +335,11 @@ scoped_ptr<DictionaryValue> ConstructAboutInformation( } if (snapshot.is_initialized()) { - session_source.SetValue( - syncer::GetUpdatesSourceString(snapshot.source().updates_source)); + if (snapshot.legacy_updates_source() != + sync_pb::GetUpdatesCallerInfo::UNKNOWN) { + session_source.SetValue( + syncer::GetUpdatesSourceString(snapshot.legacy_updates_source())); + } get_key_result.SetValue( GetSyncerErrorString( snapshot.model_neutral_state().last_get_key_result)); diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc index c89df6d..67aea310 100644 --- a/chrome/browser/sync/profile_sync_service_harness.cc +++ b/chrome/browser/sync/profile_sync_service_harness.cc @@ -501,9 +501,14 @@ void ProfileSyncServiceHarness::OnSyncCycleCompleted() { bool is_notifiable_commit = (snap.model_neutral_state().num_successful_commits > 0); if (is_notifiable_commit && p2p_invalidation_service_) { - const syncer::ObjectIdInvalidationMap& invalidation_map = - ModelTypeInvalidationMapToObjectIdInvalidationMap( - snap.source().types); + syncer::ModelTypeSet model_types = + snap.model_neutral_state().commit_request_types; + syncer::ObjectIdSet ids = ModelTypeSetToObjectIdSet(model_types); + syncer::ObjectIdInvalidationMap invalidation_map = + syncer::ObjectIdSetToInvalidationMap( + ids, + syncer::Invalidation::kUnknownVersion, + ""); p2p_invalidation_service_->SendInvalidation(invalidation_map); } diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc index 3c659d7..bb54416 100644 --- a/chrome/browser/sync/test_profile_sync_service.cc +++ b/chrome/browser/sync/test_profile_sync_service.cc @@ -23,7 +23,6 @@ using syncer::ModelSafeRoutingInfo; using syncer::TestInternalComponentsFactory; using syncer::sessions::ModelNeutralState; using syncer::sessions::SyncSessionSnapshot; -using syncer::sessions::SyncSourceInfo; using syncer::UserShare; using syncer::syncable::Directory; using syncer::DEVICE_INFO; diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc index 7596d4b..ee69812 100644 --- a/sync/engine/commit.cc +++ b/sync/engine/commit.cc @@ -102,6 +102,7 @@ SyncerError BuildAndPostCommitsImpl(ModelTypeSet requested_types, Syncer* syncer, sessions::SyncSession* session, sessions::OrderedCommitSet* commit_set) { + ModelTypeSet commit_request_types; while (!syncer->ExitRequested()) { sync_pb::ClientToServerMessage commit_message; ExtensionsActivity::Records extensions_activity_buffer; @@ -114,6 +115,10 @@ SyncerError BuildAndPostCommitsImpl(ModelTypeSet requested_types, break; } + commit_request_types.PutAll(commit_set->Types()); + session->mutable_status_controller()->set_commit_request_types( + commit_request_types); + sync_pb::ClientToServerResponse commit_response; DVLOG(1) << "Sending commit message."; diff --git a/sync/engine/download.cc b/sync/engine/download.cc index 37f2372..74bc4e2 100644 --- a/sync/engine/download.cc +++ b/sync/engine/download.cc @@ -214,7 +214,7 @@ SyncerError NormalDownloadUpdates( SyncerError DownloadUpdatesForConfigure( SyncSession* session, bool create_mobile_bookmarks_folder, - const syncer::sessions::SyncSourceInfo& source, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, ModelTypeSet request_types) { sync_pb::ClientToServerMessage client_to_server_message; InitDownloadUpdatesRequest( @@ -231,11 +231,11 @@ SyncerError DownloadUpdatesForConfigure( DCHECK(!request_types.Empty()); // Set legacy GetUpdatesMessage.GetUpdatesCallerInfo information. - get_updates->mutable_caller_info()->set_source(source.updates_source); + get_updates->mutable_caller_info()->set_source(source); // Set the new and improved version of source, too. sync_pb::SyncEnums::GetUpdatesOrigin origin = - ConvertConfigureSourceToOrigin(source.updates_source); + ConvertConfigureSourceToOrigin(source); get_updates->set_get_updates_origin(origin); return ExecuteDownloadUpdates(session, &client_to_server_message); @@ -257,8 +257,6 @@ SyncerError DownloadUpdatesForPoll( DVLOG(1) << "Polling for types " << ModelTypeSetToString(request_types); DCHECK(!request_types.Empty()); - DCHECK_EQ(sync_pb::GetUpdatesCallerInfo::PERIODIC, - session->source().updates_source); // Set legacy GetUpdatesMessage.GetUpdatesCallerInfo information. get_updates->mutable_caller_info()->set_source( diff --git a/sync/engine/download.h b/sync/engine/download.h index 5b5c2d0..4ec4b5a 100644 --- a/sync/engine/download.h +++ b/sync/engine/download.h @@ -8,6 +8,7 @@ #include "sync/base/sync_export.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/util/syncer_error.h" +#include "sync/protocol/sync.pb.h" namespace sync_pb { class DebugInfo; @@ -18,7 +19,6 @@ namespace syncer { namespace sessions { class NudgeTracker; class SyncSession; -struct SyncSourceInfo; } // namespace sessions class Syncer; @@ -38,7 +38,7 @@ SYNC_EXPORT_PRIVATE SyncerError NormalDownloadUpdates( SYNC_EXPORT_PRIVATE SyncerError DownloadUpdatesForConfigure( sessions::SyncSession* session, bool create_mobile_bookmarks_folder, - const syncer::sessions::SyncSourceInfo& source, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, ModelTypeSet request_types); // This function executes a single GetUpdate request and stores the response in diff --git a/sync/engine/download_unittest.cc b/sync/engine/download_unittest.cc index c6ce9556..9990b7b 100644 --- a/sync/engine/download_unittest.cc +++ b/sync/engine/download_unittest.cc @@ -45,8 +45,7 @@ TEST_F(DownloadUpdatesTest, ExecuteNoStates) { GetRoutingInfoTypes(routing_info())); scoped_ptr<sessions::SyncSession> session( sessions::SyncSession::Build(context(), - delegate(), - nudge_tracker.GetSourceInfo())); + delegate())); NormalDownloadUpdates(session.get(), false, GetRoutingInfoTypes(routing_info()), @@ -67,14 +66,23 @@ TEST_F(DownloadUpdatesTest, ExecuteWithStates) { ModelTypeSetToInvalidationMap(ModelTypeSet(PREFERENCES), "preferences_payload")); + ModelTypeInvalidationMap invalidation_map; + Invalidation i1; + i1.payload = "autofill_payload"; + invalidation_map.insert(std::make_pair(AUTOFILL, i1)); + Invalidation i2; + i2.payload = "bookmark_payload"; + invalidation_map.insert(std::make_pair(BOOKMARKS, i2)); + Invalidation i3; + i3.payload = "preferences_payload"; + invalidation_map.insert(std::make_pair(PREFERENCES, i3)); + mock_server()->ExpectGetUpdatesRequestTypes( GetRoutingInfoTypes(routing_info())); mock_server()->ExpectGetUpdatesRequestStates( - nudge_tracker.GetSourceInfo().types); + invalidation_map); scoped_ptr<sessions::SyncSession> session( - sessions::SyncSession::Build(context(), - delegate(), - nudge_tracker.GetSourceInfo())); + sessions::SyncSession::Build(context(), delegate())); NormalDownloadUpdates(session.get(), false, GetRoutingInfoTypes(routing_info()), diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 52e1d49..e4509e4 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -28,7 +28,6 @@ namespace syncer { using sessions::SyncSession; using sessions::SyncSessionSnapshot; -using sessions::SyncSourceInfo; using sync_pb::GetUpdatesCallerInfo; namespace { @@ -248,8 +247,7 @@ ModelTypeSet SyncSchedulerImpl::GetEnabledAndUnthrottledTypes() { void SyncSchedulerImpl::SendInitialSnapshot() { DCHECK(CalledOnValidThread()); - scoped_ptr<SyncSession> dummy( - SyncSession::Build(session_context_, this, SyncSourceInfo())); + scoped_ptr<SyncSession> dummy(SyncSession::Build(session_context_, this)); SyncEngineEvent event(SyncEngineEvent::STATUS_CHANGED); event.snapshot = dummy->TakeSnapshot(); session_context_->NotifyListeners(event); @@ -469,11 +467,7 @@ void SyncSchedulerImpl::DoNudgeSyncSessionJob(JobPriority priority) { DVLOG(2) << "Will run normal mode sync cycle with routing info " << ModelSafeRoutingInfoToString(session_context_->routing_info()); - scoped_ptr<SyncSession> session( - SyncSession::Build( - session_context_, - this, - nudge_tracker_.GetSourceInfo())); + scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this)); bool premature_exit = !syncer_->NormalSyncShare( GetEnabledAndUnthrottledTypes(), nudge_tracker_, @@ -512,14 +506,10 @@ bool SyncSchedulerImpl::DoConfigurationSyncSessionJob(JobPriority priority) { SDVLOG(2) << "Will run configure SyncShare with routes " << ModelSafeRoutingInfoToString(session_context_->routing_info()); - SyncSourceInfo source_info(pending_configure_params_->source, - ModelSafeRoutingInfoToInvalidationMap( - session_context_->routing_info(), - std::string())); - scoped_ptr<SyncSession> session( - SyncSession::Build(session_context_, this, source_info)); + scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this)); bool premature_exit = !syncer_->ConfigureSyncShare( GetRoutingInfoTypes(session_context_->routing_info()), + pending_configure_params_->source, session.get()); AdjustPolling(FORCE_RESET); // Don't run poll job till the next time poll timer fires. @@ -565,7 +555,6 @@ void SyncSchedulerImpl::DoPollSyncSessionJob() { ModelSafeRoutingInfo r; ModelTypeInvalidationMap invalidation_map = ModelSafeRoutingInfoToInvalidationMap(r, std::string()); - SyncSourceInfo info(GetUpdatesCallerInfo::PERIODIC, invalidation_map); base::AutoReset<bool> protector(&no_scheduling_allowed_, true); if (!CanRunJobNow(NORMAL_PRIORITY)) { @@ -580,8 +569,7 @@ void SyncSchedulerImpl::DoPollSyncSessionJob() { SDVLOG(2) << "Polling with routes " << ModelSafeRoutingInfoToString(session_context_->routing_info()); - scoped_ptr<SyncSession> session( - SyncSession::Build(session_context_, this, info)); + scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this)); syncer_->PollSyncShare( GetEnabledAndUnthrottledTypes(), session.get()); diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index a09079a..f76e1c5 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -11,7 +11,6 @@ #include "sync/engine/backoff_delay_provider.h" #include "sync/engine/sync_scheduler_impl.h" #include "sync/engine/syncer.h" -#include "sync/internal_api/public/base/model_type_invalidation_map_test_util.h" #include "sync/sessions/test_util.h" #include "sync/test/callback_counter.h" #include "sync/test/engine/fake_model_worker.h" @@ -26,10 +25,8 @@ using base::TimeTicks; using testing::_; using testing::AtLeast; using testing::DoAll; -using testing::Eq; using testing::Invoke; using testing::Mock; -using testing::Not; using testing::Return; using testing::WithArg; using testing::WithArgs; @@ -37,7 +34,6 @@ using testing::WithArgs; namespace syncer { using sessions::SyncSession; using sessions::SyncSessionContext; -using sessions::SyncSessionSnapshot; using sync_pb::GetUpdatesCallerInfo; class MockSyncer : public Syncer { @@ -45,15 +41,14 @@ class MockSyncer : public Syncer { MOCK_METHOD3(NormalSyncShare, bool(ModelTypeSet, const sessions::NudgeTracker&, sessions::SyncSession*)); - MOCK_METHOD2(ConfigureSyncShare, bool(ModelTypeSet, sessions::SyncSession*)); + MOCK_METHOD3(ConfigureSyncShare, + bool(ModelTypeSet, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource, + SyncSession*)); MOCK_METHOD2(PollSyncShare, bool(ModelTypeSet, sessions::SyncSession*)); }; -// Used when tests want to record syncing activity to examine later. -struct SyncShareRecords { - std::vector<TimeTicks> times; - std::vector<SyncSessionSnapshot> snapshots; -}; +typedef std::vector<TimeTicks> SyncShareTimes; void QuitLoopNow() { // We use QuitNow() instead of Quit() as the latter may get stalled @@ -90,22 +85,6 @@ ModelSafeRoutingInfo TypesToRoutingInfo(ModelTypeSet types) { return routes; } -// Compare a ModelTypeSet to a ModelTypeInvalidationMap, ignoring -// state values. -testing::AssertionResult ModelTypeSetMatchesInvalidationMap( - ModelTypeSet lhs, const ModelTypeInvalidationMap& rhs) { - ModelTypeSet rhs_set = ModelTypeInvalidationMapToSet(rhs); - - if (!rhs_set.Equals(rhs_set)) { - return testing::AssertionFailure() - << "ModelTypeSet: " << ModelTypeSetToString(lhs) - << " does not match ModelTypeInvalidationMap: " - << ModelTypeSetToString(rhs_set); - } else { - return testing::AssertionSuccess(); - } -} - // Convenient to use in tests wishing to analyze SyncShare calls over time. static const size_t kMinNumSamples = 5; class SyncSchedulerTest : public testing::Test { @@ -124,7 +103,7 @@ class SyncSchedulerTest : public testing::Test { virtual void SetUp() { dir_maker_.SetUp(); - syncer_ = new MockSyncer(); + syncer_ = new testing::StrictMock<MockSyncer>(); delay_ = NULL; extensions_activity_ = new ExtensionsActivity(); @@ -180,16 +159,13 @@ class SyncSchedulerTest : public testing::Test { dir_maker_.TearDown(); } - void AnalyzePollRun(const SyncShareRecords& records, size_t min_num_samples, + void AnalyzePollRun(const SyncShareTimes& times, size_t min_num_samples, const TimeTicks& optimal_start, const TimeDelta& poll_interval) { - const std::vector<TimeTicks>& data(records.times); - EXPECT_GE(data.size(), min_num_samples); - for (size_t i = 0; i < data.size(); i++) { + EXPECT_GE(times.size(), min_num_samples); + for (size_t i = 0; i < times.size(); i++) { SCOPED_TRACE(testing::Message() << "SyncShare # (" << i << ")"); TimeTicks optimal_next_sync = optimal_start + poll_interval * i; - EXPECT_GE(data[i], optimal_next_sync); - EXPECT_EQ(GetUpdatesCallerInfo::PERIODIC, - records.snapshots[i].source().updates_source); + EXPECT_GE(times[i], optimal_next_sync); } } @@ -249,22 +225,21 @@ class SyncSchedulerTest : public testing::Test { ModelSafeRoutingInfo routing_info_; }; -void RecordSyncShareImpl(SyncSession* s, SyncShareRecords* record) { - record->times.push_back(TimeTicks::Now()); - record->snapshots.push_back(s->TakeSnapshot()); +void RecordSyncShareImpl(SyncShareTimes* times) { + times->push_back(TimeTicks::Now()); } -ACTION_P(RecordSyncShare, record) { - RecordSyncShareImpl(arg0, record); +ACTION_P(RecordSyncShare, times) { + RecordSyncShareImpl(times); if (base::MessageLoop::current()->is_running()) QuitLoopNow(); return true; } -ACTION_P2(RecordSyncShareMultiple, record, quit_after) { - RecordSyncShareImpl(arg0, record); - EXPECT_LE(record->times.size(), quit_after); - if (record->times.size() >= quit_after && +ACTION_P2(RecordSyncShareMultiple, times, quit_after) { + RecordSyncShareImpl(times); + EXPECT_LE(times->size(), quit_after); + if (times->size() >= quit_after && base::MessageLoop::current()->is_running()) { QuitLoopNow(); } @@ -284,12 +259,12 @@ ACTION(QuitLoopNowAction) { // Test nudge scheduling. TEST_F(SyncSchedulerTest, Nudge) { - SyncShareRecords records; + SyncShareTimes times; ModelTypeSet model_types(BOOKMARKS); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records)))) + RecordSyncShare(×))) .RetiresOnSaturation(); StartSyncScheduler(SyncScheduler::NORMAL_MODE); @@ -297,42 +272,28 @@ TEST_F(SyncSchedulerTest, Nudge) { scheduler()->ScheduleLocalNudge(zero(), model_types, FROM_HERE); RunLoop(); - ASSERT_EQ(1U, records.snapshots.size()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - model_types, - records.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - records.snapshots[0].source().updates_source); - Mock::VerifyAndClearExpectations(syncer()); // Make sure a second, later, nudge is unaffected by first (no coalescing). - SyncShareRecords records2; + SyncShareTimes times2; model_types.Remove(BOOKMARKS); model_types.Put(AUTOFILL); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records2)))); + RecordSyncShare(×2))); scheduler()->ScheduleLocalNudge(zero(), model_types, FROM_HERE); RunLoop(); - - ASSERT_EQ(1U, records2.snapshots.size()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - model_types, - records2.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - records2.snapshots[0].source().updates_source); } // Make sure a regular config command is scheduled fine in the absence of any // errors. TEST_F(SyncSchedulerTest, Config) { - SyncShareRecords records; + SyncShareTimes times; const ModelTypeSet model_types(BOOKMARKS); - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), - WithArg<1>(RecordSyncShare(&records)))); + RecordSyncShare(×))); StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE); @@ -344,13 +305,6 @@ TEST_F(SyncSchedulerTest, Config) { base::Bind(&CallbackCounter::Callback, base::Unretained(&counter))); ASSERT_TRUE(scheduler()->ScheduleConfiguration(params)); ASSERT_EQ(1, counter.times_called()); - - ASSERT_EQ(1U, records.snapshots.size()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - model_types, - records.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION, - records.snapshots[0].source().updates_source); } // Simulate a failure and make sure the config request is retried. @@ -358,18 +312,15 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) { UseMockDelayProvider(); EXPECT_CALL(*delay(), GetDelay(_)) .WillRepeatedly(Return(TimeDelta::FromMilliseconds(1))); - SyncShareRecords records; + SyncShareTimes times; const ModelTypeSet model_types(BOOKMARKS); - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed), - WithArg<1>(RecordSyncShare(&records)))) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), - WithArg<1>(RecordSyncShare(&records)))); - StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE); - ASSERT_EQ(0U, records.snapshots.size()); + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed), + RecordSyncShare(×))); + CallbackCounter counter; ConfigurationParams params( GetUpdatesCallerInfo::RECONFIGURATION, @@ -379,16 +330,14 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) { ASSERT_FALSE(scheduler()->ScheduleConfiguration(params)); ASSERT_EQ(0, counter.times_called()); - ASSERT_EQ(1U, records.snapshots.size()); + Mock::VerifyAndClearExpectations(syncer()); + + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), + RecordSyncShare(×))); RunLoop(); - ASSERT_EQ(2U, records.snapshots.size()); ASSERT_EQ(1, counter.times_called()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - model_types, - records.snapshots[1].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION, - records.snapshots[1].source().updates_source); } // Issue a nudge when the config has failed. Make sure both the config and @@ -398,23 +347,14 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) { UseMockDelayProvider(); EXPECT_CALL(*delay(), GetDelay(_)) .WillRepeatedly(Return(TimeDelta::FromMilliseconds(50))); - SyncShareRecords records; - - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed), - WithArg<1>(RecordSyncShare(&records)))) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed), - WithArg<1>(RecordSyncShare(&records)))) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), - WithArg<1>(RecordSyncShare(&records)))); - - EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records)))); + SyncShareTimes times; StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE); - ASSERT_EQ(0U, records.snapshots.size()); + // Request a configure and make sure it fails. + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed), + RecordSyncShare(×))); CallbackCounter counter; ConfigurationParams params( GetUpdatesCallerInfo::RECONFIGURATION, @@ -423,50 +363,41 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) { base::Bind(&CallbackCounter::Callback, base::Unretained(&counter))); ASSERT_FALSE(scheduler()->ScheduleConfiguration(params)); ASSERT_EQ(0, counter.times_called()); - ASSERT_EQ(1U, records.snapshots.size()); + Mock::VerifyAndClearExpectations(syncer()); + // Ask for a nudge while dealing with repeated configure failure. + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureFailed), + RecordSyncShare(×))); scheduler()->ScheduleLocalNudge(zero(), model_types, FROM_HERE); RunLoop(); // Note that we're not RunLoop()ing for the NUDGE we just scheduled, but // for the first retry attempt from the config job (after // waiting ~+/- 50ms). - ASSERT_EQ(2U, records.snapshots.size()); + Mock::VerifyAndClearExpectations(syncer()); ASSERT_EQ(0, counter.times_called()); - EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION, - records.snapshots[1].source().updates_source); + // Let the next configure retry succeed. + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), + RecordSyncShare(×))); RunLoop(); - // This is the 3rd attempt, which we've set up to SimulateSuccess. - ASSERT_EQ(3U, records.snapshots.size()); - ASSERT_EQ(1, counter.times_called()); // Now change the mode so nudge can execute. + EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), + RecordSyncShare(×))); StartSyncScheduler(SyncScheduler::NORMAL_MODE); - - ASSERT_EQ(4U, records.snapshots.size()); - - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - model_types, - records.snapshots[2].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION, - records.snapshots[2].source().updates_source); - - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - model_types, - records.snapshots[3].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - records.snapshots[3].source().updates_source); - } // Test that nudges are coalesced. TEST_F(SyncSchedulerTest, NudgeCoalescing) { StartSyncScheduler(SyncScheduler::NORMAL_MODE); - SyncShareRecords r; + SyncShareTimes times; EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&r)))); + RecordSyncShare(×))); const ModelTypeSet types1(BOOKMARKS), types2(AUTOFILL), types3(THEMES); TimeDelta delay = zero(); TimeTicks optimal_time = TimeTicks::Now() + delay; @@ -474,39 +405,27 @@ TEST_F(SyncSchedulerTest, NudgeCoalescing) { scheduler()->ScheduleLocalNudge(zero(), types2, FROM_HERE); RunLoop(); - ASSERT_EQ(1U, r.snapshots.size()); - EXPECT_GE(r.times[0], optimal_time); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - Union(types1, types2), - r.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - r.snapshots[0].source().updates_source); + ASSERT_EQ(1U, times.size()); + EXPECT_GE(times[0], optimal_time); Mock::VerifyAndClearExpectations(syncer()); - SyncShareRecords r2; + SyncShareTimes times2; EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&r2)))); + RecordSyncShare(×2))); scheduler()->ScheduleLocalNudge(zero(), types3, FROM_HERE); RunLoop(); - - ASSERT_EQ(1U, r2.snapshots.size()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - types3, - r2.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - r2.snapshots[0].source().updates_source); } // Test that nudges are coalesced. TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) { StartSyncScheduler(SyncScheduler::NORMAL_MODE); - SyncShareRecords r; + SyncShareTimes times; EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&r)))); + RecordSyncShare(×))); ModelTypeSet types1(BOOKMARKS), types2(AUTOFILL), types3; // Create a huge time delay. @@ -519,64 +438,50 @@ TEST_F(SyncSchedulerTest, NudgeCoalescingWithDifferentTimings) { TimeTicks max_time = TimeTicks::Now() + delay; RunLoop(); - - // Make sure the sync has happened. - ASSERT_EQ(1U, r.snapshots.size()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - Union(types1, types2), - r.snapshots[0].source().types)); + Mock::VerifyAndClearExpectations(syncer()); // Make sure the sync happened at the right time. - EXPECT_GE(r.times[0], min_time); - EXPECT_LE(r.times[0], max_time); + ASSERT_EQ(1U, times.size()); + EXPECT_GE(times[0], min_time); + EXPECT_LE(times[0], max_time); } // Test nudge scheduling. TEST_F(SyncSchedulerTest, NudgeWithStates) { StartSyncScheduler(SyncScheduler::NORMAL_MODE); - SyncShareRecords records; + SyncShareTimes times; const ModelTypeSet types(BOOKMARKS); ModelTypeInvalidationMap invalidation_map = ModelTypeSetToInvalidationMap(types, "test"); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records)))) + RecordSyncShare(×))) .RetiresOnSaturation(); scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE); RunLoop(); - ASSERT_EQ(1U, records.snapshots.size()); - EXPECT_THAT(invalidation_map, Eq(records.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::NOTIFICATION, - records.snapshots[0].source().updates_source); - Mock::VerifyAndClearExpectations(syncer()); // Make sure a second, later, nudge is unaffected by first (no coalescing). - SyncShareRecords records2; + SyncShareTimes times2; invalidation_map.erase(BOOKMARKS); invalidation_map[AUTOFILL].payload = "test2"; EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records2)))); + RecordSyncShare(×2))); scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE); RunLoop(); - - ASSERT_EQ(1U, records2.snapshots.size()); - EXPECT_THAT(invalidation_map, Eq(records2.snapshots[0].source().types)); - EXPECT_EQ(GetUpdatesCallerInfo::NOTIFICATION, - records2.snapshots[0].source().updates_source); } // Test that polling works as expected. TEST_F(SyncSchedulerTest, Polling) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll_interval(TimeDelta::FromMilliseconds(30)); EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples)) .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples)))); + RecordSyncShareMultiple(×, kMinNumSamples))); scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval); @@ -587,16 +492,16 @@ TEST_F(SyncSchedulerTest, Polling) { RunLoop(); StopSyncScheduler(); - AnalyzePollRun(records, kMinNumSamples, optimal_start, poll_interval); + AnalyzePollRun(times, kMinNumSamples, optimal_start, poll_interval); } // Test that the short poll interval is used. TEST_F(SyncSchedulerTest, PollNotificationsDisabled) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll_interval(TimeDelta::FromMilliseconds(30)); EXPECT_CALL(*syncer(), PollSyncShare(_,_)).Times(AtLeast(kMinNumSamples)) .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples)))); + RecordSyncShareMultiple(×, kMinNumSamples))); scheduler()->OnReceivedShortPollIntervalUpdate(poll_interval); scheduler()->SetNotificationsEnabled(false); @@ -608,12 +513,12 @@ TEST_F(SyncSchedulerTest, PollNotificationsDisabled) { RunLoop(); StopSyncScheduler(); - AnalyzePollRun(records, kMinNumSamples, optimal_start, poll_interval); + AnalyzePollRun(times, kMinNumSamples, optimal_start, poll_interval); } // Test that polling intervals are updated when needed. TEST_F(SyncSchedulerTest, PollIntervalUpdate) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll1(TimeDelta::FromMilliseconds(120)); TimeDelta poll2(TimeDelta::FromMilliseconds(30)); scheduler()->OnReceivedLongPollIntervalUpdate(poll1); @@ -625,7 +530,7 @@ TEST_F(SyncSchedulerTest, PollIntervalUpdate) { .WillRepeatedly( DoAll(Invoke(sessions::test_util::SimulatePollSuccess), WithArg<1>( - RecordSyncShareMultiple(&records, kMinNumSamples)))); + RecordSyncShareMultiple(×, kMinNumSamples)))); TimeTicks optimal_start = TimeTicks::Now() + poll1 + poll2; StartSyncScheduler(SyncScheduler::NORMAL_MODE); @@ -634,12 +539,12 @@ TEST_F(SyncSchedulerTest, PollIntervalUpdate) { RunLoop(); StopSyncScheduler(); - AnalyzePollRun(records, kMinNumSamples, optimal_start, poll2); + AnalyzePollRun(times, kMinNumSamples, optimal_start, poll2); } // Test that the sessions commit delay is updated when needed. TEST_F(SyncSchedulerTest, SessionsCommitDelay) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta delay1(TimeDelta::FromMilliseconds(120)); TimeDelta delay2(TimeDelta::FromMilliseconds(30)); scheduler()->OnReceivedSessionsCommitDelay(delay1); @@ -672,9 +577,9 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) { TimeDelta throttle(TimeDelta::FromMinutes(10)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) .WillOnce(DoAll( - WithArg<1>(sessions::test_util::SimulateThrottled(throttle)), + WithArg<2>(sessions::test_util::SimulateThrottled(throttle)), Return(true))) .WillRepeatedly(AddFailureAndQuitLoopNow()); @@ -697,7 +602,7 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) { } TEST_F(SyncSchedulerTest, ThrottlingExpiresFromPoll) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll(TimeDelta::FromMilliseconds(15)); TimeDelta throttle1(TimeDelta::FromMilliseconds(150)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); @@ -710,7 +615,7 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromPoll) { .RetiresOnSaturation(); EXPECT_CALL(*syncer(), PollSyncShare(_,_)) .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples)))); + RecordSyncShareMultiple(×, kMinNumSamples))); TimeTicks optimal_start = TimeTicks::Now() + poll + throttle1; StartSyncScheduler(SyncScheduler::NORMAL_MODE); @@ -719,11 +624,11 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromPoll) { RunLoop(); StopSyncScheduler(); - AnalyzePollRun(records, kMinNumSamples, optimal_start, poll); + AnalyzePollRun(times, kMinNumSamples, optimal_start, poll); } TEST_F(SyncSchedulerTest, ThrottlingExpiresFromNudge) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll(TimeDelta::FromDays(1)); TimeDelta throttle1(TimeDelta::FromMilliseconds(150)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); @@ -751,18 +656,18 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromNudge) { } TEST_F(SyncSchedulerTest, ThrottlingExpiresFromConfigure) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll(TimeDelta::FromDays(1)); TimeDelta throttle1(TimeDelta::FromMilliseconds(150)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); ::testing::InSequence seq; - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) .WillOnce(DoAll( - WithArg<1>(sessions::test_util::SimulateThrottled(throttle1)), + WithArg<2>(sessions::test_util::SimulateThrottled(throttle1)), Return(true))) .RetiresOnSaturation(); - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), QuitLoopNowAction())); @@ -821,7 +726,7 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) { EXPECT_CALL(*delay(), GetDelay(_)) .WillRepeatedly(Return(zero())); - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll(TimeDelta::FromDays(1)); TimeDelta throttle1(TimeDelta::FromSeconds(60)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); @@ -837,14 +742,10 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) { throttled_types, throttle1)), Return(true))) .RetiresOnSaturation(); - EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) - .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records)))); StartSyncScheduler(SyncScheduler::NORMAL_MODE); scheduler()->ScheduleLocalNudge(zero(), throttled_types, FROM_HERE); PumpLoop(); - EXPECT_EQ(0U, records.snapshots.size()); EXPECT_TRUE(GetThrottledTypes().HasAll(throttled_types)); // Ignore invalidations for throttled types. @@ -852,17 +753,20 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) { ModelTypeSetToInvalidationMap(throttled_types, "test"); scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE); PumpLoop(); - EXPECT_EQ(0U, records.snapshots.size()); // Ignore refresh requests for throttled types. scheduler()->ScheduleLocalRefreshRequest(zero(), throttled_types, FROM_HERE); PumpLoop(); - EXPECT_EQ(0U, records.snapshots.size()); + + Mock::VerifyAndClearExpectations(syncer()); // Local nudges for non-throttled types will trigger a sync. + EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) + .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), + RecordSyncShare(×))); scheduler()->ScheduleLocalNudge(zero(), unthrottled_types, FROM_HERE); RunLoop(); - EXPECT_EQ(1U, records.snapshots.size()); + Mock::VerifyAndClearExpectations(syncer()); StopSyncScheduler(); } @@ -870,12 +774,8 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) { // Test nudges / polls don't run in config mode and config tasks do. TEST_F(SyncSchedulerTest, ConfigurationMode) { TimeDelta poll(TimeDelta::FromMilliseconds(15)); - SyncShareRecords records; + SyncShareTimes times; scheduler()->OnReceivedLongPollIntervalUpdate(poll); - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), - WithArg<1>(RecordSyncShare(&records)))) - .RetiresOnSaturation(); StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE); @@ -885,6 +785,10 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) { const ModelTypeSet config_types(BOOKMARKS); + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConfigureSuccess), + RecordSyncShare(×))) + .RetiresOnSaturation(); CallbackCounter counter; ConfigurationParams params( GetUpdatesCallerInfo::RECONFIGURATION, @@ -893,32 +797,20 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) { base::Bind(&CallbackCounter::Callback, base::Unretained(&counter))); ASSERT_TRUE(scheduler()->ScheduleConfiguration(params)); ASSERT_EQ(1, counter.times_called()); - - ASSERT_EQ(1U, records.snapshots.size()); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - config_types, - records.snapshots[0].source().types)); + Mock::VerifyAndClearExpectations(syncer()); // Switch to NORMAL_MODE to ensure NUDGES were properly saved and run. - // SyncSchedulerWhiteboxTest also provides coverage for this, but much - // more targeted ('whitebox' style). scheduler()->OnReceivedLongPollIntervalUpdate(TimeDelta::FromDays(1)); - SyncShareRecords records2; + SyncShareTimes times2; EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShare(&records2)))); + RecordSyncShare(×2))); // TODO(tim): Figure out how to remove this dangerous need to reset // routing info between mode switches. context()->set_routing_info(routing_info()); StartSyncScheduler(SyncScheduler::NORMAL_MODE); - ASSERT_EQ(1U, records2.snapshots.size()); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - records2.snapshots[0].source().updates_source); - EXPECT_TRUE(ModelTypeSetMatchesInvalidationMap( - nudge_types, - records2.snapshots[0].source().types)); PumpLoop(); } @@ -985,7 +877,7 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailDownloadTwice) { // Have the syncer fail to get the encryption key yet succeed in downloading // updates. Expect this will leave the scheduler in backoff. TEST_F(BackoffTriggersSyncSchedulerTest, FailGetEncryptionKey) { - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) .WillOnce(DoAll( Invoke(sessions::test_util::SimulateGetEncryptionKeyFailed), Return(true))) @@ -1009,15 +901,15 @@ TEST_F(BackoffTriggersSyncSchedulerTest, FailGetEncryptionKey) { // Test that no polls or extraneous nudges occur when in backoff. TEST_F(SyncSchedulerTest, BackoffDropsJobs) { - SyncShareRecords r; + SyncShareTimes times; TimeDelta poll(TimeDelta::FromMilliseconds(5)); const ModelTypeSet types(BOOKMARKS); scheduler()->OnReceivedLongPollIntervalUpdate(poll); UseMockDelayProvider(); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) - .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed), - WithArg<2>(RecordSyncShareMultiple(&r, 1U)))); + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed), + RecordSyncShareMultiple(×, 1U))); EXPECT_CALL(*delay(), GetDelay(_)). WillRepeatedly(Return(TimeDelta::FromDays(1))); @@ -1028,10 +920,8 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) { scheduler()->ScheduleLocalNudge(zero(), types, FROM_HERE); RunLoop(); + // From this point forward, no SyncShare functions should be invoked. Mock::VerifyAndClearExpectations(syncer()); - ASSERT_EQ(1U, r.snapshots.size()); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - r.snapshots[0].source().updates_source); // Wait a while (10x poll interval) so a few poll jobs will be attempted. PumpLoopFor(poll * 10); @@ -1045,8 +935,6 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) { Mock::VerifyAndClearExpectations(syncer()); Mock::VerifyAndClearExpectations(delay()); - ASSERT_EQ(1U, r.snapshots.size()); - EXPECT_CALL(*delay(), GetDelay(_)).Times(0); StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE); @@ -1063,12 +951,12 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) { // Test that backoff is shaping traffic properly with consecutive errors. TEST_F(SyncSchedulerTest, BackoffElevation) { - SyncShareRecords r; + SyncShareTimes times; UseMockDelayProvider(); EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)).Times(kMinNumSamples) .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed), - WithArg<2>(RecordSyncShareMultiple(&r, kMinNumSamples)))); + RecordSyncShareMultiple(×, kMinNumSamples))); const TimeDelta first = TimeDelta::FromSeconds(kInitialBackoffRetrySeconds); const TimeDelta second = TimeDelta::FromMilliseconds(2); @@ -1093,80 +981,79 @@ TEST_F(SyncSchedulerTest, BackoffElevation) { scheduler()->ScheduleLocalNudge(zero(), ModelTypeSet(BOOKMARKS), FROM_HERE); RunLoop(); - ASSERT_EQ(kMinNumSamples, r.snapshots.size()); - EXPECT_GE(r.times[1] - r.times[0], second); - EXPECT_GE(r.times[2] - r.times[1], third); - EXPECT_GE(r.times[3] - r.times[2], fourth); - EXPECT_GE(r.times[4] - r.times[3], fifth); + ASSERT_EQ(kMinNumSamples, times.size()); + EXPECT_GE(times[1] - times[0], second); + EXPECT_GE(times[2] - times[1], third); + EXPECT_GE(times[3] - times[2], fourth); + EXPECT_GE(times[4] - times[3], fifth); } // Test that things go back to normal once a retry makes forward progress. TEST_F(SyncSchedulerTest, BackoffRelief) { - SyncShareRecords r; + SyncShareTimes times; const TimeDelta poll(TimeDelta::FromMilliseconds(10)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); UseMockDelayProvider(); const TimeDelta backoff = TimeDelta::FromMilliseconds(5); - - EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) - .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed), - WithArg<2>(RecordSyncShareMultiple(&r, kMinNumSamples)))) - .WillRepeatedly(DoAll( - Invoke(sessions::test_util::SimulateNormalSuccess), - WithArg<2>(RecordSyncShareMultiple(&r, kMinNumSamples)))); - EXPECT_CALL(*syncer(), PollSyncShare(_,_)) - .WillRepeatedly(DoAll( - Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShareMultiple(&r, kMinNumSamples)))); EXPECT_CALL(*delay(), GetDelay(_)).WillOnce(Return(backoff)); // Optimal start for the post-backoff poll party. TimeTicks optimal_start = TimeTicks::Now(); StartSyncScheduler(SyncScheduler::NORMAL_MODE); - // Run again to wait for polling. + // Kick off the test with a failed nudge. + EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed), + RecordSyncShare(×))); scheduler()->ScheduleLocalNudge(zero(), ModelTypeSet(BOOKMARKS), FROM_HERE); RunLoop(); - - StopSyncScheduler(); - - EXPECT_EQ(kMinNumSamples, r.times.size()); - - // The first nudge ran as soon as possible. It failed. + Mock::VerifyAndClearExpectations(syncer()); TimeTicks optimal_job_time = optimal_start; - EXPECT_GE(r.times[0], optimal_job_time); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - r.snapshots[0].source().updates_source); + ASSERT_EQ(1U, times.size()); + EXPECT_GE(times[0], optimal_job_time); - // It was followed by a successful retry nudge shortly afterward. + // The retry succeeds. + EXPECT_CALL(*syncer(), NormalSyncShare(_,_,_)) + .WillOnce(DoAll( + Invoke(sessions::test_util::SimulateNormalSuccess), + RecordSyncShare(×))); + RunLoop(); + Mock::VerifyAndClearExpectations(syncer()); optimal_job_time = optimal_job_time + backoff; - EXPECT_GE(r.times[1], optimal_job_time); - EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, - r.snapshots[1].source().updates_source); - // After that, we went back to polling. - for (size_t i = 2; i < r.snapshots.size(); i++) { + ASSERT_EQ(2U, times.size()); + EXPECT_GE(times[1], optimal_job_time); + + // Now let the Poll timer do its thing. + EXPECT_CALL(*syncer(), PollSyncShare(_,_)) + .WillRepeatedly(DoAll( + Invoke(sessions::test_util::SimulatePollSuccess), + RecordSyncShareMultiple(×, kMinNumSamples))); + RunLoop(); + Mock::VerifyAndClearExpectations(syncer()); + ASSERT_EQ(kMinNumSamples, times.size()); + for (size_t i = 2; i < times.size(); i++) { optimal_job_time = optimal_job_time + poll; SCOPED_TRACE(testing::Message() << "SyncShare # (" << i << ")"); - EXPECT_GE(r.times[i], optimal_job_time); - EXPECT_EQ(GetUpdatesCallerInfo::PERIODIC, - r.snapshots[i].source().updates_source); + EXPECT_GE(times[i], optimal_job_time); } + + StopSyncScheduler(); } // Test that poll failures are ignored. They should have no effect on // subsequent poll attempts, nor should they trigger a backoff/retry. TEST_F(SyncSchedulerTest, TransientPollFailure) { - SyncShareRecords r; + SyncShareTimes times; const TimeDelta poll_interval(TimeDelta::FromMilliseconds(1)); scheduler()->OnReceivedLongPollIntervalUpdate(poll_interval); UseMockDelayProvider(); // Will cause test failure if backoff is initiated. EXPECT_CALL(*syncer(), PollSyncShare(_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollFailed), - WithArg<1>(RecordSyncShare(&r)))) + RecordSyncShare(×))) .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShare(&r)))); + RecordSyncShare(×))); StartSyncScheduler(SyncScheduler::NORMAL_MODE); @@ -1264,7 +1151,7 @@ TEST_F(SyncSchedulerTest, ConnectionChangeCanaryPreemptedByNudge) { // Tests that we don't crash trying to run two canaries at once if we receive // extra connection status change notifications. See crbug.com/190085. TEST_F(SyncSchedulerTest, DoubleCanaryInConfigure) { - EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_)) + EXPECT_CALL(*syncer(), ConfigureSyncShare(_,_,_)) .WillRepeatedly(DoAll( Invoke(sessions::test_util::SimulateConfigureConnectionFailure), Return(true))); @@ -1288,14 +1175,14 @@ TEST_F(SyncSchedulerTest, DoubleCanaryInConfigure) { } TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) { - SyncShareRecords records; + SyncShareTimes times; TimeDelta poll(TimeDelta::FromMilliseconds(15)); scheduler()->OnReceivedLongPollIntervalUpdate(poll); ::testing::InSequence seq; EXPECT_CALL(*syncer(), PollSyncShare(_,_)) .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShareMultiple(&records, kMinNumSamples)))); + RecordSyncShareMultiple(×, kMinNumSamples))); connection()->SetServerStatus(HttpResponse::SYNC_AUTH_ERROR); StartSyncScheduler(SyncScheduler::NORMAL_MODE); @@ -1308,7 +1195,7 @@ TEST_F(SyncSchedulerTest, PollFromCanaryAfterAuthError) { // poll once more EXPECT_CALL(*syncer(), PollSyncShare(_,_)) .WillOnce(DoAll(Invoke(sessions::test_util::SimulatePollSuccess), - WithArg<1>(RecordSyncShare(&records)))); + RecordSyncShare(×))); scheduler()->OnCredentialsUpdated(); connection()->SetServerStatus(HttpResponse::SERVER_CONNECTION_OK); StopSyncScheduler(); diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc index 8005459..338c287 100644 --- a/sync/engine/syncer.cc +++ b/sync/engine/syncer.cc @@ -73,7 +73,7 @@ bool Syncer::NormalSyncShare(ModelTypeSet request_types, kCreateMobileBookmarksFolder, request_types, base::ConstRef(nudge_tracker)))) { - return HandleCycleEnd(session); + return HandleCycleEnd(session, nudge_tracker.updates_source()); } } @@ -81,11 +81,13 @@ bool Syncer::NormalSyncShare(ModelTypeSet request_types, SyncerError commit_result = BuildAndPostCommits(request_types, this, session); session->mutable_status_controller()->set_commit_result(commit_result); - return HandleCycleEnd(session); + return HandleCycleEnd(session, nudge_tracker.updates_source()); } -bool Syncer::ConfigureSyncShare(ModelTypeSet request_types, - SyncSession* session) { +bool Syncer::ConfigureSyncShare( + ModelTypeSet request_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + SyncSession* session) { HandleCycleBegin(session); VLOG(1) << "Configuring types " << ModelTypeSetToString(request_types); DownloadAndApplyUpdates( @@ -93,9 +95,9 @@ bool Syncer::ConfigureSyncShare(ModelTypeSet request_types, base::Bind(&DownloadUpdatesForConfigure, session, kCreateMobileBookmarksFolder, - session->source(), + source, request_types)); - return HandleCycleEnd(session); + return HandleCycleEnd(session, source); } bool Syncer::PollSyncShare(ModelTypeSet request_types, @@ -108,7 +110,7 @@ bool Syncer::PollSyncShare(ModelTypeSet request_types, session, kCreateMobileBookmarksFolder, request_types)); - return HandleCycleEnd(session); + return HandleCycleEnd(session, sync_pb::GetUpdatesCallerInfo::PERIODIC); } void Syncer::ApplyUpdates(SyncSession* session) { @@ -149,9 +151,11 @@ void Syncer::HandleCycleBegin(SyncSession* session) { session->SendEventNotification(SyncEngineEvent::SYNC_CYCLE_BEGIN); } -bool Syncer::HandleCycleEnd(SyncSession* session) { +bool Syncer::HandleCycleEnd( + SyncSession* session, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source) { if (!ExitRequested()) { - session->SendEventNotification(SyncEngineEvent::SYNC_CYCLE_ENDED); + session->SendSyncCycleEndEventNotification(source); return true; } else { return false; diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h index eae6d87..8b5fc06 100644 --- a/sync/engine/syncer.h +++ b/sync/engine/syncer.h @@ -43,11 +43,28 @@ class SYNC_EXPORT_PRIVATE Syncer { bool ExitRequested(); void RequestEarlyExit(); + // Fetches and applies updates, resolves conflicts and commits local changes + // for |request_types| as necessary until client and server states are in + // sync. The |nudge_tracker| contains state that describes why the client is + // out of sync and what must be done to bring it back into sync. virtual bool NormalSyncShare(ModelTypeSet request_types, const sessions::NudgeTracker& nudge_tracker, sessions::SyncSession* session); - virtual bool ConfigureSyncShare(ModelTypeSet request_types, - sessions::SyncSession* session); + + // Performs an initial download for the |request_types|. It is assumed that + // the specified types have no local state, and that their associated change + // processors are in "passive" mode, so none of the downloaded updates will be + // applied to the model. The |source| is sent up to the server for debug + // purposes. It describes the reson for performing this initial download. + virtual bool ConfigureSyncShare( + ModelTypeSet request_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session); + + // Requests to download updates for the |request_types|. For a well-behaved + // client with a working connection to the invalidations server, this should + // be unnecessary. It may be invoked periodically to try to keep the client + // in sync despite bugs or transient failures. virtual bool PollSyncShare(ModelTypeSet request_types, sessions::SyncSession* session); @@ -58,7 +75,9 @@ class SYNC_EXPORT_PRIVATE Syncer { base::Callback<SyncerError(void)> download_fn); void HandleCycleBegin(sessions::SyncSession* session); - bool HandleCycleEnd(sessions::SyncSession* session); + bool HandleCycleEnd( + sessions::SyncSession* session, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source); bool early_exit_requested_; base::Lock early_exit_requested_lock_; diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index db7d1a1..96b664f 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -182,14 +182,7 @@ class SyncerTest : public testing::Test, } void SyncShareNudge() { - ModelSafeRoutingInfo info; - GetModelSafeRoutingInfo(&info); - ModelTypeInvalidationMap invalidation_map = - ModelSafeRoutingInfoToInvalidationMap(info, std::string()); - sessions::SyncSourceInfo source_info( - sync_pb::GetUpdatesCallerInfo::LOCAL, - invalidation_map); - session_.reset(SyncSession::Build(context_.get(), this, source_info)); + session_.reset(SyncSession::Build(context_.get(), this)); // Pretend we've seen a local change, to make the nudge_tracker look normal. nudge_tracker_.RecordLocalChange(ModelTypeSet(BOOKMARKS)); @@ -202,18 +195,10 @@ class SyncerTest : public testing::Test, } void SyncShareConfigure() { - ModelSafeRoutingInfo info; - GetModelSafeRoutingInfo(&info); - ModelTypeInvalidationMap invalidation_map = - ModelSafeRoutingInfoToInvalidationMap(info, std::string()); - sessions::SyncSourceInfo source_info( - sync_pb::GetUpdatesCallerInfo::RECONFIGURATION, - invalidation_map); - session_.reset(SyncSession::Build(context_.get(), - this, - source_info)); + session_.reset(SyncSession::Build(context_.get(), this)); EXPECT_TRUE(syncer_->ConfigureSyncShare( GetRoutingInfoTypes(context_->routing_info()), + sync_pb::GetUpdatesCallerInfo::RECONFIGURATION, session_.get())); } diff --git a/sync/internal_api/debug_info_event_listener.cc b/sync/internal_api/debug_info_event_listener.cc index d0c9be0..66953d4 100644 --- a/sync/internal_api/debug_info_event_listener.cc +++ b/sync/internal_api/debug_info_event_listener.cc @@ -39,7 +39,7 @@ void DebugInfoEventListener::OnSyncCycleCompleted( sync_completed_event_info->set_num_reflected_updates_downloaded( snapshot.model_neutral_state().num_reflected_updates_downloaded_total); sync_completed_event_info->mutable_caller_info()->set_source( - snapshot.source().updates_source); + snapshot.legacy_updates_source()); sync_completed_event_info->mutable_caller_info()->set_notifications_enabled( snapshot.notifications_enabled()); diff --git a/sync/internal_api/js_sync_manager_observer_unittest.cc b/sync/internal_api/js_sync_manager_observer_unittest.cc index 2b58fd7..65cb77e 100644 --- a/sync/internal_api/js_sync_manager_observer_unittest.cc +++ b/sync/internal_api/js_sync_manager_observer_unittest.cc @@ -82,12 +82,12 @@ TEST_F(JsSyncManagerObserverTest, OnSyncCycleCompleted) { 5, 2, 7, - sessions::SyncSourceInfo(), false, 0, base::Time::Now(), std::vector<int>(MODEL_TYPE_COUNT, 0), - std::vector<int>(MODEL_TYPE_COUNT, 0)); + std::vector<int>(MODEL_TYPE_COUNT, 0), + sync_pb::GetUpdatesCallerInfo::UNKNOWN); base::DictionaryValue expected_details; expected_details.Set("snapshot", snapshot.ToValue()); diff --git a/sync/internal_api/public/sessions/model_neutral_state.h b/sync/internal_api/public/sessions/model_neutral_state.h index e61e995..4abf158 100644 --- a/sync/internal_api/public/sessions/model_neutral_state.h +++ b/sync/internal_api/public/sessions/model_neutral_state.h @@ -26,6 +26,9 @@ struct SYNC_EXPORT ModelNeutralState { // requested_update_types stores the set of types which were requested. ModelTypeSet updates_request_types; + // The set of types for which commits were sent to the server. + ModelTypeSet commit_request_types; + sync_pb::ClientToServerResponse updates_response; int num_successful_commits; diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.cc b/sync/internal_api/public/sessions/sync_session_snapshot.cc index 03da223..97bc094 100644 --- a/sync/internal_api/public/sessions/sync_session_snapshot.cc +++ b/sync/internal_api/public/sessions/sync_session_snapshot.cc @@ -7,6 +7,7 @@ #include "base/json/json_writer.h" #include "base/memory/scoped_ptr.h" #include "base/values.h" +#include "sync/protocol/proto_enum_conversions.h" namespace syncer { namespace sessions { @@ -30,24 +31,24 @@ SyncSessionSnapshot::SyncSessionSnapshot( int num_encryption_conflicts, int num_hierarchy_conflicts, int num_server_conflicts, - const SyncSourceInfo& source, bool notifications_enabled, size_t num_entries, base::Time sync_start_time, const std::vector<int>& num_entries_by_type, - const std::vector<int>& num_to_delete_entries_by_type) + const std::vector<int>& num_to_delete_entries_by_type, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source) : model_neutral_state_(model_neutral_state), download_progress_markers_(download_progress_markers), is_silenced_(is_silenced), num_encryption_conflicts_(num_encryption_conflicts), num_hierarchy_conflicts_(num_hierarchy_conflicts), num_server_conflicts_(num_server_conflicts), - source_(source), notifications_enabled_(notifications_enabled), num_entries_(num_entries), sync_start_time_(sync_start_time), num_entries_by_type_(num_entries_by_type), num_to_delete_entries_by_type_(num_to_delete_entries_by_type), + legacy_updates_source_(legacy_updates_source), is_initialized_(true) { } @@ -83,7 +84,8 @@ base::DictionaryValue* SyncSessionSnapshot::ToValue() const { value->SetInteger("numServerConflicts", num_server_conflicts_); value->SetInteger("numEntries", num_entries_); - value->Set("source", source_.ToValue()); + value->SetString("legacySource", + GetUpdatesSourceString(legacy_updates_source_)); value->SetBoolean("notificationsEnabled", notifications_enabled_); scoped_ptr<base::DictionaryValue> counter_entries( @@ -135,10 +137,6 @@ int SyncSessionSnapshot::num_server_conflicts() const { return num_server_conflicts_; } -SyncSourceInfo SyncSessionSnapshot::source() const { - return source_; -} - bool SyncSessionSnapshot::notifications_enabled() const { return notifications_enabled_; } @@ -164,5 +162,10 @@ SyncSessionSnapshot::num_to_delete_entries_by_type() const { return num_to_delete_entries_by_type_; } +sync_pb::GetUpdatesCallerInfo::GetUpdatesSource +SyncSessionSnapshot::legacy_updates_source() const { + return legacy_updates_source_; +} + } // namespace sessions } // namespace syncer diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.h b/sync/internal_api/public/sessions/sync_session_snapshot.h index e1053ad..97a97aa 100644 --- a/sync/internal_api/public/sessions/sync_session_snapshot.h +++ b/sync/internal_api/public/sessions/sync_session_snapshot.h @@ -13,7 +13,6 @@ #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/base/progress_marker_map.h" #include "sync/internal_api/public/sessions/model_neutral_state.h" -#include "sync/internal_api/public/sessions/sync_source_info.h" namespace base { class DictionaryValue; @@ -37,12 +36,12 @@ class SYNC_EXPORT SyncSessionSnapshot { int num_encryption_conflicts, int num_hierarchy_conflicts, int num_server_conflicts, - const SyncSourceInfo& source, bool notifications_enabled, size_t num_entries, base::Time sync_start_time, const std::vector<int>& num_entries_by_type, - const std::vector<int>& num_to_delete_entries_by_type); + const std::vector<int>& num_to_delete_entries_by_type, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source); ~SyncSessionSnapshot(); // Caller takes ownership of the returned dictionary. @@ -59,12 +58,12 @@ class SYNC_EXPORT SyncSessionSnapshot { int num_encryption_conflicts() const; int num_hierarchy_conflicts() const; int num_server_conflicts() const; - SyncSourceInfo source() const; bool notifications_enabled() const; size_t num_entries() const; base::Time sync_start_time() const; const std::vector<int>& num_entries_by_type() const; const std::vector<int>& num_to_delete_entries_by_type() const; + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source() const; // Set iff this snapshot was not built using the default constructor. bool is_initialized() const; @@ -76,7 +75,6 @@ class SYNC_EXPORT SyncSessionSnapshot { int num_encryption_conflicts_; int num_hierarchy_conflicts_; int num_server_conflicts_; - SyncSourceInfo source_; bool notifications_enabled_; size_t num_entries_; base::Time sync_start_time_; @@ -84,6 +82,11 @@ class SYNC_EXPORT SyncSessionSnapshot { std::vector<int> num_entries_by_type_; std::vector<int> num_to_delete_entries_by_type_; + // This enum value used to be an important part of the sync protocol, but is + // now deprecated. We continue to use it in the snapshot because there is + // still some value in displaying it on the about:sync page. + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source_; + bool is_initialized_; }; diff --git a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc index 36056e3..881ab01 100644 --- a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc +++ b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc @@ -7,7 +7,6 @@ #include "base/memory/scoped_ptr.h" #include "base/test/values_test_util.h" #include "base/values.h" -#include "sync/internal_api/public/sessions/sync_source_info.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { @@ -44,21 +43,18 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) { const int kNumHierarchyConflicts = 1055; const int kNumServerConflicts = 1057; - SyncSourceInfo source; - scoped_ptr<base::DictionaryValue> expected_source_value(source.ToValue()); - SyncSessionSnapshot snapshot(model_neutral, download_progress_markers, kIsSilenced, kNumEncryptionConflicts, kNumHierarchyConflicts, kNumServerConflicts, - source, false, 0, base::Time::Now(), std::vector<int>(MODEL_TYPE_COUNT,0), - std::vector<int>(MODEL_TYPE_COUNT, 0)); + std::vector<int>(MODEL_TYPE_COUNT, 0), + sync_pb::GetUpdatesCallerInfo::UNKNOWN); scoped_ptr<base::DictionaryValue> value(snapshot.ToValue()); EXPECT_EQ(17u, value->size()); ExpectDictIntegerValue(model_neutral.num_successful_commits, @@ -86,7 +82,6 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) { "numHierarchyConflicts"); ExpectDictIntegerValue(kNumServerConflicts, *value, "numServerConflicts"); - ExpectDictDictionaryValue(*expected_source_value, *value, "source"); ExpectDictBooleanValue(false, *value, "notificationsEnabled"); } diff --git a/sync/internal_api/public/sessions/sync_source_info.cc b/sync/internal_api/public/sessions/sync_source_info.cc deleted file mode 100644 index 5c05de6..0000000 --- a/sync/internal_api/public/sessions/sync_source_info.cc +++ /dev/null @@ -1,35 +0,0 @@ -// 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/public/sessions/sync_source_info.h" - -#include "base/values.h" -#include "sync/protocol/proto_enum_conversions.h" - -namespace syncer { -namespace sessions { - -SyncSourceInfo::SyncSourceInfo() - : updates_source(sync_pb::GetUpdatesCallerInfo::UNKNOWN) {} - -SyncSourceInfo::SyncSourceInfo(const ModelTypeInvalidationMap& t) - : updates_source(sync_pb::GetUpdatesCallerInfo::UNKNOWN), types(t) {} - -SyncSourceInfo::SyncSourceInfo( - const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource& u, - const ModelTypeInvalidationMap& t) - : updates_source(u), types(t) {} - -SyncSourceInfo::~SyncSourceInfo() {} - -base::DictionaryValue* SyncSourceInfo::ToValue() const { - base::DictionaryValue* value = new base::DictionaryValue(); - value->SetString("updatesSource", - GetUpdatesSourceString(updates_source)); - value->Set("types", ModelTypeInvalidationMapToValue(types)); - return value; -} - -} // namespace sessions -} // namespace syncer diff --git a/sync/internal_api/public/sessions/sync_source_info.h b/sync/internal_api/public/sessions/sync_source_info.h deleted file mode 100644 index f41727b..0000000 --- a/sync/internal_api/public/sessions/sync_source_info.h +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 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. - -#ifndef SYNC_INTERNAL_API_PUBLIC_SESSIONS_SYNC_SOURCE_INFO_H_ -#define SYNC_INTERNAL_API_PUBLIC_SESSIONS_SYNC_SOURCE_INFO_H_ - -#include "base/basictypes.h" -#include "sync/base/sync_export.h" -#include "sync/internal_api/public/base/model_type.h" -#include "sync/internal_api/public/base/model_type_invalidation_map.h" -#include "sync/protocol/sync.pb.h" - -namespace base { -class DictionaryValue; -} - -namespace syncer { -namespace sessions { - -// A container for the source of a sync session. This includes the update -// source, the datatypes triggering the sync session, and possible session -// specific payloads which should be sent to the server. -struct SYNC_EXPORT SyncSourceInfo { - SyncSourceInfo(); - explicit SyncSourceInfo(const ModelTypeInvalidationMap& t); - SyncSourceInfo( - const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource& u, - const ModelTypeInvalidationMap& t); - ~SyncSourceInfo(); - - // Caller takes ownership of the returned dictionary. - base::DictionaryValue* ToValue() const; - - sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source; - ModelTypeInvalidationMap types; -}; - -} // namespace sessions -} // namespace syncer - -#endif // SYNC_INTERNAL_API_PUBLIC_SESSIONS_SYNC_SOURCE_INFO_H_ diff --git a/sync/internal_api/public/sessions/sync_source_info_unittest.cc b/sync/internal_api/public/sessions/sync_source_info_unittest.cc deleted file mode 100644 index cbdecf17..0000000 --- a/sync/internal_api/public/sessions/sync_source_info_unittest.cc +++ /dev/null @@ -1,42 +0,0 @@ -// 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/public/sessions/sync_source_info.h" - -#include "base/test/values_test_util.h" -#include "base/values.h" -#include "sync/internal_api/public/base/model_type.h" -#include "sync/internal_api/public/base/model_type_invalidation_map.h" -#include "sync/protocol/sync.pb.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace syncer { -namespace sessions { -namespace { - -using base::ExpectDictDictionaryValue; -using base::ExpectDictStringValue; - -class SyncSourceInfoTest : public testing::Test {}; - -TEST_F(SyncSourceInfoTest, SyncSourceInfoToValue) { - sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source = - sync_pb::GetUpdatesCallerInfo::PERIODIC; - ModelTypeInvalidationMap types; - types[PREFERENCES].payload = "preferencespayload"; - types[EXTENSIONS].payload = ""; - scoped_ptr<base::DictionaryValue> expected_types_value( - ModelTypeInvalidationMapToValue(types)); - - SyncSourceInfo source_info(updates_source, types); - - scoped_ptr<base::DictionaryValue> value(source_info.ToValue()); - EXPECT_EQ(2u, value->size()); - ExpectDictStringValue("PERIODIC", *value, "updatesSource"); - ExpectDictDictionaryValue(*expected_types_value, *value, "types"); -} - -} // namespace syncer -} // namespace sessions -} // namespace diff --git a/sync/sessions/nudge_tracker.cc b/sync/sessions/nudge_tracker.cc index f9b9038..9587f39 100644 --- a/sync/sessions/nudge_tracker.cc +++ b/sync/sessions/nudge_tracker.cc @@ -6,7 +6,6 @@ #include "base/basictypes.h" #include "sync/internal_api/public/base/invalidation.h" -#include "sync/internal_api/public/sessions/sync_source_info.h" #include "sync/protocol/sync.pb.h" namespace syncer { @@ -176,35 +175,6 @@ ModelTypeSet NudgeTracker::GetThrottledTypes() const { return result; } -// This function is intended to mimic the behavior of older clients. Newer -// clients and servers will not rely on SyncSourceInfo. See FillProtoMessage -// for the more modern equivalent. -SyncSourceInfo NudgeTracker::GetSourceInfo() const { - ModelTypeInvalidationMap invalidation_map; - for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); - it != type_trackers_.end(); ++it) { - if (it->second.IsThrottled()) { - // We pretend throttled types are not enabled by skipping them. - continue; - } else if (it->second.HasPendingInvalidation()) { - // The old-style source info can contain only one hint per type. We grab - // the most recent, to mimic the old coalescing behaviour. - Invalidation invalidation; - invalidation.payload = it->second.GetMostRecentInvalidationPayload(); - invalidation_map.insert(std::make_pair(it->first, invalidation)); - } else if (it->second.HasLocalChangePending()) { - // The old-style source info sent up an empty string (as opposed to - // nothing at all) when the type was locally nudged, but had not received - // any invalidations. - Invalidation invalidation; - invalidation.payload = ""; - invalidation_map.insert(std::make_pair(it->first, invalidation)); - } - } - - return SyncSourceInfo(updates_source_, invalidation_map); -} - void NudgeTracker::SetLegacyNotificationHint( ModelType type, sync_pb::DataTypeProgressMarker* progress) const { diff --git a/sync/sessions/nudge_tracker.h b/sync/sessions/nudge_tracker.h index f8109a4..8d8839e 100644 --- a/sync/sessions/nudge_tracker.h +++ b/sync/sessions/nudge_tracker.h @@ -20,8 +20,6 @@ namespace syncer { namespace sessions { -struct SyncSourceInfo; - class SYNC_EXPORT_PRIVATE NudgeTracker { public: static size_t kDefaultMaxPayloadsPerType; @@ -78,10 +76,6 @@ class SYNC_EXPORT_PRIVATE NudgeTracker { // Returns the set of currently throttled types. ModelTypeSet GetThrottledTypes() const; - // A helper to return an old-style source info. Used only to maintain - // compatibility with some old code. - SyncSourceInfo GetSourceInfo() const; - // Returns the 'source' of the GetUpdate request. // // This flag is deprecated, but still used by the server. There can be more diff --git a/sync/sessions/nudge_tracker_unittest.cc b/sync/sessions/nudge_tracker_unittest.cc index 598a3e6b..d3efc02 100644 --- a/sync/sessions/nudge_tracker_unittest.cc +++ b/sync/sessions/nudge_tracker_unittest.cc @@ -5,7 +5,6 @@ #include "sync/sessions/nudge_tracker.h" #include "sync/internal_api/public/base/model_type_invalidation_map.h" -#include "sync/internal_api/public/sessions/sync_source_info.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { @@ -68,9 +67,8 @@ TEST_F(NudgeTrackerTest, EmptyNudgeTracker) { sync_pb::GetUpdateTriggers gu_trigger; nudge_tracker.FillProtoMessage(BOOKMARKS, &gu_trigger); - SyncSourceInfo source_info = nudge_tracker.GetSourceInfo(); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::UNKNOWN, - source_info.updates_source); + nudge_tracker.updates_source()); } // Verify that nudges override each other based on a priority order. diff --git a/sync/sessions/ordered_commit_set.cc b/sync/sessions/ordered_commit_set.cc index f2f3755..842a0e9 100644 --- a/sync/sessions/ordered_commit_set.cc +++ b/sync/sessions/ordered_commit_set.cc @@ -27,6 +27,7 @@ void OrderedCommitSet::AddCommitItem(const int64 metahandle, projections_[GetGroupForModelType(type, routes_)].push_back( commit_ids_.size() - 1); types_.push_back(type); + types_in_list_.Put(type); } } @@ -86,6 +87,7 @@ void OrderedCommitSet::Clear() { it->second.clear(); } types_.clear(); + types_in_list_.Clear(); } OrderedCommitSet::CommitItem OrderedCommitSet::GetCommitItemAt( diff --git a/sync/sessions/ordered_commit_set.h b/sync/sessions/ordered_commit_set.h index 862ee23..ae99dcd 100644 --- a/sync/sessions/ordered_commit_set.h +++ b/sync/sessions/ordered_commit_set.h @@ -75,6 +75,11 @@ class SYNC_EXPORT_PRIVATE OrderedCommitSet { return Size() == 0; } + // Returns all the types that are included in this list. + ModelTypeSet Types() const { + return types_in_list_; + } + // Returns true iff any of the commit ids added to this set have model type // BOOKMARKS. bool HasBookmarkCommitId() const; @@ -115,6 +120,9 @@ class SYNC_EXPORT_PRIVATE OrderedCommitSet { // and shouldn't take up too much extra space since commit lists are small. std::vector<ModelType> types_; + // The set of types which are included in this particular list. + ModelTypeSet types_in_list_; + ModelSafeRoutingInfo routes_; }; diff --git a/sync/sessions/ordered_commit_set_unittest.cc b/sync/sessions/ordered_commit_set_unittest.cc index afb76b7..4bf1d61 100644 --- a/sync/sessions/ordered_commit_set_unittest.cc +++ b/sync/sessions/ordered_commit_set_unittest.cc @@ -18,7 +18,7 @@ class OrderedCommitSetTest : public testing::Test { routes_[BOOKMARKS] = GROUP_UI; routes_[PREFERENCES] = GROUP_UI; routes_[AUTOFILL] = GROUP_DB; - routes_[TOP_LEVEL_FOLDER] = GROUP_PASSIVE; + routes_[SESSIONS] = GROUP_PASSIVE; } protected: TestIdFactory ids_; @@ -36,8 +36,8 @@ TEST_F(OrderedCommitSetTest, Projections) { commit_set1.AddCommitItem(2, expected[2], PREFERENCES); // Duplicates should be dropped. commit_set1.AddCommitItem(2, expected[2], PREFERENCES); - commit_set1.AddCommitItem(3, expected[3], TOP_LEVEL_FOLDER); - commit_set1.AddCommitItem(4, expected[4], TOP_LEVEL_FOLDER); + commit_set1.AddCommitItem(3, expected[3], SESSIONS); + commit_set1.AddCommitItem(4, expected[4], SESSIONS); commit_set2.AddCommitItem(7, expected[7], AUTOFILL); commit_set2.AddCommitItem(6, expected[6], AUTOFILL); commit_set2.AddCommitItem(5, expected[5], AUTOFILL); @@ -102,7 +102,7 @@ TEST_F(OrderedCommitSetTest, HasBookmarkCommitId) { OrderedCommitSet commit_set(routes_); commit_set.AddCommitItem(0, ids_.NewLocalId(), AUTOFILL); - commit_set.AddCommitItem(1, ids_.NewLocalId(), TOP_LEVEL_FOLDER); + commit_set.AddCommitItem(1, ids_.NewLocalId(), SESSIONS); EXPECT_FALSE(commit_set.HasBookmarkCommitId()); commit_set.AddCommitItem(2, ids_.NewLocalId(), PREFERENCES); diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h index ec70f2c..a547c1b 100644 --- a/sync/sessions/status_controller.h +++ b/sync/sessions/status_controller.h @@ -53,6 +53,12 @@ class SYNC_EXPORT_PRIVATE StatusController { void set_updates_request_types(ModelTypeSet value) { model_neutral_.updates_request_types = value; } + const ModelTypeSet commit_request_types() const { + return model_neutral_.commit_request_types; + } + void set_commit_request_types(ModelTypeSet value) { + model_neutral_.commit_request_types = value; + } const sync_pb::ClientToServerResponse& updates_response() const { return model_neutral_.updates_response; } diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc index eb81733..030ef0f 100644 --- a/sync/sessions/sync_session.cc +++ b/sync/sessions/sync_session.cc @@ -17,17 +17,14 @@ namespace sessions { // static SyncSession* SyncSession::Build(SyncSessionContext* context, - Delegate* delegate, - const SyncSourceInfo& source) { - return new SyncSession(context, delegate, source); + Delegate* delegate) { + return new SyncSession(context, delegate); } SyncSession::SyncSession( SyncSessionContext* context, - Delegate* delegate, - const SyncSourceInfo& source) + Delegate* delegate) : context_(context), - source_(source), delegate_(delegate) { status_controller_.reset(new StatusController()); } @@ -35,6 +32,11 @@ SyncSession::SyncSession( SyncSession::~SyncSession() {} SyncSessionSnapshot SyncSession::TakeSnapshot() const { + return TakeSnapshotWithSource(sync_pb::GetUpdatesCallerInfo::UNKNOWN); +} + +SyncSessionSnapshot SyncSession::TakeSnapshotWithSource( + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source) const { syncable::Directory* dir = context_->directory(); ProgressMarkerMap download_progress_markers; @@ -55,16 +57,26 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const { status_controller_->num_encryption_conflicts(), status_controller_->num_hierarchy_conflicts(), status_controller_->num_server_conflicts(), - source_, context_->notifications_enabled(), dir->GetEntriesCount(), status_controller_->sync_start_time(), num_entries_by_type, - num_to_delete_entries_by_type); + num_to_delete_entries_by_type, + legacy_updates_source); return snapshot; } +void SyncSession::SendSyncCycleEndEventNotification( + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source) { + SyncEngineEvent event(SyncEngineEvent::SYNC_CYCLE_ENDED); + event.snapshot = TakeSnapshotWithSource(source); + + DVLOG(1) << "Sending cycle end event with snapshot: " + << event.snapshot.ToString(); + context()->NotifyListeners(event); +} + void SyncSession::SendEventNotification(SyncEngineEvent::EventCause cause) { SyncEngineEvent event(cause); event.snapshot = TakeSnapshot(); diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h index 8f4e9ed..cd4a22c 100644 --- a/sync/sessions/sync_session.h +++ b/sync/sessions/sync_session.h @@ -104,14 +104,18 @@ class SYNC_EXPORT_PRIVATE SyncSession { // Build a session without a nudge tracker. Used for poll or configure type // sync cycles. static SyncSession* Build(SyncSessionContext* context, - Delegate* delegate, - const SyncSourceInfo& source); + Delegate* delegate); ~SyncSession(); // Builds a thread-safe and read-only copy of the current session state. SyncSessionSnapshot TakeSnapshot() const; + SyncSessionSnapshot TakeSnapshotWithSource( + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource legacy_updates_source) + const; // Builds and sends a snapshot to the session context's listeners. + void SendSyncCycleEndEventNotification( + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source); void SendEventNotification(SyncEngineEvent::EventCause cause); // TODO(akalin): Split this into context() and mutable_context(). @@ -124,19 +128,12 @@ class SYNC_EXPORT_PRIVATE SyncSession { return status_controller_.get(); } - const SyncSourceInfo& source() const { return source_; } - private: - SyncSession(SyncSessionContext* context, - Delegate* delegate, - const SyncSourceInfo& source); + SyncSession(SyncSessionContext* context, Delegate* delegate); // The context for this session, guaranteed to outlive |this|. SyncSessionContext* const context_; - // The source for initiating this sync session. - SyncSourceInfo source_; - // The delegate for this session, must never be NULL. Delegate* const delegate_; diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index 979ddbd..ea64afc 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -32,9 +32,7 @@ class SyncSessionTest : public testing::Test, SyncSessionTest() : controller_invocations_allowed_(false) {} SyncSession* MakeSession() { - return SyncSession::Build(context_.get(), - this, - SyncSourceInfo()); + return SyncSession::Build(context_.get(), this); } virtual void SetUp() { diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc index 3abfa97..538ed0f 100644 --- a/sync/sessions/test_util.cc +++ b/sync/sessions/test_util.cc @@ -8,31 +8,39 @@ namespace syncer { namespace sessions { namespace test_util { -void SimulateGetEncryptionKeyFailed(ModelTypeSet requsted_types, - sessions::SyncSession* session) { +void SimulateGetEncryptionKeyFailed( + ModelTypeSet requsted_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_get_key_result( SERVER_RESPONSE_VALIDATION_FAILED); session->mutable_status_controller()->set_last_download_updates_result( SYNCER_OK); } -void SimulateConfigureSuccess(ModelTypeSet requsted_types, - sessions::SyncSession* session) { +void SimulateConfigureSuccess( + ModelTypeSet requsted_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session) { ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); session->mutable_status_controller()->set_last_download_updates_result( SYNCER_OK); } -void SimulateConfigureFailed(ModelTypeSet requsted_types, - sessions::SyncSession* session) { +void SimulateConfigureFailed( + ModelTypeSet requsted_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); session->mutable_status_controller()->set_last_download_updates_result( SERVER_RETURN_TRANSIENT_ERROR); } -void SimulateConfigureConnectionFailure(ModelTypeSet requsted_types, - sessions::SyncSession* session) { +void SimulateConfigureConnectionFailure( + ModelTypeSet requsted_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session) { session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); session->mutable_status_controller()->set_last_download_updates_result( NETWORK_CONNECTION_UNAVAILABLE); diff --git a/sync/sessions/test_util.h b/sync/sessions/test_util.h index 56c13c7..c670128 100644 --- a/sync/sessions/test_util.h +++ b/sync/sessions/test_util.h @@ -16,14 +16,22 @@ namespace sessions { namespace test_util { // Configure sync cycle successes and failures. -void SimulateGetEncryptionKeyFailed(ModelTypeSet requested_types, - sessions::SyncSession* session); -void SimulateConfigureSuccess(ModelTypeSet requested_types, - sessions::SyncSession* session); -void SimulateConfigureFailed(ModelTypeSet requested_types, - sessions::SyncSession* session); -void SimulateConfigureConnectionFailure(ModelTypeSet requested_types, - sessions::SyncSession* session); +void SimulateGetEncryptionKeyFailed( + ModelTypeSet requested_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session); +void SimulateConfigureSuccess( + ModelTypeSet requested_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session); +void SimulateConfigureFailed( + ModelTypeSet requested_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session); +void SimulateConfigureConnectionFailure( + ModelTypeSet requested_types, + sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source, + sessions::SyncSession* session); // Normal mode sync cycle successes and failures. void SimulateNormalSuccess(ModelTypeSet requested_types, diff --git a/sync/sync_internal_api.gypi b/sync/sync_internal_api.gypi index 681f3bf..704e960 100644 --- a/sync/sync_internal_api.gypi +++ b/sync/sync_internal_api.gypi @@ -73,8 +73,6 @@ 'internal_api/public/sessions/model_neutral_state.h', 'internal_api/public/sessions/sync_session_snapshot.cc', 'internal_api/public/sessions/sync_session_snapshot.h', - 'internal_api/public/sessions/sync_source_info.cc', - 'internal_api/public/sessions/sync_source_info.h', 'internal_api/public/sync_encryption_handler.cc', 'internal_api/public/sync_encryption_handler.h', 'internal_api/public/sync_manager_factory.h', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 847d862..933ad7b 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -385,7 +385,6 @@ 'internal_api/js_sync_manager_observer_unittest.cc', 'internal_api/public/change_record_unittest.cc', 'internal_api/public/sessions/sync_session_snapshot_unittest.cc', - 'internal_api/public/sessions/sync_source_info_unittest.cc', 'internal_api/syncapi_server_connection_manager_unittest.cc', 'internal_api/sync_encryption_handler_impl_unittest.cc', 'internal_api/sync_manager_impl_unittest.cc', diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h index 89c2aba..f0904cc 100644 --- a/sync/test/engine/syncer_command_test.h +++ b/sync/test/engine/syncer_command_test.h @@ -103,24 +103,8 @@ class SyncerCommandTestBase : public testing::Test, // Lazily create a session requesting all datatypes with no state. sessions::SyncSession* session() { - ModelTypeInvalidationMap types = - ModelSafeRoutingInfoToInvalidationMap(routing_info_, std::string()); - return session(sessions::SyncSourceInfo(types)); - } - - // Create a session with the provided source. - sessions::SyncSession* session(const sessions::SyncSourceInfo& source) { - // These sources require a valid nudge tracker. - DCHECK_NE(sync_pb::GetUpdatesCallerInfo::LOCAL, source.updates_source); - DCHECK_NE(sync_pb::GetUpdatesCallerInfo::NOTIFICATION, - source.updates_source); - DCHECK_NE(sync_pb::GetUpdatesCallerInfo::DATATYPE_REFRESH, - source.updates_source); - if (!session_.get()) { - std::vector<ModelSafeWorker*> workers = GetWorkers(); - session_.reset( - sessions::SyncSession::Build(context(), delegate(), source)); - } + if (!session_.get()) + session_.reset(sessions::SyncSession::Build(context(), delegate())); return session_.get(); } |