diff options
author | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-08-15 06:18:36 +0000 |
---|---|---|
committer | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-08-15 06:18:36 +0000 |
commit | f706df5706936983f97b4898997da87029787da2 (patch) | |
tree | a1315c180059844bf0bc6344452f18544da6c91f /sync/engine | |
parent | 639ec914f10c955f3b0c0c7dc2b1cdfd6b9f9527 (diff) | |
download | chromium_src-f706df5706936983f97b4898997da87029787da2.zip chromium_src-f706df5706936983f97b4898997da87029787da2.tar.gz chromium_src-f706df5706936983f97b4898997da87029787da2.tar.bz2 |
sync: add InternalComponentsFactory::Switches to simplify passing switches to internal components.
Cleans up backoff retry override code to use InternalComponentsFactory::Switches rather than
global bool hack.
Also puts keystore encryption flag atop new mechanism.
(TBR sky for new chrome_switch).
TBR=sky@chromium.org
BUG=142029, 139839
Review URL: https://chromiumcodereview.appspot.com/10837231
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151664 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/engine')
-rw-r--r-- | sync/engine/backoff_delay_provider.cc | 79 | ||||
-rw-r--r-- | sync/engine/backoff_delay_provider.h | 55 | ||||
-rw-r--r-- | sync/engine/backoff_delay_provider_unittest.cc | 103 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_impl.cc | 81 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_impl.h | 36 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_unittest.cc | 64 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_whitebox_unittest.cc | 7 |
7 files changed, 265 insertions, 160 deletions
diff --git a/sync/engine/backoff_delay_provider.cc b/sync/engine/backoff_delay_provider.cc new file mode 100644 index 0000000..af70442 --- /dev/null +++ b/sync/engine/backoff_delay_provider.cc @@ -0,0 +1,79 @@ +// 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/backoff_delay_provider.h" + +#include "base/rand_util.h" +#include "sync/internal_api/public/engine/polling_constants.h" +#include "sync/internal_api/public/sessions/model_neutral_state.h" +#include "sync/internal_api/public/util/syncer_error.h" + +using base::TimeDelta; + +namespace syncer { + +// static +BackoffDelayProvider* BackoffDelayProvider::FromDefaults() { + return new BackoffDelayProvider( + TimeDelta::FromSeconds(kInitialBackoffRetrySeconds), + TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds)); +} + +// static +BackoffDelayProvider* BackoffDelayProvider::WithShortInitialRetryOverride() { + return new BackoffDelayProvider( + TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds), + TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds)); +} + +BackoffDelayProvider::BackoffDelayProvider( + const base::TimeDelta& default_initial_backoff, + const base::TimeDelta& short_initial_backoff) + : default_initial_backoff_(default_initial_backoff), + short_initial_backoff_(short_initial_backoff) { +} + +BackoffDelayProvider::~BackoffDelayProvider() {} + +TimeDelta BackoffDelayProvider::GetDelay(const base::TimeDelta& last_delay) { + if (last_delay.InSeconds() >= kMaxBackoffSeconds) + return TimeDelta::FromSeconds(kMaxBackoffSeconds); + + // This calculates approx. base_delay_seconds * 2 +/- base_delay_seconds / 2 + int64 backoff_s = + std::max(static_cast<int64>(1), + last_delay.InSeconds() * kBackoffRandomizationFactor); + + // Flip a coin to randomize backoff interval by +/- 50%. + int rand_sign = base::RandInt(0, 1) * 2 - 1; + + // Truncation is adequate for rounding here. + backoff_s = backoff_s + + (rand_sign * (last_delay.InSeconds() / kBackoffRandomizationFactor)); + + // Cap the backoff interval. + backoff_s = std::max(static_cast<int64>(1), + std::min(backoff_s, kMaxBackoffSeconds)); + + return TimeDelta::FromSeconds(backoff_s); +} + +TimeDelta BackoffDelayProvider::GetInitialDelay( + const sessions::ModelNeutralState& state) const { + if (SyncerErrorIsError(state.last_get_key_result)) + return default_initial_backoff_; + // Note: If we received a MIGRATION_DONE on download updates, then commit + // should not have taken place. Moreover, if we receive a MIGRATION_DONE + // on commit, it means that download updates succeeded. Therefore, we only + // need to check if either code is equal to SERVER_RETURN_MIGRATION_DONE, + // and not if there were any more serious errors requiring the long retry. + if (state.last_download_updates_result == SERVER_RETURN_MIGRATION_DONE || + state.commit_result == SERVER_RETURN_MIGRATION_DONE) { + return short_initial_backoff_; + } + + return default_initial_backoff_; +} + +} // namespace syncer diff --git a/sync/engine/backoff_delay_provider.h b/sync/engine/backoff_delay_provider.h new file mode 100644 index 0000000..84293f9 --- /dev/null +++ b/sync/engine/backoff_delay_provider.h @@ -0,0 +1,55 @@ +// 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. + +#ifndef SYNC_ENGINE_BACKOFF_DELAY_PROVIDER_H_ +#define SYNC_ENGINE_BACKOFF_DELAY_PROVIDER_H_ + +#include "base/time.h" + +namespace syncer { + +namespace sessions { +struct ModelNeutralState; +} + +// A component used to get time delays associated with exponential backoff. +class BackoffDelayProvider { + public: + // Factory function to create a standard BackoffDelayProvider. + static BackoffDelayProvider* FromDefaults(); + + // Similar to above, but causes sync to retry very quickly (see + // polling_constants.h) when it encounters an error before exponential + // backoff. + // + // *** NOTE *** This should only be used if kSyncShortInitialRetryOverride + // was passed to command line. + static BackoffDelayProvider* WithShortInitialRetryOverride(); + + virtual ~BackoffDelayProvider(); + + // DDOS avoidance function. Calculates how long we should wait before trying + // again after a failed sync attempt, where the last delay was |base_delay|. + // TODO(tim): Look at URLRequestThrottlerEntryInterface. + virtual base::TimeDelta GetDelay(const base::TimeDelta& last_delay); + + // Helper to calculate the initial value for exponential backoff. + // See possible values and comments in polling_constants.h. + virtual base::TimeDelta GetInitialDelay( + const sessions::ModelNeutralState& state) const; + + protected: + BackoffDelayProvider(const base::TimeDelta& default_initial_backoff, + const base::TimeDelta& short_initial_backoff); + + private: + const base::TimeDelta default_initial_backoff_; + const base::TimeDelta short_initial_backoff_; + + DISALLOW_COPY_AND_ASSIGN(BackoffDelayProvider); +}; + +} // namespace syncer + +#endif // SYNC_ENGINE_BACKOFF_DELAY_PROVIDER_H_ diff --git a/sync/engine/backoff_delay_provider_unittest.cc b/sync/engine/backoff_delay_provider_unittest.cc new file mode 100644 index 0000000..31ed515 --- /dev/null +++ b/sync/engine/backoff_delay_provider_unittest.cc @@ -0,0 +1,103 @@ +// 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/backoff_delay_provider.h" + +#include "base/memory/scoped_ptr.h" +#include "base/time.h" +#include "sync/internal_api/public/engine/polling_constants.h" +#include "sync/internal_api/public/sessions/model_neutral_state.h" +#include "sync/internal_api/public/util/syncer_error.h" +#include "testing/gtest/include/gtest/gtest.h" + +using base::TimeDelta; + +namespace syncer { + +class BackoffDelayProviderTest : public testing::Test {}; + +TEST_F(BackoffDelayProviderTest, GetRecommendedDelay) { + scoped_ptr<BackoffDelayProvider> delay(BackoffDelayProvider::FromDefaults()); + EXPECT_LE(TimeDelta::FromSeconds(0), + delay->GetDelay(TimeDelta::FromSeconds(0))); + EXPECT_LE(TimeDelta::FromSeconds(1), + delay->GetDelay(TimeDelta::FromSeconds(1))); + EXPECT_LE(TimeDelta::FromSeconds(50), + delay->GetDelay(TimeDelta::FromSeconds(50))); + EXPECT_LE(TimeDelta::FromSeconds(10), + delay->GetDelay(TimeDelta::FromSeconds(10))); + EXPECT_EQ(TimeDelta::FromSeconds(kMaxBackoffSeconds), + delay->GetDelay(TimeDelta::FromSeconds(kMaxBackoffSeconds))); + EXPECT_EQ(TimeDelta::FromSeconds(kMaxBackoffSeconds), + delay->GetDelay(TimeDelta::FromSeconds(kMaxBackoffSeconds + 1))); +} + +TEST_F(BackoffDelayProviderTest, GetInitialDelay) { + scoped_ptr<BackoffDelayProvider> delay(BackoffDelayProvider::FromDefaults()); + sessions::ModelNeutralState state; + state.last_get_key_result = SYNC_SERVER_ERROR; + EXPECT_EQ(kInitialBackoffRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_get_key_result = UNSET; + state.last_download_updates_result = SERVER_RETURN_MIGRATION_DONE; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_download_updates_result = SERVER_RETURN_TRANSIENT_ERROR; + EXPECT_EQ(kInitialBackoffRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_download_updates_result = SERVER_RESPONSE_VALIDATION_FAILED; + EXPECT_EQ(kInitialBackoffRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_download_updates_result = SYNCER_OK; + // Note that updating credentials triggers a canary job, trumping + // the initial delay, but in theory we still expect this function to treat + // it like any other error in the system (except migration). + state.commit_result = SERVER_RETURN_INVALID_CREDENTIAL; + EXPECT_EQ(kInitialBackoffRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.commit_result = SERVER_RETURN_MIGRATION_DONE; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); +} + +TEST_F(BackoffDelayProviderTest, GetInitialDelayWithOverride) { + scoped_ptr<BackoffDelayProvider> delay( + BackoffDelayProvider::WithShortInitialRetryOverride()); + sessions::ModelNeutralState state; + state.last_get_key_result = SYNC_SERVER_ERROR; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_get_key_result = UNSET; + state.last_download_updates_result = SERVER_RETURN_MIGRATION_DONE; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_download_updates_result = SERVER_RETURN_TRANSIENT_ERROR; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_download_updates_result = SERVER_RESPONSE_VALIDATION_FAILED; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.last_download_updates_result = SYNCER_OK; + // Note that updating credentials triggers a canary job, trumping + // the initial delay, but in theory we still expect this function to treat + // it like any other error in the system (except migration). + state.commit_result = SERVER_RETURN_INVALID_CREDENTIAL; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + + state.commit_result = SERVER_RETURN_MIGRATION_DONE; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); +} + +} // namespace syncer diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 6db3260..ff177b4 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -12,7 +12,7 @@ #include "base/location.h" #include "base/logging.h" #include "base/message_loop.h" -#include "base/rand_util.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" @@ -32,11 +32,6 @@ using sync_pb::GetUpdatesCallerInfo; namespace { -// For integration tests only. Override initial backoff value. -// TODO(tim): Remove this egregiousness, use command line flag and plumb -// through. Done this way to reduce diffs in hotfix. -static bool g_force_short_retry = false; - bool ShouldRequestEarlyExit(const SyncProtocolError& error) { switch (error.error_type) { case SYNC_SUCCESS: @@ -85,9 +80,6 @@ ConfigurationParams::ConfigurationParams( } ConfigurationParams::~ConfigurationParams() {} -SyncSchedulerImpl::DelayProvider::DelayProvider() {} -SyncSchedulerImpl::DelayProvider::~DelayProvider() {} - SyncSchedulerImpl::WaitInterval::WaitInterval() : mode(UNKNOWN), had_nudge(false) { @@ -140,11 +132,6 @@ const char* SyncSchedulerImpl::SyncSessionJob::GetPurposeString( return ""; } -TimeDelta SyncSchedulerImpl::DelayProvider::GetDelay( - const base::TimeDelta& last_delay) { - return SyncSchedulerImpl::GetRecommendedDelay(last_delay); -} - GetUpdatesCallerInfo::GetUpdatesSource GetUpdatesFromNudgeSource( NudgeSource source) { switch (source) { @@ -197,6 +184,7 @@ bool IsConfigRelatedUpdateSourceValue( } // namespace SyncSchedulerImpl::SyncSchedulerImpl(const std::string& name, + BackoffDelayProvider* delay_provider, sessions::SyncSessionContext* context, Syncer* syncer) : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), @@ -216,7 +204,7 @@ SyncSchedulerImpl::SyncSchedulerImpl(const std::string& name, // Start with assuming everything is fine with the connection. // At the end of the sync cycle we would have the correct status. connection_code_(HttpResponse::SERVER_CONNECTION_OK), - delay_provider_(new DelayProvider()), + delay_provider_(delay_provider), syncer_(syncer), session_context_(context) { DCHECK(sync_loop_); @@ -903,43 +891,6 @@ void SyncSchedulerImpl::RestartWaiting() { this, &SyncSchedulerImpl::DoCanaryJob); } -namespace { -// TODO(tim): Move this function to syncer_error.h. -// Return true if the command in question was attempted and did not complete -// successfully. -bool IsError(SyncerError error) { - return error != UNSET && error != SYNCER_OK; -} -} // namespace - -// static -void SyncSchedulerImpl::ForceShortInitialBackoffRetry() { - g_force_short_retry = true; -} - -TimeDelta SyncSchedulerImpl::GetInitialBackoffDelay( - const sessions::ModelNeutralState& state) const { - // TODO(tim): Remove this, provide integration-test-only mechanism - // for override. - if (g_force_short_retry) { - return TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds); - } - - if (IsError(state.last_get_key_result)) - return TimeDelta::FromSeconds(kInitialBackoffRetrySeconds); - // Note: If we received a MIGRATION_DONE on download updates, then commit - // should not have taken place. Moreover, if we receive a MIGRATION_DONE - // on commit, it means that download updates succeeded. Therefore, we only - // need to check if either code is equal to SERVER_RETURN_MIGRATION_DONE, - // and not if there were any more serious errors requiring the long retry. - if (state.last_download_updates_result == SERVER_RETURN_MIGRATION_DONE || - state.commit_result == SERVER_RETURN_MIGRATION_DONE) { - return TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds); - } - - return TimeDelta::FromSeconds(kInitialBackoffRetrySeconds); -} - void SyncSchedulerImpl::HandleContinuationError( const SyncSessionJob& old_job) { DCHECK_EQ(MessageLoop::current(), sync_loop_); @@ -951,7 +902,7 @@ void SyncSchedulerImpl::HandleContinuationError( TimeDelta length = delay_provider_->GetDelay( IsBackingOff() ? wait_interval_->length : - GetInitialBackoffDelay( + delay_provider_->GetInitialDelay( old_job.session->status_controller().model_neutral_state())); SDVLOG(2) << "In handle continuation error with " @@ -984,30 +935,6 @@ void SyncSchedulerImpl::HandleContinuationError( RestartWaiting(); } -// static -TimeDelta SyncSchedulerImpl::GetRecommendedDelay(const TimeDelta& last_delay) { - if (last_delay.InSeconds() >= kMaxBackoffSeconds) - return TimeDelta::FromSeconds(kMaxBackoffSeconds); - - // This calculates approx. base_delay_seconds * 2 +/- base_delay_seconds / 2 - int64 backoff_s = - std::max(static_cast<int64>(1), - last_delay.InSeconds() * kBackoffRandomizationFactor); - - // Flip a coin to randomize backoff interval by +/- 50%. - int rand_sign = base::RandInt(0, 1) * 2 - 1; - - // Truncation is adequate for rounding here. - backoff_s = backoff_s + - (rand_sign * (last_delay.InSeconds() / kBackoffRandomizationFactor)); - - // Cap the backoff interval. - backoff_s = std::max(static_cast<int64>(1), - std::min(backoff_s, kMaxBackoffSeconds)); - - return TimeDelta::FromSeconds(backoff_s); -} - void SyncSchedulerImpl::RequestStop(const base::Closure& callback) { syncer_->RequestEarlyExit(); // Safe to call from any thread. DCHECK(weak_handle_this_.IsInitialized()); diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h index 586ccdc..e5f3acf 100644 --- a/sync/engine/sync_scheduler_impl.h +++ b/sync/engine/sync_scheduler_impl.h @@ -28,12 +28,16 @@ namespace syncer { +class BackoffDelayProvider; + class SyncSchedulerImpl : public SyncScheduler { public: // |name| is a display string to identify the syncer thread. Takes - // |ownership of |syncer|. + // |ownership of |syncer| and |delay_provider|. SyncSchedulerImpl(const std::string& name, - sessions::SyncSessionContext* context, Syncer* syncer); + BackoffDelayProvider* delay_provider, + sessions::SyncSessionContext* context, + Syncer* syncer); // Calls Stop(). virtual ~SyncSchedulerImpl(); @@ -72,16 +76,6 @@ class SyncSchedulerImpl : public SyncScheduler { virtual void OnSyncProtocolError( const sessions::SyncSessionSnapshot& snapshot) OVERRIDE; - // DDOS avoidance function. Calculates how long we should wait before trying - // again after a failed sync attempt, where the last delay was |base_delay|. - // TODO(tim): Look at URLRequestThrottlerEntryInterface. - static base::TimeDelta GetRecommendedDelay(const base::TimeDelta& base_delay); - - // For integration tests only. Override initial backoff value. - // TODO(tim): Remove this, use command line flag and plumb through. Done - // this way to reduce diffs in hotfix. - static void ForceShortInitialBackoffRetry(); - private: enum JobProcessDecision { // Indicates we should continue with the current job. @@ -150,17 +144,6 @@ class SyncSchedulerImpl : public SyncScheduler { FRIEND_TEST_ALL_PREFIXES(SyncSchedulerTest, TransientPollFailure); FRIEND_TEST_ALL_PREFIXES(SyncSchedulerTest, GetInitialBackoffDelay); - // A component used to get time delays associated with exponential backoff. - // Encapsulated into a class to facilitate testing. - class DelayProvider { - public: - DelayProvider(); - virtual base::TimeDelta GetDelay(const base::TimeDelta& last_delay); - virtual ~DelayProvider(); - private: - DISALLOW_COPY_AND_ASSIGN(DelayProvider); - }; - struct WaitInterval { enum Mode { // Uninitialized state, should not be set in practice. @@ -234,11 +217,6 @@ class SyncSchedulerImpl : public SyncScheduler { // Helper to ScheduleNextSync in case of consecutive sync errors. void HandleContinuationError(const SyncSessionJob& old_job); - // Helper to calculate the initial value for exponential backoff. - // See possible values and comments in polling_constants.h. - base::TimeDelta GetInitialBackoffDelay( - const sessions::ModelNeutralState& state) const; - // Determines if it is legal to run |job| by checking current // operational mode, backoff or throttling, freshness // (so we don't make redundant syncs), and connection. @@ -348,7 +326,7 @@ class SyncSchedulerImpl : public SyncScheduler { // Current wait state. Null if we're not in backoff and not throttled. scoped_ptr<WaitInterval> wait_interval_; - scoped_ptr<DelayProvider> delay_provider_; + scoped_ptr<BackoffDelayProvider> delay_provider_; // Invoked to run through the sync cycle. scoped_ptr<Syncer> syncer_; diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index 0420bcb..53d2ed4 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -8,6 +8,7 @@ #include "base/memory/weak_ptr.h" #include "base/message_loop.h" #include "base/test/test_timeouts.h" +#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" @@ -88,8 +89,13 @@ class SyncSchedulerTest : public testing::Test { syncer_(NULL), delay_(NULL) {} - class MockDelayProvider : public SyncSchedulerImpl::DelayProvider { + class MockDelayProvider : public BackoffDelayProvider { public: + MockDelayProvider() : BackoffDelayProvider( + TimeDelta::FromSeconds(kInitialBackoffRetrySeconds), + TimeDelta::FromSeconds(kInitialBackoffShortRetrySeconds)) { + } + MOCK_METHOD1(GetDelay, TimeDelta(const TimeDelta&)); }; @@ -126,7 +132,10 @@ class SyncSchedulerTest : public testing::Test { context_->set_notifications_enabled(true); context_->set_account_name("Test"); scheduler_.reset( - new SyncSchedulerImpl("TestSyncScheduler", context(), syncer_)); + new SyncSchedulerImpl("TestSyncScheduler", + BackoffDelayProvider::FromDefaults(), + context(), + syncer_)); } SyncSchedulerImpl* scheduler() { return scheduler_.get(); } @@ -959,38 +968,6 @@ TEST_F(SyncSchedulerTest, BackoffElevation) { EXPECT_GE(r.times[4] - r.times[3], fifth); } -TEST_F(SyncSchedulerTest, GetInitialBackoffDelay) { - sessions::ModelNeutralState state; - state.last_get_key_result = SYNC_SERVER_ERROR; - EXPECT_EQ(kInitialBackoffRetrySeconds, - scheduler()->GetInitialBackoffDelay(state).InSeconds()); - - state.last_get_key_result = UNSET; - state.last_download_updates_result = SERVER_RETURN_MIGRATION_DONE; - EXPECT_EQ(kInitialBackoffShortRetrySeconds, - scheduler()->GetInitialBackoffDelay(state).InSeconds()); - - state.last_download_updates_result = SERVER_RETURN_TRANSIENT_ERROR; - EXPECT_EQ(kInitialBackoffRetrySeconds, - scheduler()->GetInitialBackoffDelay(state).InSeconds()); - - state.last_download_updates_result = SERVER_RESPONSE_VALIDATION_FAILED; - EXPECT_EQ(kInitialBackoffRetrySeconds, - scheduler()->GetInitialBackoffDelay(state).InSeconds()); - - state.last_download_updates_result = SYNCER_OK; - // Note that updating credentials triggers a canary job, trumping - // the initial delay, but in theory we still expect this function to treat - // it like any other error in the system (except migration). - state.commit_result = SERVER_RETURN_INVALID_CREDENTIAL; - EXPECT_EQ(kInitialBackoffRetrySeconds, - scheduler()->GetInitialBackoffDelay(state).InSeconds()); - - state.commit_result = SERVER_RETURN_MIGRATION_DONE; - EXPECT_EQ(kInitialBackoffShortRetrySeconds, - scheduler()->GetInitialBackoffDelay(state).InSeconds()); -} - // Test that things go back to normal once a retry makes forward progress. TEST_F(SyncSchedulerTest, BackoffRelief) { SyncShareRecords r; @@ -1066,25 +1043,6 @@ TEST_F(SyncSchedulerTest, TransientPollFailure) { EXPECT_FALSE(scheduler()->IsBackingOff()); } -TEST_F(SyncSchedulerTest, GetRecommendedDelay) { - EXPECT_LE(TimeDelta::FromSeconds(0), - SyncSchedulerImpl::GetRecommendedDelay(TimeDelta::FromSeconds(0))); - EXPECT_LE(TimeDelta::FromSeconds(1), - SyncSchedulerImpl::GetRecommendedDelay(TimeDelta::FromSeconds(1))); - EXPECT_LE(TimeDelta::FromSeconds(50), - SyncSchedulerImpl::GetRecommendedDelay( - TimeDelta::FromSeconds(50))); - EXPECT_LE(TimeDelta::FromSeconds(10), - SyncSchedulerImpl::GetRecommendedDelay( - TimeDelta::FromSeconds(10))); - EXPECT_EQ(TimeDelta::FromSeconds(kMaxBackoffSeconds), - SyncSchedulerImpl::GetRecommendedDelay( - TimeDelta::FromSeconds(kMaxBackoffSeconds))); - EXPECT_EQ(TimeDelta::FromSeconds(kMaxBackoffSeconds), - SyncSchedulerImpl::GetRecommendedDelay( - TimeDelta::FromSeconds(kMaxBackoffSeconds + 1))); -} - // Test that appropriate syncer steps are requested for each job type. TEST_F(SyncSchedulerTest, SyncerSteps) { // Nudges. diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc index f6f88c2..afc8c6f 100644 --- a/sync/engine/sync_scheduler_whitebox_unittest.cc +++ b/sync/engine/sync_scheduler_whitebox_unittest.cc @@ -4,8 +4,10 @@ #include "base/message_loop.h" #include "base/time.h" +#include "sync/engine/backoff_delay_provider.h" #include "sync/engine/sync_scheduler_impl.h" #include "sync/engine/throttled_data_type_tracker.h" +#include "sync/internal_api/public/engine/polling_constants.h" #include "sync/sessions/sync_session_context.h" #include "sync/sessions/test_util.h" #include "sync/test/engine/fake_model_worker.h" @@ -55,7 +57,10 @@ class SyncSchedulerWhiteboxTest : public testing::Test { context_->set_notifications_enabled(true); context_->set_account_name("Test"); scheduler_.reset( - new SyncSchedulerImpl("TestSyncSchedulerWhitebox", context(), syncer)); + new SyncSchedulerImpl("TestSyncSchedulerWhitebox", + BackoffDelayProvider::FromDefaults(), + context(), + syncer)); } virtual void TearDown() { |