summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
authorrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-06-14 19:45:30 +0000
committerrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-06-14 19:45:30 +0000
commitd9d8f9f150646e0adcb2948097cea99f89db41c4 (patch)
tree77437a172d1a057e8c465b20ec8ef517187dddde /sync
parent3c6de13983e12c3506fd53502fc92075b2595136 (diff)
downloadchromium_src-d9d8f9f150646e0adcb2948097cea99f89db41c4.zip
chromium_src-d9d8f9f150646e0adcb2948097cea99f89db41c4.tar.gz
chromium_src-d9d8f9f150646e0adcb2948097cea99f89db41c4.tar.bz2
sync: Handle type throttling in NudgeTracker
This change removes the ThrottledDataTypeTracker and moves much of its functionality into the NudgeTracker. This allows us to better control throttling behavior and fix crbug.com/155296. This CL re-routes syncer_proto_util type throttling callbacks through the SyncSession::Delegate to the SyncScheduler. It adds a timer and some related functions to the SyncScheduler, so the scheduler can now wake up, unthrottle types, and attempt a sync cycle at the exact time when throttling expires. The information about which types are currently throttled has been moved to the NudgeTracker. This allows the NudgeTracker to take type throttling into account in functions like NudgeTracker::IsSyncRequired() and NudgeTracker::RecordSuccessfulSyncCycle(). The DownloadUpdatesCommand's special case for nudge-type syncs has been extended to take throttling into account. GetCommitIdsCommand has been updated to fetch its list of throttled types from the nudge tracker. Unfortunately, this meant that committing from poll-triggered sync sessions had to be disabled, since poll sync cycles do not have access to the nudge tracker. BUG=155296, 116184 Review URL: https://chromiumcodereview.appspot.com/16402013 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206475 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r--sync/engine/all_status.cc8
-rw-r--r--sync/engine/all_status.h2
-rw-r--r--sync/engine/download_updates_command.cc11
-rw-r--r--sync/engine/get_commit_ids_command.cc10
-rw-r--r--sync/engine/sync_engine_event.h8
-rw-r--r--sync/engine/sync_scheduler_impl.cc115
-rw-r--r--sync/engine/sync_scheduler_impl.h22
-rw-r--r--sync/engine/sync_scheduler_unittest.cc42
-rw-r--r--sync/engine/syncer.cc3
-rw-r--r--sync/engine/syncer_proto_util.cc29
-rw-r--r--sync/engine/syncer_proto_util.h7
-rw-r--r--sync/engine/syncer_proto_util_unittest.cc29
-rw-r--r--sync/engine/syncer_unittest.cc30
-rw-r--r--sync/engine/throttled_data_type_tracker.cc72
-rw-r--r--sync/engine/throttled_data_type_tracker.h58
-rw-r--r--sync/engine/throttled_data_type_tracker_unittest.cc67
-rw-r--r--sync/internal_api/internal_components_factory_impl.cc3
-rw-r--r--sync/internal_api/public/internal_components_factory.h3
-rw-r--r--sync/internal_api/public/internal_components_factory_impl.h1
-rw-r--r--sync/internal_api/public/test/test_internal_components_factory.h1
-rw-r--r--sync/internal_api/sync_manager_impl.cc2
-rw-r--r--sync/internal_api/sync_manager_impl.h3
-rw-r--r--sync/internal_api/test/test_internal_components_factory.cc3
-rw-r--r--sync/sessions/data_type_tracker.cc39
-rw-r--r--sync/sessions/data_type_tracker.h24
-rw-r--r--sync/sessions/nudge_tracker.cc92
-rw-r--r--sync/sessions/nudge_tracker.h30
-rw-r--r--sync/sessions/nudge_tracker_unittest.cc180
-rw-r--r--sync/sessions/sync_session.cc2
-rw-r--r--sync/sessions/sync_session.h9
-rw-r--r--sync/sessions/sync_session_context.cc3
-rw-r--r--sync/sessions/sync_session_context.h8
-rw-r--r--sync/sessions/sync_session_unittest.cc15
-rw-r--r--sync/sessions/test_util.cc18
-rw-r--r--sync/sync_core.gypi2
-rw-r--r--sync/sync_tests.gypi1
-rw-r--r--sync/test/engine/fake_sync_scheduler.cc12
-rw-r--r--sync/test/engine/fake_sync_scheduler.h9
-rw-r--r--sync/test/engine/syncer_command_test.h15
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);
};