diff options
Diffstat (limited to 'sync')
39 files changed, 498 insertions, 490 deletions
diff --git a/sync/engine/all_status.cc b/sync/engine/all_status.cc index 10201f8..af3d4fc 100644 --- a/sync/engine/all_status.cc +++ b/sync/engine/all_status.cc @@ -111,6 +111,9 @@ void AllStatus::OnSyncEngineEvent(const SyncEngineEvent& event) { case SyncEngineEvent::RETRY_TIME_CHANGED: status_.retry_time = event.retry_time; break; + case SyncEngineEvent::THROTTLED_TYPES_CHANGED: + status_.throttled_types = event.throttled_types; + break; default: LOG(ERROR) << "Unrecognized Syncer Event: " << event.what_happened; break; @@ -137,11 +140,6 @@ void AllStatus::SetEncryptedTypes(ModelTypeSet types) { status_.encrypted_types = types; } -void AllStatus::SetThrottledTypes(ModelTypeSet types) { - ScopedStatusLock lock(this); - status_.throttled_types = types; -} - void AllStatus::SetCryptographerReady(bool ready) { ScopedStatusLock lock(this); status_.cryptographer_ready = ready; diff --git a/sync/engine/all_status.h b/sync/engine/all_status.h index 76ce3073..8954eae 100644 --- a/sync/engine/all_status.h +++ b/sync/engine/all_status.h @@ -49,8 +49,6 @@ class AllStatus : public SyncEngineEventListener { void IncrementNotificationsReceived(); - void SetThrottledTypes(ModelTypeSet types); - void SetEncryptedTypes(ModelTypeSet types); void SetCryptographerReady(bool ready); void SetCryptoHasPendingKeys(bool has_pending_keys); diff --git a/sync/engine/download_updates_command.cc b/sync/engine/download_updates_command.cc index 2bb2372..10f3107 100644 --- a/sync/engine/download_updates_command.cc +++ b/sync/engine/download_updates_command.cc @@ -117,6 +117,15 @@ SyncerError DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { it.Good(); it.Inc()) { if (ProxyTypes().Has(it.Get())) continue; + + if (origin == sync_pb::SyncEnums::GU_TRIGGER) { + if (session->nudge_tracker()->IsTypeThrottled(it.Get())) { + continue; // Skip throttled types. + } + } else { + DCHECK(!session->nudge_tracker()); + } + sync_pb::DataTypeProgressMarker* progress_marker = get_updates->add_from_progress_marker(); dir->GetDownloadProgress(it.Get(), progress_marker); @@ -132,8 +141,6 @@ SyncerError DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { session->nudge_tracker()->FillProtoMessage( it.Get(), progress_marker->mutable_get_update_triggers()); - } else { - DCHECK(!session->nudge_tracker()); } } diff --git a/sync/engine/get_commit_ids_command.cc b/sync/engine/get_commit_ids_command.cc index 5d47a6c..61d2f78 100644 --- a/sync/engine/get_commit_ids_command.cc +++ b/sync/engine/get_commit_ids_command.cc @@ -9,7 +9,7 @@ #include <vector> #include "sync/engine/syncer_util.h" -#include "sync/engine/throttled_data_type_tracker.h" +#include "sync/sessions/nudge_tracker.h" #include "sync/syncable/entry.h" #include "sync/syncable/nigori_handler.h" #include "sync/syncable/nigori_util.h" @@ -56,13 +56,15 @@ SyncerError GetCommitIdsCommand::ExecuteImpl(SyncSession* session) { passphrase_missing = cryptographer->has_pending_keys(); }; - const ModelTypeSet throttled_types = - session->context()->throttled_data_type_tracker()->GetThrottledTypes(); + // If we're comitting, then we must be performing a nudge job and must have a + // session with a nudge tracker. + DCHECK(session->nudge_tracker()); + // We filter out all unready entries from the set of unsynced handles. This // new set of ready and unsynced items (which excludes throttled items as // well) is then what we use to determine what is a candidate for commit. FilterUnreadyEntries(trans_, - throttled_types, + session->nudge_tracker()->GetThrottledTypes(), encrypted_types, passphrase_missing, all_unsynced_handles, diff --git a/sync/engine/sync_engine_event.h b/sync/engine/sync_engine_event.h index 2aa6b26..3328ff3 100644 --- a/sync/engine/sync_engine_event.h +++ b/sync/engine/sync_engine_event.h @@ -49,6 +49,9 @@ struct SYNC_EXPORT_PRIVATE SyncEngineEvent { // either because it gets throttled by server or because it backs off after // request failure. Retry time is passed in retry_time field of event. RETRY_TIME_CHANGED, + + // This event is sent when types are throttled or unthrottled. + THROTTLED_TYPES_CHANGED, }; explicit SyncEngineEvent(EventCause cause); @@ -62,8 +65,11 @@ struct SYNC_EXPORT_PRIVATE SyncEngineEvent { // Update-Client-Auth returns a new token for sync use. std::string updated_token; - // Time when scheduler will try to send request after backoff + // Time when scheduler will try to send request after backoff. base::Time retry_time; + + // Set of types that are currently throttled. + ModelTypeSet throttled_types; }; class SYNC_EXPORT_PRIVATE SyncEngineEventListener { diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 0875325..3921f67 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -16,7 +16,6 @@ #include "base/message_loop.h" #include "sync/engine/backoff_delay_provider.h" #include "sync/engine/syncer.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/protocol/proto_enum_conversions.h" #include "sync/protocol/sync.pb.h" #include "sync/util/data_type_histogram.h" @@ -228,8 +227,10 @@ void SyncSchedulerImpl::Start(Mode mode) { mode_ = mode; AdjustPolling(UPDATE_INTERVAL); // Will kick start poll timer if needed. - if (old_mode != mode_ && mode_ == NORMAL_MODE && - nudge_tracker_.IsSyncRequired()) { + if (old_mode != mode_ && + mode_ == NORMAL_MODE && + nudge_tracker_.IsSyncRequired() && + CanRunNudgeJobNow(NORMAL_PRIORITY)) { // We just got back to normal mode. Let's try to run the work that was // queued up while we were configuring. DoNudgeSyncSessionJob(NORMAL_PRIORITY); @@ -335,28 +336,10 @@ bool SyncSchedulerImpl::CanRunNudgeJobNow(JobPriority priority) { return false; } - // Per-datatype throttling will prevent a sync cycle if the only reason to - // perform this cycle is to commit a local change. If there is any other - // reason to perform the sync cycle (such as an invalidation for the throttled - // type or a local change to non-throttled type) then the cycle will be - // allowed to proceed. - // - // If the cycle is prevented, we will drop it without scheduling another. - // Unlike global throttling, we have no timer to tell us when to wake us up - // when a type is unthrottled. - // - // If the sync cycle is not prevented, the commit message will be filtered to - // prevent the commit of any items of the throttled data type. However, if - // the sync cycle succeeds, the nudge tracker will be told to assume that all - // outstanding commits (including the ones for throttled types) were completed - // successfully. It does not yet have the ability to selectively clear the - // uncommitted status of only some types. - ModelTypeSet throttled_types = - session_context_->throttled_data_type_tracker()->GetThrottledTypes(); - if (!nudge_tracker_.IsGetUpdatesRequired() && - throttled_types.HasAll(nudge_tracker_.GetLocallyModifiedTypes())) { - // TODO(sync): Throttled types should be pruned from the sources list. - SDVLOG(1) << "Not running a nudge because we're fully datatype throttled."; + const ModelTypeSet enabled_types = + GetRoutingInfoTypes(session_context_->routing_info()); + if (nudge_tracker_.GetThrottledTypes().HasAll(enabled_types)) { + SDVLOG(1) << "Not running a nudge because we're fully type throttled."; return false; } @@ -459,9 +442,8 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( pending_wakeup_timer_.Start( nudge_location, delay, - base::Bind(&SyncSchedulerImpl::DoNudgeSyncSessionJob, - weak_ptr_factory_.GetWeakPtr(), - NORMAL_PRIORITY)); + base::Bind(&SyncSchedulerImpl::PerformDelayedNudge, + weak_ptr_factory_.GetWeakPtr())); } const char* SyncSchedulerImpl::GetModeString(SyncScheduler::Mode mode) { @@ -474,9 +456,7 @@ const char* SyncSchedulerImpl::GetModeString(SyncScheduler::Mode mode) { void SyncSchedulerImpl::DoNudgeSyncSessionJob(JobPriority priority) { DCHECK(CalledOnValidThread()); - - if (!CanRunNudgeJobNow(priority)) - return; + DCHECK(CanRunNudgeJobNow(priority)); DVLOG(2) << "Will run normal mode sync cycle with routing info " << ModelSafeRoutingInfoToString(session_context_->routing_info()); @@ -553,7 +533,7 @@ bool SyncSchedulerImpl::DoConfigurationSyncSessionJob(JobPriority priority) { void SyncSchedulerImpl::HandleFailure( const sessions::ModelNeutralState& model_neutral_state) { - if (IsSyncingCurrentlySilenced()) { + if (IsCurrentlyThrottled()) { SDVLOG(2) << "Was throttled during previous sync cycle."; RestartWaiting(); } else if (!IsBackingOff()) { @@ -589,11 +569,11 @@ void SyncSchedulerImpl::DoPollSyncSessionJob() { << ModelSafeRoutingInfoToString(session_context_->routing_info()); scoped_ptr<SyncSession> session( SyncSession::Build(session_context_, this, info)); - syncer_->SyncShare(session.get(), SYNCER_BEGIN, SYNCER_END); + syncer_->SyncShare(session.get(), DOWNLOAD_UPDATES, APPLY_UPDATES); AdjustPolling(UPDATE_INTERVAL); - if (IsSyncingCurrentlySilenced()) { + if (IsCurrentlyThrottled()) { SDVLOG(2) << "Poll request got us throttled."; // The OnSilencedUntil() call set up the WaitInterval for us. All we need // to do is start the timer. @@ -694,7 +674,8 @@ void SyncSchedulerImpl::TryCanaryJob() { if (mode_ == CONFIGURATION_MODE && pending_configure_params_) { SDVLOG(2) << "Found pending configure job; will run as canary"; DoConfigurationSyncSessionJob(CANARY_PRIORITY); - } else if (mode_ == NORMAL_MODE && nudge_tracker_.IsSyncRequired()) { + } else if (mode_ == NORMAL_MODE && nudge_tracker_.IsSyncRequired() && + CanRunNudgeJobNow(CANARY_PRIORITY)) { SDVLOG(2) << "Found pending nudge job; will run as canary"; DoNudgeSyncSessionJob(CANARY_PRIORITY); } else { @@ -734,6 +715,40 @@ void SyncSchedulerImpl::Unthrottle() { TryCanaryJob(); } +void SyncSchedulerImpl::TypeUnthrottle(base::TimeTicks unthrottle_time) { + DCHECK(CalledOnValidThread()); + nudge_tracker_.UpdateTypeThrottlingState(unthrottle_time); + NotifyThrottledTypesChanged(nudge_tracker_.GetThrottledTypes()); + + if (nudge_tracker_.IsAnyTypeThrottled()) { + base::TimeDelta time_until_next_unthrottle = + nudge_tracker_.GetTimeUntilNextUnthrottle(unthrottle_time); + type_unthrottle_timer_.Start( + FROM_HERE, + time_until_next_unthrottle, + base::Bind(&SyncSchedulerImpl::TypeUnthrottle, + weak_ptr_factory_.GetWeakPtr(), + unthrottle_time + time_until_next_unthrottle)); + } + + // Maybe this is a good time to run a nudge job. Let's try it. + if (nudge_tracker_.IsSyncRequired() && CanRunNudgeJobNow(NORMAL_PRIORITY)) + DoNudgeSyncSessionJob(NORMAL_PRIORITY); +} + +void SyncSchedulerImpl::PerformDelayedNudge() { + // Circumstances may have changed since we scheduled this delayed nudge. + // We must check to see if it's OK to run the job before we do so. + if (CanRunNudgeJobNow(NORMAL_PRIORITY)) + DoNudgeSyncSessionJob(NORMAL_PRIORITY); + + // We're not responsible for setting up any retries here. The functions that + // first put us into a state that prevents successful sync cycles (eg. global + // throttling, type throttling, network errors, transient errors) will also + // setup the appropriate retry logic (eg. retry after timeout, exponential + // backoff, retry when the network changes). +} + void SyncSchedulerImpl::ExponentialBackoffRetry() { TryCanaryJob(); @@ -761,21 +776,43 @@ void SyncSchedulerImpl::NotifyRetryTime(base::Time retry_time) { session_context_->NotifyListeners(event); } +void SyncSchedulerImpl::NotifyThrottledTypesChanged(ModelTypeSet types) { + SyncEngineEvent event(SyncEngineEvent::THROTTLED_TYPES_CHANGED); + event.throttled_types = types; + session_context_->NotifyListeners(event); +} + bool SyncSchedulerImpl::IsBackingOff() const { DCHECK(CalledOnValidThread()); return wait_interval_.get() && wait_interval_->mode == WaitInterval::EXPONENTIAL_BACKOFF; } -void SyncSchedulerImpl::OnSilencedUntil( - const base::TimeTicks& silenced_until) { +void SyncSchedulerImpl::OnThrottled(const base::TimeDelta& throttle_duration) { DCHECK(CalledOnValidThread()); wait_interval_.reset(new WaitInterval(WaitInterval::THROTTLED, - silenced_until - TimeTicks::Now())); + throttle_duration)); NotifyRetryTime(base::Time::Now() + wait_interval_->length); } -bool SyncSchedulerImpl::IsSyncingCurrentlySilenced() { +void SyncSchedulerImpl::OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) { + base::TimeTicks now = base::TimeTicks::Now(); + + nudge_tracker_.SetTypesThrottledUntil(types, throttle_duration, now); + base::TimeDelta time_until_next_unthrottle = + nudge_tracker_.GetTimeUntilNextUnthrottle(now); + type_unthrottle_timer_.Start( + FROM_HERE, + time_until_next_unthrottle, + base::Bind(&SyncSchedulerImpl::TypeUnthrottle, + weak_ptr_factory_.GetWeakPtr(), + now + time_until_next_unthrottle)); + NotifyThrottledTypesChanged(nudge_tracker_.GetThrottledTypes()); +} + +bool SyncSchedulerImpl::IsCurrentlyThrottled() { DCHECK(CalledOnValidThread()); return wait_interval_.get() && wait_interval_->mode == WaitInterval::THROTTLED; diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h index 46f5c35..3350267 100644 --- a/sync/engine/sync_scheduler_impl.h +++ b/sync/engine/sync_scheduler_impl.h @@ -76,9 +76,11 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl virtual void OnConnectionStatusChange() OVERRIDE; // SyncSession::Delegate implementation. - virtual void OnSilencedUntil( - const base::TimeTicks& silenced_until) OVERRIDE; - virtual bool IsSyncingCurrentlySilenced() OVERRIDE; + virtual void OnThrottled(const base::TimeDelta& throttle_duration) OVERRIDE; + virtual void OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) OVERRIDE; + virtual bool IsCurrentlyThrottled() OVERRIDE; virtual void OnReceivedShortPollIntervalUpdate( const base::TimeDelta& new_interval) OVERRIDE; virtual void OnReceivedLongPollIntervalUpdate( @@ -195,9 +197,12 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl // Helper to signal all listeners registered with |session_context_|. void Notify(SyncEngineEvent::EventCause cause); - // Helper to signal listeners about changed retry time + // Helper to signal listeners about changed retry time. void NotifyRetryTime(base::Time retry_time); + // Helper to signal listeners about changed throttled types. + void NotifyThrottledTypesChanged(ModelTypeSet types); + // Looks for pending work and, if it finds any, run this work at "canary" // priority. void TryCanaryJob(); @@ -205,6 +210,12 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl // Transitions out of the THROTTLED WaitInterval then calls TryCanaryJob(). void Unthrottle(); + // Called when a per-type throttling interval expires. + void TypeUnthrottle(base::TimeTicks unthrottle_time); + + // Runs a normal nudge job when the scheduled timer expires. + void PerformDelayedNudge(); + // Attempts to exit EXPONENTIAL_BACKOFF by calling TryCanaryJob(). void ExponentialBackoffRetry(); @@ -264,6 +275,9 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl // The event that will wake us up. base::OneShotTimer<SyncSchedulerImpl> pending_wakeup_timer_; + // An event that fires when data type throttling expires. + base::OneShotTimer<SyncSchedulerImpl> type_unthrottle_timer_; + // Storage for variables related to an in-progress configure request. Note // that (mode_ != CONFIGURATION_MODE) \implies !pending_configure_params_. scoped_ptr<ConfigurationParams> pending_configure_params_; diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index add837a..33d9764 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/engine/throttled_data_type_tracker.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" @@ -142,10 +141,9 @@ class SyncSchedulerTest : public testing::Test { connection_.reset(new MockConnectionManager(directory())); connection_->SetServerReachable(); - throttled_data_type_tracker_.reset(new ThrottledDataTypeTracker(NULL)); context_.reset(new SyncSessionContext( connection_.get(), directory(), workers, - &extensions_activity_monitor_, throttled_data_type_tracker_.get(), + &extensions_activity_monitor_, std::vector<SyncEngineEventListener*>(), NULL, NULL, true, // enable keystore encryption "fake_invalidator_client_id")); @@ -221,8 +219,8 @@ class SyncSchedulerTest : public testing::Test { SyncSessionContext* context() { return context_.get(); } - ThrottledDataTypeTracker* throttled_data_type_tracker() { - return throttled_data_type_tracker_.get(); + ModelTypeSet GetThrottledTypes() { + return scheduler_->nudge_tracker_.GetThrottledTypes(); } private: @@ -240,7 +238,6 @@ class SyncSchedulerTest : public testing::Test { MockDelayProvider* delay_; std::vector<scoped_refptr<FakeModelWorker> > workers_; FakeExtensionsActivityMonitor extensions_activity_monitor_; - scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_; ModelSafeRoutingInfo routing_info_; }; @@ -736,9 +733,9 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromNudge) { scheduler()->ScheduleLocalNudge(zero(), types, FROM_HERE); PumpLoop(); - EXPECT_TRUE(scheduler()->IsSyncingCurrentlySilenced()); + EXPECT_TRUE(scheduler()->IsCurrentlyThrottled()); RunLoop(); - EXPECT_FALSE(scheduler()->IsSyncingCurrentlySilenced()); + EXPECT_FALSE(scheduler()->IsCurrentlyThrottled()); StopSyncScheduler(); } @@ -770,10 +767,10 @@ TEST_F(SyncSchedulerTest, ThrottlingExpiresFromConfigure) { base::Bind(&CallbackCounter::Callback, base::Unretained(&counter))); EXPECT_FALSE(scheduler()->ScheduleConfiguration(params)); EXPECT_EQ(0, counter.times_called()); - EXPECT_TRUE(scheduler()->IsSyncingCurrentlySilenced()); + EXPECT_TRUE(scheduler()->IsCurrentlyThrottled()); RunLoop(); - EXPECT_FALSE(scheduler()->IsSyncingCurrentlySilenced()); + EXPECT_FALSE(scheduler()->IsCurrentlyThrottled()); StopSyncScheduler(); } @@ -800,7 +797,7 @@ TEST_F(SyncSchedulerTest, TypeThrottlingBlocksNudge) { StartSyncScheduler(SyncScheduler::NORMAL_MODE); scheduler()->ScheduleLocalNudge(zero(), types, FROM_HERE); PumpLoop(); - EXPECT_TRUE(throttled_data_type_tracker()->GetThrottledTypes().HasAll(types)); + EXPECT_TRUE(GetThrottledTypes().HasAll(types)); // This won't cause a sync cycle because the types are throttled. scheduler()->ScheduleLocalNudge(zero(), types, FROM_HERE); @@ -809,7 +806,7 @@ TEST_F(SyncSchedulerTest, TypeThrottlingBlocksNudge) { StopSyncScheduler(); } -TEST_F(SyncSchedulerTest, TypeThrottlingDoesntBlockOtherSources) { +TEST_F(SyncSchedulerTest, TypeThrottlingDoesBlockOtherSources) { UseMockDelayProvider(); EXPECT_CALL(*delay(), GetDelay(_)) .WillRepeatedly(Return(zero())); @@ -838,25 +835,24 @@ TEST_F(SyncSchedulerTest, TypeThrottlingDoesntBlockOtherSources) { scheduler()->ScheduleLocalNudge(zero(), throttled_types, FROM_HERE); PumpLoop(); EXPECT_EQ(0U, records.snapshots.size()); - EXPECT_TRUE(throttled_data_type_tracker()->GetThrottledTypes().HasAll( - throttled_types)); + EXPECT_TRUE(GetThrottledTypes().HasAll(throttled_types)); - // This invalidaiton will cause a sync even though the types are throttled. + // Ignore invalidations for throttled types. ModelTypeInvalidationMap invalidation_map = ModelTypeSetToInvalidationMap(throttled_types, "test"); scheduler()->ScheduleInvalidationNudge(zero(), invalidation_map, FROM_HERE); - RunLoop(); - EXPECT_EQ(1U, records.snapshots.size()); + PumpLoop(); + EXPECT_EQ(0U, records.snapshots.size()); - // Refresh requests will cause a sync, too. + // Ignore refresh requests for throttled types. scheduler()->ScheduleLocalRefreshRequest(zero(), throttled_types, FROM_HERE); - RunLoop(); - EXPECT_EQ(2U, records.snapshots.size()); + PumpLoop(); + EXPECT_EQ(0U, records.snapshots.size()); - // Even local nudges for other data types will trigger a sync. + // Local nudges for non-throttled types will trigger a sync. scheduler()->ScheduleLocalNudge(zero(), unthrottled_types, FROM_HERE); RunLoop(); - EXPECT_EQ(3U, records.snapshots.size()); + EXPECT_EQ(1U, records.snapshots.size()); StopSyncScheduler(); } @@ -1194,7 +1190,7 @@ TEST_F(SyncSchedulerTest, SyncerSteps) { StartSyncScheduler(SyncScheduler::NORMAL_MODE); // Poll. - EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END)) + EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES)) .Times(AtLeast(1)) .WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateSuccess), QuitLoopNowAction())); diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc index ad0d534..070f9ab 100644 --- a/sync/engine/syncer.cc +++ b/sync/engine/syncer.cc @@ -21,7 +21,6 @@ #include "sync/engine/process_updates_command.h" #include "sync/engine/store_timestamps_command.h" #include "sync/engine/syncer_types.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/internal_api/public/base/unique_position.h" #include "sync/syncable/mutable_entry.h" #include "sync/syncable/syncable-inl.h" @@ -92,8 +91,6 @@ bool Syncer::SyncShare(sessions::SyncSession* session, switch (current_step) { case SYNCER_BEGIN: - session->context()->throttled_data_type_tracker()-> - PruneUnthrottledTypes(base::TimeTicks::Now()); session->SendEventNotification(SyncEngineEvent::SYNC_CYCLE_BEGIN); next_step = DOWNLOAD_UPDATES; diff --git a/sync/engine/syncer_proto_util.cc b/sync/engine/syncer_proto_util.cc index 41054a3..690a837 100644 --- a/sync/engine/syncer_proto_util.cc +++ b/sync/engine/syncer_proto_util.cc @@ -10,7 +10,6 @@ #include "sync/engine/net/server_connection_manager.h" #include "sync/engine/syncer.h" #include "sync/engine/syncer_types.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/engine/traffic_logger.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/protocol/sync_enums.pb.h" @@ -307,20 +306,6 @@ base::TimeDelta SyncerProtoUtil::GetThrottleDelay( return throttle_delay; } -void SyncerProtoUtil::HandleThrottleError( - const SyncProtocolError& error, - const base::TimeTicks& throttled_until, - ThrottledDataTypeTracker* tracker, - sessions::SyncSession::Delegate* delegate) { - DCHECK_EQ(error.error_type, THROTTLED); - if (error.error_data_types.Empty()) { - // No datatypes indicates the client should be completely throttled. - delegate->OnSilencedUntil(throttled_until); - } else { - tracker->SetUnthrottleTime(error.error_data_types, throttled_until); - } -} - namespace { // Helper function for an assertion in PostClientToServerMessage. @@ -457,11 +442,15 @@ SyncerError SyncerProtoUtil::PostClientToServerMessage( LogResponseProfilingData(*response); return SYNCER_OK; case THROTTLED: - LOG(WARNING) << "Client silenced by server."; - HandleThrottleError(sync_protocol_error, - base::TimeTicks::Now() + GetThrottleDelay(*response), - session->context()->throttled_data_type_tracker(), - session->delegate()); + if (sync_protocol_error.error_data_types.Empty()) { + DLOG(WARNING) << "Client fully throttled by syncer."; + session->delegate()->OnThrottled(GetThrottleDelay(*response)); + } else { + DLOG(WARNING) << "Some types throttled by syncer."; + session->delegate()->OnTypesThrottled( + sync_protocol_error.error_data_types, + GetThrottleDelay(*response)); + } return SERVER_RETURN_THROTTLED; case TRANSIENT_ERROR: return SERVER_RETURN_TRANSIENT_ERROR; diff --git a/sync/engine/syncer_proto_util.h b/sync/engine/syncer_proto_util.h index fedd211..9faddf8 100644 --- a/sync/engine/syncer_proto_util.h +++ b/sync/engine/syncer_proto_util.h @@ -26,7 +26,6 @@ class SyncEntity; namespace syncer { -class ThrottledDataTypeTracker; class ServerConnectionManager; namespace sessions { @@ -140,12 +139,6 @@ class SYNC_EXPORT_PRIVATE SyncerProtoUtil { static base::TimeDelta GetThrottleDelay( const sync_pb::ClientToServerResponse& response); - static void HandleThrottleError( - const SyncProtocolError& error, - const base::TimeTicks& throttled_until, - ThrottledDataTypeTracker* tracker, - sessions::SyncSession::Delegate* delegate); - friend class SyncerProtoUtilTest; FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, AddRequestBirthday); FRIEND_TEST_ALL_PREFIXES(SyncerProtoUtilTest, PostAndProcessHeaders); diff --git a/sync/engine/syncer_proto_util_unittest.cc b/sync/engine/syncer_proto_util_unittest.cc index bc2feb9..32e4531 100644 --- a/sync/engine/syncer_proto_util_unittest.cc +++ b/sync/engine/syncer_proto_util_unittest.cc @@ -10,7 +10,6 @@ #include "base/compiler_specific.h" #include "base/message_loop.h" #include "base/time.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/internal_api/public/base/model_type_test_util.h" #include "sync/protocol/bookmark_specifics.pb.h" #include "sync/protocol/password_specifics.pb.h" @@ -297,32 +296,4 @@ TEST_F(SyncerProtoUtilTest, PostAndProcessHeaders) { msg, &response)); } -TEST_F(SyncerProtoUtilTest, HandleThrottlingWithDatatypes) { - ThrottledDataTypeTracker tracker(NULL); - SyncProtocolError error; - error.error_type = THROTTLED; - ModelTypeSet types; - types.Put(BOOKMARKS); - types.Put(PASSWORDS); - error.error_data_types = types; - - base::TimeTicks ticks = base::TimeTicks::FromInternalValue(1); - SyncerProtoUtil::HandleThrottleError(error, ticks, &tracker, NULL); - EXPECT_TRUE(tracker.GetThrottledTypes().Equals(types)); -} - -TEST_F(SyncerProtoUtilTest, HandleThrottlingNoDatatypes) { - ThrottledDataTypeTracker tracker(NULL); - MockDelegate delegate; - SyncProtocolError error; - error.error_type = THROTTLED; - - base::TimeTicks ticks = base::TimeTicks::FromInternalValue(1); - - EXPECT_CALL(delegate, OnSilencedUntil(ticks)); - - SyncerProtoUtil::HandleThrottleError(error, ticks, &tracker, &delegate); - EXPECT_TRUE(tracker.GetThrottledTypes().Empty()); -} - } // namespace syncer diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index 1b62696..3361caf 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -29,7 +29,6 @@ #include "sync/engine/sync_scheduler_impl.h" #include "sync/engine/syncer.h" #include "sync/engine/syncer_proto_util.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/engine/traffic_recorder.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/engine/model_safe_worker.h" @@ -121,10 +120,15 @@ class SyncerTest : public testing::Test, } // SyncSession::Delegate implementation. - virtual void OnSilencedUntil(const base::TimeTicks& silenced_until) OVERRIDE { + virtual void OnThrottled(const base::TimeDelta& throttle_duration) OVERRIDE { FAIL() << "Should not get silenced."; } - virtual bool IsSyncingCurrentlySilenced() OVERRIDE { + virtual void OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) OVERRIDE { + FAIL() << "Should not get silenced."; + } + virtual bool IsCurrentlyThrottled() OVERRIDE { return false; } virtual void OnReceivedLongPollIntervalUpdate( @@ -228,12 +232,10 @@ class SyncerTest : public testing::Test, GetModelSafeRoutingInfo(&routing_info); GetWorkers(&workers); - throttled_data_type_tracker_.reset(new ThrottledDataTypeTracker(NULL)); - context_.reset( new SyncSessionContext( mock_server_.get(), directory(), workers, - &extensions_activity_monitor_, throttled_data_type_tracker_.get(), + &extensions_activity_monitor_, listeners, NULL, &traffic_recorder_, true, // enable keystore encryption "fake_invalidator_client_id")); @@ -570,7 +572,6 @@ class SyncerTest : public testing::Test, TestDirectorySetterUpper dir_maker_; FakeEncryptor encryptor_; FakeExtensionsActivityMonitor extensions_activity_monitor_; - scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_; scoped_ptr<MockConnectionManager> mock_server_; Syncer* syncer_; @@ -662,7 +663,8 @@ TEST_F(SyncerTest, GetCommitIdsCommandTruncates) { DoTruncationTest(unsynced_handle_view, expected_order); } -TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) { +// TODO(rlarocque): re-enable this test. +TEST_F(SyncerTest, DISABLED_GetCommitIdsFiltersThrottledEntries) { const ModelTypeSet throttled_types(BOOKMARKS); sync_pb::EntitySpecifics bookmark_data; AddDefaultFieldValue(BOOKMARKS, &bookmark_data); @@ -681,9 +683,9 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) { } // Now set the throttled types. - context_->throttled_data_type_tracker()->SetUnthrottleTime( - throttled_types, - base::TimeTicks::Now() + base::TimeDelta::FromSeconds(1200)); + // context_->throttled_data_type_tracker()->SetUnthrottleTime( + // throttled_types, + // base::TimeTicks::Now() + base::TimeDelta::FromSeconds(1200)); SyncShareNudge(); { @@ -695,9 +697,9 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) { } // Now unthrottle. - context_->throttled_data_type_tracker()->SetUnthrottleTime( - throttled_types, - base::TimeTicks::Now() - base::TimeDelta::FromSeconds(1200)); + // context_->throttled_data_type_tracker()->SetUnthrottleTime( + // throttled_types, + // base::TimeTicks::Now() - base::TimeDelta::FromSeconds(1200)); SyncShareNudge(); { // It should have been committed. diff --git a/sync/engine/throttled_data_type_tracker.cc b/sync/engine/throttled_data_type_tracker.cc deleted file mode 100644 index fb7b08a..0000000 --- a/sync/engine/throttled_data_type_tracker.cc +++ /dev/null @@ -1,72 +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/engine/throttled_data_type_tracker.h" - -#include "base/time.h" -#include "sync/engine/all_status.h" -#include "sync/internal_api/public/base/model_type.h" - -namespace syncer { - -ThrottledDataTypeTracker::ThrottledDataTypeTracker(AllStatus *allstatus) - : allstatus_(allstatus) { - if (allstatus_) { - allstatus_->SetThrottledTypes(ModelTypeSet()); - } -} - -ThrottledDataTypeTracker::~ThrottledDataTypeTracker() { } - -void ThrottledDataTypeTracker::SetUnthrottleTime(ModelTypeSet types, - const base::TimeTicks& time) { - for (ModelTypeSet::Iterator it = types.First(); it.Good(); it.Inc()) { - unthrottle_times_[it.Get()] = time; - } - - DVLOG(2) - << "Throttling types: " << ModelTypeSetToString(types) - << ", throttled list is now: " - << ModelTypeSetToString(GetThrottledTypes()); - if (allstatus_) { - allstatus_->SetThrottledTypes(GetThrottledTypes()); - } -} - -void ThrottledDataTypeTracker::PruneUnthrottledTypes( - const base::TimeTicks& time) { - bool modified = false; - - UnthrottleTimes::iterator it = unthrottle_times_.begin(); - while (it != unthrottle_times_.end()) { - if (it->second <= time) { - // Delete and increment the iterator. - UnthrottleTimes::iterator iterator_to_delete = it; - ++it; - unthrottle_times_.erase(iterator_to_delete); - modified = true; - } else { - // Just increment the iterator. - ++it; - } - } - - DVLOG_IF(2, modified) - << "Remaining throttled types: " - << ModelTypeSetToString(GetThrottledTypes()); - if (modified && allstatus_) { - allstatus_->SetThrottledTypes(GetThrottledTypes()); - } -} - -ModelTypeSet ThrottledDataTypeTracker::GetThrottledTypes() const { - ModelTypeSet types; - for (UnthrottleTimes::const_iterator it = unthrottle_times_.begin(); - it != unthrottle_times_.end(); ++it) { - types.Put(it->first); - } - return types; -} - -} // namespace syncer diff --git a/sync/engine/throttled_data_type_tracker.h b/sync/engine/throttled_data_type_tracker.h deleted file mode 100644 index 9054ae6..0000000 --- a/sync/engine/throttled_data_type_tracker.h +++ /dev/null @@ -1,58 +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_ENGINE_THROTTLED_DATA_TYPE_TRACKER_H_ -#define SYNC_ENGINE_THROTTLED_DATA_TYPE_TRACKER_H_ - -#include <map> - -#include "base/gtest_prod_util.h" -#include "sync/base/sync_export.h" -#include "sync/internal_api/public/base/model_type.h" - -namespace base { -class TimeTicks; -} - -namespace syncer { - -class AllStatus; - -class SYNC_EXPORT_PRIVATE ThrottledDataTypeTracker { - public: - // The given allstatus argument will be kept up to date on this object's list - // of throttled types. The argument may be NULL in tests. - explicit ThrottledDataTypeTracker(AllStatus* allstatus); - ~ThrottledDataTypeTracker(); - - // Throttles a set of data types until the specified time is reached. - void SetUnthrottleTime(ModelTypeSet types, const base::TimeTicks& time); - - // Given an input of the current time (usually from time::Now()), removes from - // the set of throttled types any types whose throttling period has expired. - void PruneUnthrottledTypes(const base::TimeTicks& time); - - // Returns the set of types which are currently throttled. - ModelTypeSet GetThrottledTypes() const; - - private: - FRIEND_TEST_ALL_PREFIXES(ThrottledDataTypeTrackerTest, - AddUnthrottleTimeTest); - FRIEND_TEST_ALL_PREFIXES(ThrottledDataTypeTrackerTest, - GetCurrentlyThrottledTypesTest); - - typedef std::map<ModelType, base::TimeTicks> UnthrottleTimes; - - // This is a map from throttled data types to the time at which they can be - // unthrottled. - UnthrottleTimes unthrottle_times_; - - AllStatus* allstatus_; - - DISALLOW_COPY_AND_ASSIGN(ThrottledDataTypeTracker); -}; - -} // namespace syncer - -#endif // SYNC_ENGINE_THROTTLED_DATA_TYPE_TRACKER_H_ diff --git a/sync/engine/throttled_data_type_tracker_unittest.cc b/sync/engine/throttled_data_type_tracker_unittest.cc deleted file mode 100644 index cde427d..0000000 --- a/sync/engine/throttled_data_type_tracker_unittest.cc +++ /dev/null @@ -1,67 +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/engine/throttled_data_type_tracker.h" - -#include "base/time.h" -#include "sync/internal_api/public/base/model_type.h" -#include "testing/gtest/include/gtest/gtest.h" - -using base::TimeDelta; -using base::TimeTicks; - -namespace syncer { - -TEST(ThrottledDataTypeTrackerTest, AddUnthrottleTimeTest) { - const ModelTypeSet types(BOOKMARKS, PASSWORDS); - - ThrottledDataTypeTracker throttler(NULL); - TimeTicks now = TimeTicks::Now(); - throttler.SetUnthrottleTime(types, now); - - EXPECT_EQ(throttler.unthrottle_times_.size(), 2U); - EXPECT_EQ(throttler.unthrottle_times_[BOOKMARKS], now); - EXPECT_EQ(throttler.unthrottle_times_[PASSWORDS], now); -} - -TEST(ThrottledDataTypeTrackerTest, GetCurrentlyThrottledTypesTest) { - const ModelTypeSet types(BOOKMARKS, PASSWORDS); - - ThrottledDataTypeTracker throttler(NULL); - TimeTicks now = TimeTicks::Now(); - - // Now update the throttled types with time set to 10 seconds earlier from - // now. - throttler.SetUnthrottleTime(types, now - TimeDelta::FromSeconds(10)); - throttler.PruneUnthrottledTypes(TimeTicks::Now()); - EXPECT_TRUE(throttler.GetThrottledTypes().Empty()); - - // Now update the throttled types with time set to 2 hours from now. - throttler.SetUnthrottleTime(types, now + TimeDelta::FromSeconds(1200)); - throttler.PruneUnthrottledTypes(TimeTicks::Now()); - EXPECT_TRUE(throttler.GetThrottledTypes().Equals(types)); -} - -// Have two data types whose throttling is set to expire at different times. -TEST(ThrottledDataTypeTrackerTest, UnthrottleSomeTypesTest) { - const ModelTypeSet long_throttled(BOOKMARKS); - const ModelTypeSet short_throttled(PASSWORDS); - - const TimeTicks start_time = TimeTicks::Now(); - const TimeTicks short_throttle_time = start_time + TimeDelta::FromSeconds(1); - const TimeTicks long_throttle_time = start_time + TimeDelta::FromSeconds(20); - const TimeTicks in_between_time = start_time + TimeDelta::FromSeconds(5); - - ThrottledDataTypeTracker throttler(NULL); - - throttler.SetUnthrottleTime(long_throttled, long_throttle_time); - throttler.SetUnthrottleTime(short_throttled, short_throttle_time); - EXPECT_TRUE(throttler.GetThrottledTypes().Equals( - Union(short_throttled, long_throttled))); - - throttler.PruneUnthrottledTypes(in_between_time); - EXPECT_TRUE(throttler.GetThrottledTypes().Equals(long_throttled)); -} - -} // namespace syncer diff --git a/sync/internal_api/internal_components_factory_impl.cc b/sync/internal_api/internal_components_factory_impl.cc index ea60250..f4fd942 100644 --- a/sync/internal_api/internal_components_factory_impl.cc +++ b/sync/internal_api/internal_components_factory_impl.cc @@ -38,7 +38,6 @@ InternalComponentsFactoryImpl::BuildContext( syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, @@ -46,7 +45,7 @@ InternalComponentsFactoryImpl::BuildContext( return scoped_ptr<sessions::SyncSessionContext>( new sessions::SyncSessionContext( connection_manager, directory, workers, monitor, - throttled_data_type_tracker, listeners, debug_info_getter, + listeners, debug_info_getter, traffic_recorder, switches_.encryption_method == ENCRYPTION_KEYSTORE, invalidation_client_id)); diff --git a/sync/internal_api/public/internal_components_factory.h b/sync/internal_api/public/internal_components_factory.h index 1d5b122..6b79ddd 100644 --- a/sync/internal_api/public/internal_components_factory.h +++ b/sync/internal_api/public/internal_components_factory.h @@ -24,8 +24,6 @@ class SyncEngineEventListener; class SyncScheduler; class TrafficRecorder; -class ThrottledDataTypeTracker; - namespace sessions { class DebugInfoGetter; class SyncSessionContext; @@ -73,7 +71,6 @@ class SYNC_EXPORT InternalComponentsFactory { syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/internal_api/public/internal_components_factory_impl.h b/sync/internal_api/public/internal_components_factory_impl.h index 646da5f..6fec27f 100644 --- a/sync/internal_api/public/internal_components_factory_impl.h +++ b/sync/internal_api/public/internal_components_factory_impl.h @@ -28,7 +28,6 @@ class SYNC_EXPORT InternalComponentsFactoryImpl syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/internal_api/public/test/test_internal_components_factory.h b/sync/internal_api/public/test/test_internal_components_factory.h index 665999d..b608154 100644 --- a/sync/internal_api/public/test/test_internal_components_factory.h +++ b/sync/internal_api/public/test/test_internal_components_factory.h @@ -34,7 +34,6 @@ class TestInternalComponentsFactory : public InternalComponentsFactory { syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index eff419a..55150f1 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -173,7 +173,6 @@ SyncManagerImpl::SyncManagerImpl(const std::string& name) initialized_(false), observing_network_connectivity_changes_(false), invalidator_state_(DEFAULT_INVALIDATION_ERROR), - throttled_data_type_tracker_(&allstatus_), traffic_recorder_(kMaxMessagesToRecord, kMaxMessageSizeToRecord), encryptor_(NULL), unrecoverable_error_handler_(NULL), @@ -460,7 +459,6 @@ void SyncManagerImpl::Init( directory(), workers, extensions_activity_monitor, - &throttled_data_type_tracker_, listeners, &debug_info_event_listener_, &traffic_recorder_, diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h index a078e6a..3d2c2cf 100644 --- a/sync/internal_api/sync_manager_impl.h +++ b/sync/internal_api/sync_manager_impl.h @@ -13,7 +13,6 @@ #include "sync/engine/all_status.h" #include "sync/engine/net/server_connection_manager.h" #include "sync/engine/sync_engine_event.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/engine/traffic_recorder.h" #include "sync/internal_api/change_reorder_buffer.h" #include "sync/internal_api/debug_info_event_listener.h" @@ -374,8 +373,6 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl : JsMutationEventObserver js_mutation_event_observer_; JsSyncEncryptionHandlerObserver js_sync_encryption_handler_observer_; - ThrottledDataTypeTracker throttled_data_type_tracker_; - // This is for keeping track of client events to send to the server. DebugInfoEventListener debug_info_event_listener_; diff --git a/sync/internal_api/test/test_internal_components_factory.cc b/sync/internal_api/test/test_internal_components_factory.cc index 18962bc..beb249e 100644 --- a/sync/internal_api/test/test_internal_components_factory.cc +++ b/sync/internal_api/test/test_internal_components_factory.cc @@ -32,7 +32,6 @@ TestInternalComponentsFactory::BuildContext( syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, sessions::DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, @@ -43,7 +42,7 @@ TestInternalComponentsFactory::BuildContext( return scoped_ptr<sessions::SyncSessionContext>( new sessions::SyncSessionContext( connection_manager, directory, workers, monitor, - throttled_data_type_tracker, empty_listeners, debug_info_getter, + empty_listeners, debug_info_getter, traffic_recorder, switches_.encryption_method == ENCRYPTION_KEYSTORE, invalidator_client_id)); diff --git a/sync/sessions/data_type_tracker.cc b/sync/sessions/data_type_tracker.cc index 639ccc0..857b86a 100644 --- a/sync/sessions/data_type_tracker.cc +++ b/sync/sessions/data_type_tracker.cc @@ -37,6 +37,11 @@ void DataTypeTracker::RecordRemoteInvalidation( } void DataTypeTracker::RecordSuccessfulSyncCycle() { + // If we were throttled, then we would have been excluded from this cycle's + // GetUpdates and Commit actions. Our state remains unchanged. + if (IsThrottled()) + return; + local_nudge_count_ = 0; local_refresh_request_count_ = 0; pending_payloads_.clear(); @@ -50,11 +55,10 @@ void DataTypeTracker::UpdatePayloadBufferSize(size_t new_size) { } bool DataTypeTracker::IsSyncRequired() const { - return local_nudge_count_ > 0 || IsGetUpdatesRequired(); -} - -bool DataTypeTracker::IsGetUpdatesRequired() const { - return local_refresh_request_count_ > 0 || !pending_payloads_.empty(); + return !IsThrottled() && + (local_nudge_count_ > 0 || + local_refresh_request_count_ > 0 || + !pending_payloads_.empty()); } bool DataTypeTracker::HasLocalChangePending() const { @@ -87,5 +91,30 @@ void DataTypeTracker::FillGetUpdatesTriggersMessage( // msg->set_server_dropped_hints(server_payload_oveflow_); } +bool DataTypeTracker::IsThrottled() const { + return !unthrottle_time_.is_null(); +} + +base::TimeDelta DataTypeTracker::GetTimeUntilUnthrottle( + base::TimeTicks now) const { + if (!IsThrottled()) { + NOTREACHED(); + return base::TimeDelta::FromSeconds(0); + } + return std::max(base::TimeDelta::FromSeconds(0), + unthrottle_time_ - now); +} + +void DataTypeTracker::ThrottleType(base::TimeDelta duration, + base::TimeTicks now) { + unthrottle_time_ = std::max(unthrottle_time_, now + duration); +} + +void DataTypeTracker::UpdateThrottleState(base::TimeTicks now) { + if (now >= unthrottle_time_) { + unthrottle_time_ = base::TimeTicks(); + } +} + } // namespace sessions } // namespace syncer diff --git a/sync/sessions/data_type_tracker.h b/sync/sessions/data_type_tracker.h index 494c881..d6da946 100644 --- a/sync/sessions/data_type_tracker.h +++ b/sync/sessions/data_type_tracker.h @@ -10,6 +10,7 @@ #include <string> #include "base/basictypes.h" +#include "base/time.h" #include "sync/protocol/sync.pb.h" namespace syncer { @@ -48,11 +49,6 @@ class DataTypeTracker { // cycle. That's for the scheduler to decide. bool IsSyncRequired() const; - // Returns true if one of the reasons behind the need for a sync cycle is to - // fetch updates. If this is true, then IsSyncRequired() will also return - // true. - bool IsGetUpdatesRequired() const; - // Returns true if there is an uncommitted local change. bool HasLocalChangePending() const; @@ -68,6 +64,20 @@ class DataTypeTracker { // handle a request. void FillGetUpdatesTriggersMessage(sync_pb::GetUpdateTriggers* msg) const; + // Returns true if the type is currently throttled. + bool IsThrottled() const; + + // Returns the time until this type's throttling interval expires. Should not + // be called unless IsThrottled() returns true. The returned value will be + // increased to zero if it would otherwise have been negative. + base::TimeDelta GetTimeUntilUnthrottle(base::TimeTicks now) const; + + // Throttles the type from |now| until |now| + |duration|. + void ThrottleType(base::TimeDelta duration, base::TimeTicks now); + + // Unthrottles the type if |now| >= the throttle expiry time. + void UpdateThrottleState(base::TimeTicks now); + private: // Number of local change nudges received for this type since the last // successful sync cycle. @@ -91,6 +101,10 @@ class DataTypeTracker { bool server_payload_overflow_; size_t payload_buffer_size_; + + // If !unthrottle_time_.is_null(), this type is throttled and may not download + // or commit data until the specified time. + base::TimeTicks unthrottle_time_; }; } // namespace syncer diff --git a/sync/sessions/nudge_tracker.cc b/sync/sessions/nudge_tracker.cc index 9d4d3b2..97f211f 100644 --- a/sync/sessions/nudge_tracker.cc +++ b/sync/sessions/nudge_tracker.cc @@ -4,6 +4,7 @@ #include "sync/sessions/nudge_tracker.h" +#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" @@ -27,16 +28,6 @@ NudgeTracker::NudgeTracker() NudgeTracker::~NudgeTracker() { } -bool NudgeTracker::IsGetUpdatesRequired() { - for (TypeTrackerMap::iterator it = type_trackers_.begin(); - it != type_trackers_.end(); ++it) { - if (it->second.IsGetUpdatesRequired()) { - return true; - } - } - return false; -} - bool NudgeTracker::IsSyncRequired() { for (TypeTrackerMap::iterator it = type_trackers_.begin(); it != type_trackers_.end(); ++it) { @@ -112,6 +103,69 @@ void NudgeTracker::OnInvalidationsDisabled() { invalidations_out_of_sync_ = true; } +void NudgeTracker::SetTypesThrottledUntil( + ModelTypeSet types, + base::TimeDelta length, + base::TimeTicks now) { + for (ModelTypeSet::Iterator it = types.First(); it.Good(); it.Inc()) { + type_trackers_[it.Get()].ThrottleType(length, now); + } +} + +void NudgeTracker::UpdateTypeThrottlingState(base::TimeTicks now) { + for (TypeTrackerMap::iterator it = type_trackers_.begin(); + it != type_trackers_.end(); ++it) { + it->second.UpdateThrottleState(now); + } +} + +bool NudgeTracker::IsAnyTypeThrottled() const { + for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); + it != type_trackers_.end(); ++it) { + if (it->second.IsThrottled()) { + return true; + } + } + return false; +} + +bool NudgeTracker::IsTypeThrottled(ModelType type) const { + DCHECK(type_trackers_.find(type) != type_trackers_.end()); + return type_trackers_.find(type)->second.IsThrottled(); +} + +base::TimeDelta NudgeTracker::GetTimeUntilNextUnthrottle( + base::TimeTicks now) const { + DCHECK(IsAnyTypeThrottled()) << "This function requires a pending unthrottle"; + const base::TimeDelta kMaxTimeDelta = + base::TimeDelta::FromInternalValue(kint64max); + + // Return min of GetTimeUntilUnthrottle() values for all IsThrottled() types. + base::TimeDelta time_until_next_unthrottle = kMaxTimeDelta; + for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); + it != type_trackers_.end(); ++it) { + if (it->second.IsThrottled()) { + time_until_next_unthrottle = + std::min(time_until_next_unthrottle, + it->second.GetTimeUntilUnthrottle(now)); + } + } + DCHECK(kMaxTimeDelta != time_until_next_unthrottle); + + return time_until_next_unthrottle; +} + +ModelTypeSet NudgeTracker::GetThrottledTypes() const { + ModelTypeSet result; + for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); + it != type_trackers_.end(); ++it) { + if (it->second.IsThrottled()) { + result.Put(it->first); + } + } + 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. @@ -119,7 +173,10 @@ SyncSourceInfo NudgeTracker::GetSourceInfo() const { ModelTypeInvalidationMap invalidation_map; for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); it != type_trackers_.end(); ++it) { - if (it->second.HasPendingInvalidation()) { + 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; @@ -138,17 +195,6 @@ SyncSourceInfo NudgeTracker::GetSourceInfo() const { return SyncSourceInfo(updates_source_, invalidation_map); } -ModelTypeSet NudgeTracker::GetLocallyModifiedTypes() const { - ModelTypeSet nudged_types; - for (TypeTrackerMap::const_iterator it = type_trackers_.begin(); - it != type_trackers_.end(); ++it) { - if (it->second.HasLocalChangePending()) { - nudged_types.Put(it->first); - } - } - return nudged_types; -} - sync_pb::GetUpdatesCallerInfo::GetUpdatesSource NudgeTracker::updates_source() const { return updates_source_; @@ -162,7 +208,7 @@ void NudgeTracker::FillProtoMessage( // Fill what we can from the global data. msg->set_invalidations_out_of_sync(invalidations_out_of_sync_); - // Delegate the type-specific work to TypeSchedulingData class. + // Delegate the type-specific work to the DataTypeTracker class. type_trackers_.find(type)->second.FillGetUpdatesTriggersMessage(msg); } diff --git a/sync/sessions/nudge_tracker.h b/sync/sessions/nudge_tracker.h index c71fd00..3db3143 100644 --- a/sync/sessions/nudge_tracker.h +++ b/sync/sessions/nudge_tracker.h @@ -29,11 +29,6 @@ class SYNC_EXPORT_PRIVATE NudgeTracker { NudgeTracker(); ~NudgeTracker(); - // Returns true if one of the main reasons for performing the sync cycle is to - // fetch updates. This is true when we have pending invalidations or refresh - // requests. - bool IsGetUpdatesRequired(); - // Returns true if there is a good reason for performing a sync cycle. // This does not take into account whether or not this is a good *time* to // perform a sync cycle; that's the scheduler's job. @@ -58,14 +53,31 @@ class SYNC_EXPORT_PRIVATE NudgeTracker { void OnInvalidationsEnabled(); void OnInvalidationsDisabled(); + // Marks |types| as being throttled from |now| until |now| + |length|. + void SetTypesThrottledUntil(ModelTypeSet types, + base::TimeDelta length, + base::TimeTicks now); + + // Removes any throttling that have expired by time |now|. + void UpdateTypeThrottlingState(base::TimeTicks now); + + // Returns the time of the next type unthrottling, relative to + // the input |now| value. + base::TimeDelta GetTimeUntilNextUnthrottle(base::TimeTicks now) const; + + // Returns true if any type is currenlty throttled. + bool IsAnyTypeThrottled() const; + + // Returns true if |type| is currently throttled. + bool IsTypeThrottled(ModelType type) const; + + // 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 set of locally modified types, according to the nudges received - // since the last successful sync cycle. - ModelTypeSet GetLocallyModifiedTypes() 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 f6031a3..9e6ce99 100644 --- a/sync/sessions/nudge_tracker_unittest.cc +++ b/sync/sessions/nudge_tracker_unittest.cc @@ -60,9 +60,9 @@ class NudgeTrackerTest : public ::testing::Test { TEST_F(NudgeTrackerTest, EmptyNudgeTracker) { NudgeTracker nudge_tracker; + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::UNKNOWN, nudge_tracker.updates_source()); - EXPECT_TRUE(nudge_tracker.GetLocallyModifiedTypes().Empty()); sync_pb::GetUpdateTriggers gu_trigger; nudge_tracker.FillProtoMessage(BOOKMARKS, &gu_trigger); @@ -109,44 +109,6 @@ TEST_F(NudgeTrackerTest, SourcePriorities) { nudge_tracker.updates_source()); } -// Verify locally modified type coalescing and independence from other nudges. -TEST_F(NudgeTrackerTest, LocallyModifiedTypes) { - NudgeTracker nudge_tracker; - - // Start with a notification. Verify it has no effect. - ModelTypeInvalidationMap invalidation_map1 = - ModelTypeSetToInvalidationMap(ModelTypeSet(PREFERENCES), - std::string("hint")); - nudge_tracker.RecordRemoteInvalidation(invalidation_map1); - EXPECT_TRUE(nudge_tracker.GetLocallyModifiedTypes().Empty()); - - // Record a local bookmark change. Verify it was registered correctly. - nudge_tracker.RecordLocalChange(ModelTypeSet(BOOKMARKS)); - EXPECT_TRUE(ModelTypeSetEquals( - ModelTypeSet(BOOKMARKS), - nudge_tracker.GetLocallyModifiedTypes())); - - // Record a notification and a refresh request. Verify they have no effect. - ModelTypeInvalidationMap invalidation_map2 = - ModelTypeSetToInvalidationMap(ModelTypeSet(PASSWORDS), - std::string("hint")); - nudge_tracker.RecordRemoteInvalidation(invalidation_map2); - EXPECT_TRUE(ModelTypeSetEquals( - ModelTypeSet(BOOKMARKS), - nudge_tracker.GetLocallyModifiedTypes())); - - nudge_tracker.RecordLocalRefreshRequest(ModelTypeSet(AUTOFILL)); - EXPECT_TRUE(ModelTypeSetEquals( - ModelTypeSet(BOOKMARKS), - nudge_tracker.GetLocallyModifiedTypes())); - - // Record another local nudge. Verify it was coalesced correctly. - nudge_tracker.RecordLocalChange(ModelTypeSet(THEMES)); - EXPECT_TRUE(ModelTypeSetEquals( - ModelTypeSet(THEMES, BOOKMARKS), - nudge_tracker.GetLocallyModifiedTypes())); -} - TEST_F(NudgeTrackerTest, HintCoalescing) { NudgeTracker nudge_tracker; @@ -311,5 +273,145 @@ TEST_F(NudgeTrackerTest, WriteRefreshRequestedTypesToProto) { EXPECT_EQ(0, ProtoRefreshRequestedCount(nudge_tracker, SESSIONS)); } +// Basic tests for the IsSyncRequired() flag. +TEST_F(NudgeTrackerTest, IsSyncRequired) { + NudgeTracker nudge_tracker; + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); + + // Local changes. + nudge_tracker.RecordLocalChange(ModelTypeSet(SESSIONS)); + EXPECT_TRUE(nudge_tracker.IsSyncRequired()); + nudge_tracker.RecordSuccessfulSyncCycle(); + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); + + // Refresh requests. + nudge_tracker.RecordLocalRefreshRequest(ModelTypeSet(SESSIONS)); + EXPECT_TRUE(nudge_tracker.IsSyncRequired()); + nudge_tracker.RecordSuccessfulSyncCycle(); + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); + + // Invalidations. + ModelTypeInvalidationMap invalidation_map = + ModelTypeSetToInvalidationMap(ModelTypeSet(PREFERENCES), + std::string("hint")); + nudge_tracker.RecordRemoteInvalidation(invalidation_map); + EXPECT_TRUE(nudge_tracker.IsSyncRequired()); + nudge_tracker.RecordSuccessfulSyncCycle(); + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); +} + +// Test IsSyncRequired() responds correctly to data type throttling. +TEST_F(NudgeTrackerTest, IsSyncRequired_Throttling) { + NudgeTracker nudge_tracker; + const base::TimeTicks t0 = base::TimeTicks::FromInternalValue(1234); + const base::TimeDelta throttle_length = base::TimeDelta::FromMinutes(10); + const base::TimeTicks t1 = t0 + throttle_length; + + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); + + // A local change to sessions enables the flag. + nudge_tracker.RecordLocalChange(ModelTypeSet(SESSIONS)); + EXPECT_TRUE(nudge_tracker.IsSyncRequired()); + + // But the throttling of sessions unsets it. + nudge_tracker.SetTypesThrottledUntil(ModelTypeSet(SESSIONS), + throttle_length, + t0); + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); + + // A refresh request for bookmarks means we have reason to sync again. + nudge_tracker.RecordLocalChange(ModelTypeSet(BOOKMARKS)); + EXPECT_TRUE(nudge_tracker.IsSyncRequired()); + + // A successful sync cycle means we took care of bookmarks. + nudge_tracker.RecordSuccessfulSyncCycle(); + EXPECT_FALSE(nudge_tracker.IsSyncRequired()); + + // But we still haven't dealt with sessions. We'll need to remember + // that sessions are out of sync and re-enable the flag when their + // throttling interval expires. + nudge_tracker.UpdateTypeThrottlingState(t1); + EXPECT_FALSE(nudge_tracker.IsTypeThrottled(SESSIONS)); + EXPECT_TRUE(nudge_tracker.IsSyncRequired()); +} + +// Tests throttling-related getter functions when no types are throttled. +TEST_F(NudgeTrackerTest, NoTypesThrottled) { + NudgeTracker nudge_tracker; + + EXPECT_FALSE(nudge_tracker.IsAnyTypeThrottled()); + EXPECT_FALSE(nudge_tracker.IsTypeThrottled(SESSIONS)); + EXPECT_TRUE(nudge_tracker.GetThrottledTypes().Empty()); +} + +// Tests throttling-related getter functions when some types are throttled. +TEST_F(NudgeTrackerTest, ThrottleAndUnthrottle) { + NudgeTracker nudge_tracker; + const base::TimeTicks t0 = base::TimeTicks::FromInternalValue(1234); + const base::TimeDelta throttle_length = base::TimeDelta::FromMinutes(10); + const base::TimeTicks t1 = t0 + throttle_length; + + nudge_tracker.SetTypesThrottledUntil(ModelTypeSet(SESSIONS, PREFERENCES), + throttle_length, + t0); + + EXPECT_TRUE(nudge_tracker.IsAnyTypeThrottled()); + EXPECT_TRUE(nudge_tracker.IsTypeThrottled(SESSIONS)); + EXPECT_TRUE(nudge_tracker.IsTypeThrottled(PREFERENCES)); + EXPECT_FALSE(nudge_tracker.GetThrottledTypes().Empty()); + EXPECT_EQ(throttle_length, nudge_tracker.GetTimeUntilNextUnthrottle(t0)); + + nudge_tracker.UpdateTypeThrottlingState(t1); + + EXPECT_FALSE(nudge_tracker.IsAnyTypeThrottled()); + EXPECT_FALSE(nudge_tracker.IsTypeThrottled(SESSIONS)); + EXPECT_TRUE(nudge_tracker.GetThrottledTypes().Empty()); +} + +TEST_F(NudgeTrackerTest, OverlappingThrottleIntervals) { + NudgeTracker nudge_tracker; + const base::TimeTicks t0 = base::TimeTicks::FromInternalValue(1234); + const base::TimeDelta throttle1_length = base::TimeDelta::FromMinutes(10); + const base::TimeDelta throttle2_length = base::TimeDelta::FromMinutes(20); + const base::TimeTicks t1 = t0 + throttle1_length; + const base::TimeTicks t2 = t0 + throttle2_length; + + // Setup the longer of two intervals. + nudge_tracker.SetTypesThrottledUntil(ModelTypeSet(SESSIONS, PREFERENCES), + throttle2_length, + t0); + EXPECT_TRUE(ModelTypeSetEquals( + ModelTypeSet(SESSIONS, PREFERENCES), + nudge_tracker.GetThrottledTypes())); + EXPECT_EQ(throttle2_length, + nudge_tracker.GetTimeUntilNextUnthrottle(t0)); + + // Setup the shorter interval. + nudge_tracker.SetTypesThrottledUntil(ModelTypeSet(SESSIONS, BOOKMARKS), + throttle1_length, + t0); + EXPECT_TRUE(ModelTypeSetEquals( + ModelTypeSet(SESSIONS, PREFERENCES, BOOKMARKS), + nudge_tracker.GetThrottledTypes())); + EXPECT_EQ(throttle1_length, + nudge_tracker.GetTimeUntilNextUnthrottle(t0)); + + // Expire the first interval. + nudge_tracker.UpdateTypeThrottlingState(t1); + + // SESSIONS appeared in both intervals. We expect it will be throttled for + // the longer of the two, so it's still throttled at time t1. + EXPECT_TRUE(ModelTypeSetEquals( + ModelTypeSet(SESSIONS, PREFERENCES), + nudge_tracker.GetThrottledTypes())); + EXPECT_EQ(throttle2_length - throttle1_length, + nudge_tracker.GetTimeUntilNextUnthrottle(t1)); + + // Expire the second interval. + nudge_tracker.UpdateTypeThrottlingState(t2); + EXPECT_TRUE(nudge_tracker.GetThrottledTypes().Empty()); +} + + } // namespace sessions } // namespace syncer diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc index c2214e5..e526954 100644 --- a/sync/sessions/sync_session.cc +++ b/sync/sessions/sync_session.cc @@ -61,7 +61,7 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const { SyncSessionSnapshot snapshot( status_controller_->model_neutral_state(), download_progress_markers, - delegate_->IsSyncingCurrentlySilenced(), + delegate_->IsCurrentlyThrottled(), status_controller_->num_encryption_conflicts(), status_controller_->num_hierarchy_conflicts(), status_controller_->num_server_conflicts(), diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h index a8bfb4d..dcb0220 100644 --- a/sync/sessions/sync_session.h +++ b/sync/sessions/sync_session.h @@ -47,7 +47,12 @@ class SYNC_EXPORT_PRIVATE SyncSession { public: // The client was throttled and should cease-and-desist syncing activity // until the specified time. - virtual void OnSilencedUntil(const base::TimeTicks& silenced_until) = 0; + virtual void OnThrottled(const base::TimeDelta& throttle_duration) = 0; + + // Some of the client's types were throttled. + virtual void OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) = 0; // Silenced intervals can be out of phase with individual sessions, so the // delegate is the only thing that can give an authoritative answer for @@ -58,7 +63,7 @@ class SYNC_EXPORT_PRIVATE SyncSession { // solely based on absolute time values. So, this cannot be used to infer // that any given session _instance_ is silenced. An example of reasonable // use is for UI reporting. - virtual bool IsSyncingCurrentlySilenced() = 0; + virtual bool IsCurrentlyThrottled() = 0; // The client has been instructed to change its short poll interval. virtual void OnReceivedShortPollIntervalUpdate( diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc index 41ed246..9043db3 100644 --- a/sync/sessions/sync_session_context.cc +++ b/sync/sessions/sync_session_context.cc @@ -4,7 +4,6 @@ #include "sync/sessions/sync_session_context.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/sessions/debug_info_getter.h" #include "sync/util/extensions_activity_monitor.h" @@ -19,7 +18,6 @@ SyncSessionContext::SyncSessionContext( syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* extensions_activity_monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, @@ -30,7 +28,6 @@ SyncSessionContext::SyncSessionContext( extensions_activity_monitor_(extensions_activity_monitor), notifications_enabled_(false), max_commit_batch_size_(kDefaultMaxCommitBatchSize), - throttled_data_type_tracker_(throttled_data_type_tracker), debug_info_getter_(debug_info_getter), traffic_recorder_(traffic_recorder), keystore_encryption_enabled_(keystore_encryption_enabled), diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h index a6f693a..b694162 100644 --- a/sync/sessions/sync_session_context.h +++ b/sync/sessions/sync_session_context.h @@ -34,7 +34,6 @@ namespace syncer { class ExtensionsActivityMonitor; class ServerConnectionManager; -class ThrottledDataTypeTracker; namespace syncable { class Directory; @@ -52,7 +51,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext { syncable::Directory* directory, const std::vector<ModelSafeWorker*>& workers, ExtensionsActivityMonitor* extensions_activity_monitor, - ThrottledDataTypeTracker* throttled_data_type_tracker, const std::vector<SyncEngineEventListener*>& listeners, DebugInfoGetter* debug_info_getter, TrafficRecorder* traffic_recorder, @@ -84,10 +82,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext { return extensions_activity_monitor_; } - ThrottledDataTypeTracker* throttled_data_type_tracker() { - return throttled_data_type_tracker_; - } - DebugInfoGetter* debug_info_getter() { return debug_info_getter_; } @@ -167,8 +161,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext { // The server limits the number of items a client can commit in one batch. int max_commit_batch_size_; - ThrottledDataTypeTracker* throttled_data_type_tracker_; - // We use this to get debug info to send to the server for debugging // client behavior on server side. DebugInfoGetter* const debug_info_getter_; diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index 861abbe..5de32e2 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -9,7 +9,6 @@ #include "base/memory/ref_counted.h" #include "base/message_loop.h" #include "sync/engine/syncer_types.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/base/model_type_invalidation_map_test_util.h" #include "sync/sessions/status_controller.h" @@ -62,7 +61,6 @@ class SyncSessionTest : public testing::Test, NULL, workers, &extensions_activity_monitor_, - throttled_data_type_tracker_.get(), std::vector<SyncEngineEventListener*>(), NULL, NULL, @@ -71,17 +69,21 @@ class SyncSessionTest : public testing::Test, context_->set_routing_info(routes_); session_.reset(MakeSession()); - throttled_data_type_tracker_.reset(new ThrottledDataTypeTracker(NULL)); } virtual void TearDown() { session_.reset(); context_.reset(); } - virtual void OnSilencedUntil(const base::TimeTicks& silenced_until) OVERRIDE { - FailControllerInvocationIfDisabled("OnSilencedUntil"); + virtual void OnThrottled(const base::TimeDelta& throttle_duration) OVERRIDE { + FailControllerInvocationIfDisabled("OnThrottled"); } - virtual bool IsSyncingCurrentlySilenced() OVERRIDE { + virtual void OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) OVERRIDE { + FailControllerInvocationIfDisabled("OnTypesThrottled"); + } + virtual bool IsCurrentlyThrottled() OVERRIDE { FailControllerInvocationIfDisabled("IsSyncingCurrentlySilenced"); return false; } @@ -144,7 +146,6 @@ class SyncSessionTest : public testing::Test, std::vector<scoped_refptr<ModelSafeWorker> > workers_; ModelSafeRoutingInfo routes_; FakeExtensionsActivityMonitor extensions_activity_monitor_; - scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_; }; TEST_F(SyncSessionTest, MoreToDownloadIfDownloadFailed) { diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc index 03f6de2..24caa7b 100644 --- a/sync/sessions/test_util.cc +++ b/sync/sessions/test_util.cc @@ -4,8 +4,6 @@ #include "sync/sessions/test_util.h" -#include "sync/engine/throttled_data_type_tracker.h" - namespace syncer { namespace sessions { namespace test_util { @@ -42,14 +40,19 @@ void SimulateConnectionFailure(sessions::SyncSession* session, void SimulateSuccess(sessions::SyncSession* session, SyncerStep begin, SyncerStep end) { + const sync_pb::GetUpdatesCallerInfo::GetUpdatesSource source = + session->source().updates_source; ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); switch(end) { case SYNCER_END: session->mutable_status_controller()->set_commit_result(SYNCER_OK); - // Fall through. + session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); + session->mutable_status_controller()->set_last_download_updates_result( + SYNCER_OK); + break; case APPLY_UPDATES: - DCHECK_EQ(end == APPLY_UPDATES, session->source().updates_source == - sync_pb::GetUpdatesCallerInfo::RECONFIGURATION); + DCHECK(source == sync_pb::GetUpdatesCallerInfo::RECONFIGURATION + || source == sync_pb::GetUpdatesCallerInfo::PERIODIC); session->mutable_status_controller()->set_last_get_key_result(SYNCER_OK); session->mutable_status_controller()->set_last_download_updates_result( SYNCER_OK); @@ -63,7 +66,7 @@ void SimulateThrottledImpl(sessions::SyncSession* session, const base::TimeDelta& delta) { session->mutable_status_controller()->set_last_download_updates_result( SERVER_RETURN_THROTTLED); - session->delegate()->OnSilencedUntil(base::TimeTicks::Now() + delta); + session->delegate()->OnThrottled(delta); } void SimulateTypesThrottledImpl( @@ -72,8 +75,7 @@ void SimulateTypesThrottledImpl( const base::TimeDelta& delta) { session->mutable_status_controller()->set_last_download_updates_result( SERVER_RETURN_THROTTLED); - session->context()->throttled_data_type_tracker()-> - SetUnthrottleTime(types, base::TimeTicks::Now() + delta); + session->delegate()->OnTypesThrottled(types, delta); } void SimulatePollIntervalUpdateImpl(sessions::SyncSession* session, diff --git a/sync/sync_core.gypi b/sync/sync_core.gypi index 2f1370a..5d0bac9 100644 --- a/sync/sync_core.gypi +++ b/sync/sync_core.gypi @@ -77,8 +77,6 @@ 'engine/syncer_types.h', 'engine/syncer_util.cc', 'engine/syncer_util.h', - 'engine/throttled_data_type_tracker.cc', - 'engine/throttled_data_type_tracker.h', 'engine/traffic_logger.cc', 'engine/traffic_logger.h', 'engine/traffic_recorder.cc', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 110c955..7c56492 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -249,7 +249,6 @@ 'engine/sync_scheduler_unittest.cc', 'engine/syncer_proto_util_unittest.cc', 'engine/syncer_unittest.cc', - 'engine/throttled_data_type_tracker_unittest.cc', 'engine/traffic_recorder_unittest.cc', 'js/js_arg_list_unittest.cc', 'js/js_event_details_unittest.cc', diff --git a/sync/test/engine/fake_sync_scheduler.cc b/sync/test/engine/fake_sync_scheduler.cc index c2909e8..200edb0 100644 --- a/sync/test/engine/fake_sync_scheduler.cc +++ b/sync/test/engine/fake_sync_scheduler.cc @@ -57,10 +57,16 @@ void FakeSyncScheduler::OnConnectionStatusChange() { } -void FakeSyncScheduler::OnSilencedUntil( - const base::TimeTicks& silenced_until) { +void FakeSyncScheduler::OnThrottled( + const base::TimeDelta& throttle_duration) { } -bool FakeSyncScheduler::IsSyncingCurrentlySilenced() { + +void FakeSyncScheduler::OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) { +} + +bool FakeSyncScheduler::IsCurrentlyThrottled() { return false; } diff --git a/sync/test/engine/fake_sync_scheduler.h b/sync/test/engine/fake_sync_scheduler.h index 7bfe318..96805f5 100644 --- a/sync/test/engine/fake_sync_scheduler.h +++ b/sync/test/engine/fake_sync_scheduler.h @@ -42,9 +42,12 @@ class FakeSyncScheduler : public SyncScheduler { virtual void OnConnectionStatusChange() OVERRIDE; // SyncSession::Delegate implementation. - virtual void OnSilencedUntil( - const base::TimeTicks& silenced_until) OVERRIDE; - virtual bool IsSyncingCurrentlySilenced() OVERRIDE; + virtual void OnThrottled( + const base::TimeDelta& throttle_duration) OVERRIDE; + virtual void OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) OVERRIDE; + virtual bool IsCurrentlyThrottled() OVERRIDE; virtual void OnReceivedShortPollIntervalUpdate( const base::TimeDelta& new_interval) OVERRIDE; virtual void OnReceivedLongPollIntervalUpdate( diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h index a57663d5..2aa0f2f 100644 --- a/sync/test/engine/syncer_command_test.h +++ b/sync/test/engine/syncer_command_test.h @@ -13,7 +13,6 @@ #include "base/memory/ref_counted.h" #include "base/message_loop.h" #include "sync/engine/model_changing_syncer_command.h" -#include "sync/engine/throttled_data_type_tracker.h" #include "sync/engine/traffic_recorder.h" #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/sessions/debug_info_getter.h" @@ -45,11 +44,16 @@ class SyncerCommandTestBase : public testing::Test, public sessions::SyncSession::Delegate { public: // SyncSession::Delegate implementation. - virtual void OnSilencedUntil( - const base::TimeTicks& silenced_until) OVERRIDE { + virtual void OnThrottled( + const base::TimeDelta& throttle_duration) OVERRIDE { FAIL() << "Should not get silenced."; } - virtual bool IsSyncingCurrentlySilenced() OVERRIDE { + virtual void OnTypesThrottled( + ModelTypeSet types, + const base::TimeDelta& throttle_duration) OVERRIDE { + FAIL() << "Should not get silenced."; + } + virtual bool IsCurrentlyThrottled() OVERRIDE { return false; } virtual void OnReceivedLongPollIntervalUpdate( @@ -125,11 +129,9 @@ class SyncerCommandTestBase : public testing::Test, } void ResetContext() { - throttled_data_type_tracker_.reset(new ThrottledDataTypeTracker(NULL)); context_.reset(new sessions::SyncSessionContext( mock_server_.get(), directory(), GetWorkers(), &extensions_activity_monitor_, - throttled_data_type_tracker_.get(), std::vector<SyncEngineEventListener*>(), &mock_debug_info_getter_, &traffic_recorder_, @@ -208,7 +210,6 @@ class SyncerCommandTestBase : public testing::Test, ModelSafeRoutingInfo routing_info_; NiceMock<MockDebugInfoGetter> mock_debug_info_getter_; FakeExtensionsActivityMonitor extensions_activity_monitor_; - scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_; TrafficRecorder traffic_recorder_; DISALLOW_COPY_AND_ASSIGN(SyncerCommandTestBase); }; |