diff options
Diffstat (limited to 'sync/engine')
| -rw-r--r-- | sync/engine/backoff_delay_provider.cc | 24 | ||||
| -rw-r--r-- | sync/engine/backoff_delay_provider_unittest.cc | 8 |
2 files changed, 32 insertions, 0 deletions
diff --git a/sync/engine/backoff_delay_provider.cc b/sync/engine/backoff_delay_provider.cc index af70442..979a7b4db 100644 --- a/sync/engine/backoff_delay_provider.cc +++ b/sync/engine/backoff_delay_provider.cc @@ -61,8 +61,32 @@ TimeDelta BackoffDelayProvider::GetDelay(const base::TimeDelta& last_delay) { TimeDelta BackoffDelayProvider::GetInitialDelay( const sessions::ModelNeutralState& state) const { + // NETWORK_CONNECTION_UNAVAILABLE implies we did not even manage to hit the + // wire; the failure occurred locally. Note that if commit_result is *not* + // UNSET, this implies download_updates_result succeeded. Also note that + // last_get_key_result is coupled to last_download_updates_result in that + // they are part of the same GetUpdates request, so we only check if + // the download request is CONNECTION_UNAVAILABLE. + // + // TODO(tim): Should we treat NETWORK_IO_ERROR similarly? It's different + // from CONNECTION_UNAVAILABLE in that a request may well have succeeded + // in contacting the server (e.g we got a 200 back), but we failed + // trying to parse the response (actual content length != HTTP response + // header content length value). For now since we're considering + // merging this code to branches and I haven't audited all the + // NETWORK_IO_ERROR cases carefully, I'm going to target the fix + // very tightly (see bug chromium-os:35073). DIRECTORY_LOOKUP_FAILED is + // another example of something that shouldn't backoff, though the + // scheduler should probably be handling these cases differently. See + // the TODO(rlarocque) in ScheduleNextSync. + if (state.commit_result == NETWORK_CONNECTION_UNAVAILABLE || + state.last_download_updates_result == NETWORK_CONNECTION_UNAVAILABLE) { + return short_initial_backoff_; + } + 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 diff --git a/sync/engine/backoff_delay_provider_unittest.cc b/sync/engine/backoff_delay_provider_unittest.cc index 31ed515..4a9be53 100644 --- a/sync/engine/backoff_delay_provider_unittest.cc +++ b/sync/engine/backoff_delay_provider_unittest.cc @@ -45,6 +45,10 @@ TEST_F(BackoffDelayProviderTest, GetInitialDelay) { EXPECT_EQ(kInitialBackoffShortRetrySeconds, delay->GetInitialDelay(state).InSeconds()); + state.last_download_updates_result = NETWORK_CONNECTION_UNAVAILABLE; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); + state.last_download_updates_result = SERVER_RETURN_TRANSIENT_ERROR; EXPECT_EQ(kInitialBackoffRetrySeconds, delay->GetInitialDelay(state).InSeconds()); @@ -64,6 +68,10 @@ TEST_F(BackoffDelayProviderTest, GetInitialDelay) { state.commit_result = SERVER_RETURN_MIGRATION_DONE; EXPECT_EQ(kInitialBackoffShortRetrySeconds, delay->GetInitialDelay(state).InSeconds()); + + state.commit_result = NETWORK_CONNECTION_UNAVAILABLE; + EXPECT_EQ(kInitialBackoffShortRetrySeconds, + delay->GetInitialDelay(state).InSeconds()); } TEST_F(BackoffDelayProviderTest, GetInitialDelayWithOverride) { |