From 6727a62ae541389ec4a5264b279485d75e2c4fe5 Mon Sep 17 00:00:00 2001 From: johnme Date: Thu, 7 May 2015 06:48:30 -0700 Subject: [PATCH] Allow BackoffEntry to be serialized and deserialized. This will be used by push messaging for https://crbug.com/465399, where we need to continue retrying unregistration even if the browser gets killed and restarted. BUG=465399 Review URL: https://codereview.chromium.org/1023473003 Cr-Commit-Position: refs/heads/master@{#328755} --- net/base/backoff_entry.cc | 23 ++-- net/base/backoff_entry.h | 15 ++- net/base/backoff_entry_serializer.cc | 92 ++++++++++++++ net/base/backoff_entry_serializer.h | 55 ++++++++ net/base/backoff_entry_serializer_unittest.cc | 174 ++++++++++++++++++++++++++ net/net.gypi | 3 + 6 files changed, 353 insertions(+), 9 deletions(-) create mode 100644 net/base/backoff_entry_serializer.cc create mode 100644 net/base/backoff_entry_serializer.h create mode 100644 net/base/backoff_entry_serializer_unittest.cc diff --git a/net/base/backoff_entry.cc b/net/base/backoff_entry.cc index bfcc9dd67d6b..870d6a156310 100644 --- a/net/base/backoff_entry.cc +++ b/net/base/backoff_entry.cc @@ -142,11 +142,24 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const { delay_ms -= base::RandDouble() * policy_->jitter_factor * delay_ms; // Do overflow checking in microseconds, the internal unit of TimeTicks. + base::internal::CheckedNumeric backoff_duration_us = delay_ms + 0.5; + backoff_duration_us *= base::Time::kMicrosecondsPerMillisecond; + base::TimeDelta backoff_duration = base::TimeDelta::FromMicroseconds( + backoff_duration_us.ValueOrDefault(kint64max)); + base::TimeTicks release_time = BackoffDurationToReleaseTime(backoff_duration); + + // Never reduce previously set release horizon, e.g. due to Retry-After + // header. + return std::max(release_time, exponential_backoff_release_time_); +} + +base::TimeTicks BackoffEntry::BackoffDurationToReleaseTime( + base::TimeDelta backoff_duration) const { const int64 kTimeTicksNowUs = (GetTimeTicksNow() - base::TimeTicks()).InMicroseconds(); + // Do overflow checking in microseconds, the internal unit of TimeTicks. base::internal::CheckedNumeric calculated_release_time_us = - delay_ms + 0.5; - calculated_release_time_us *= base::Time::kMicrosecondsPerMillisecond; + backoff_duration.InMicroseconds(); calculated_release_time_us += kTimeTicksNowUs; base::internal::CheckedNumeric maximum_release_time_us = kint64max; @@ -162,11 +175,7 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const { calculated_release_time_us.ValueOrDefault(kint64max), maximum_release_time_us.ValueOrDefault(kint64max)); - // Never reduce previously set release horizon, e.g. due to Retry-After - // header. - return std::max( - base::TimeTicks() + base::TimeDelta::FromMicroseconds(release_time_us), - exponential_backoff_release_time_); + return base::TimeTicks() + base::TimeDelta::FromMicroseconds(release_time_us); } base::TimeTicks BackoffEntry::GetTimeTicksNow() const { diff --git a/net/base/backoff_entry.h b/net/base/backoff_entry.h index dbc7488b7a4b..be9d510474e9 100644 --- a/net/base/backoff_entry.h +++ b/net/base/backoff_entry.h @@ -5,6 +5,7 @@ #ifndef NET_BASE_BACKOFF_ENTRY_H_ #define NET_BASE_BACKOFF_ENTRY_H_ +#include "base/macros.h" #include "base/threading/non_thread_safe.h" #include "base/time/time.h" #include "net/base/net_export.h" @@ -22,7 +23,8 @@ namespace net { // intended for reuse in various networking scenarios. class NET_EXPORT BackoffEntry : NON_EXPORTED_BASE(public base::NonThreadSafe) { public: - // The set of parameters that define a back-off policy. + // The set of parameters that define a back-off policy. When modifying this, + // increment SERIALIZATION_VERSION_NUMBER in backoff_entry_serializer.cc. struct Policy { // Number of initial errors (in sequence) to ignore before applying // exponential back-off rules. @@ -80,9 +82,15 @@ class NET_EXPORT BackoffEntry : NON_EXPORTED_BASE(public base::NonThreadSafe) { // state) will no longer reject requests. base::TimeTicks GetReleaseTime() const; - // Returns the time until a request can be sent. + // Returns the time until a request can be sent (will be zero if the release + // time is in the past). base::TimeDelta GetTimeUntilRelease() const; + // Converts |backoff_duration| to a release time, by adding it to + // GetTimeTicksNow(), limited by maximum_backoff_ms. + base::TimeTicks BackoffDurationToReleaseTime( + base::TimeDelta backoff_duration) const; + // Causes this object reject requests until the specified absolute time. // This can be used to e.g. implement support for a Retry-After header. void SetCustomReleaseTime(const base::TimeTicks& release_time); @@ -98,6 +106,9 @@ class NET_EXPORT BackoffEntry : NON_EXPORTED_BASE(public base::NonThreadSafe) { // Returns the failure count for this entry. int failure_count() const { return failure_count_; } + // Returns the TickClock passed in to the constructor. May be NULL. + base::TickClock* tick_clock() const { return clock_; } + private: // Calculates when requests should again be allowed through. base::TimeTicks CalculateReleaseTime() const; diff --git a/net/base/backoff_entry_serializer.cc b/net/base/backoff_entry_serializer.cc new file mode 100644 index 000000000000..a4ce27d6c448 --- /dev/null +++ b/net/base/backoff_entry_serializer.cc @@ -0,0 +1,92 @@ +// Copyright 2015 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 "net/base/backoff_entry_serializer.h" + +#include "base/strings/string_number_conversions.h" +#include "base/time/tick_clock.h" +#include "base/values.h" +#include "net/base/backoff_entry.h" + +namespace { +// Increment this number when changing the serialization format, to avoid old +// serialized values loaded from disk etc being misinterpreted. +const int kSerializationFormatVersion = 1; +} // namespace + +namespace net { + +scoped_ptr BackoffEntrySerializer::SerializeToValue( + const BackoffEntry& entry, base::Time time_now) { + scoped_ptr serialized(new base::ListValue()); + serialized->AppendInteger(kSerializationFormatVersion); + + serialized->AppendInteger(entry.failure_count()); + + // Can't use entry.GetTimeUntilRelease as it doesn't allow negative deltas. + base::TimeDelta backoff_duration = + entry.GetReleaseTime() - entry.tick_clock()->NowTicks(); + // Redundantly stores both the remaining time delta and the absolute time. + // The delta is used to work around some cases where wall clock time changes. + serialized->AppendDouble(backoff_duration.InSecondsF()); + base::Time absolute_release_time = backoff_duration + time_now; + serialized->AppendString( + base::Int64ToString(absolute_release_time.ToInternalValue())); + + return serialized.Pass(); +} + +scoped_ptr BackoffEntrySerializer::DeserializeFromValue( + const base::Value& serialized, const BackoffEntry::Policy* policy, + base::TickClock* tick_clock, base::Time time_now) { + const base::ListValue* serialized_list = nullptr; + if (!serialized.GetAsList(&serialized_list)) + return nullptr; + if (serialized_list->GetSize() != 4) + return nullptr; + int version_number; + if (!serialized_list->GetInteger(0, &version_number) || + version_number != kSerializationFormatVersion) { + return nullptr; + } + + int failure_count; + if (!serialized_list->GetInteger(1, &failure_count) || failure_count < 0) + return nullptr; + double original_backoff_duration_double; + if (!serialized_list->GetDouble(2, &original_backoff_duration_double)) + return nullptr; + std::string absolute_release_time_string; + if (!serialized_list->GetString(3, &absolute_release_time_string)) + return nullptr; + int64 absolute_release_time_us; + if (!base::StringToInt64(absolute_release_time_string, + &absolute_release_time_us) || + absolute_release_time_us < 0) { + return nullptr; + } + + scoped_ptr entry(new BackoffEntry(policy, tick_clock)); + + for (int n = 0; n < failure_count; n++) + entry->InformOfRequest(false); + + base::TimeDelta original_backoff_duration = + base::TimeDelta::FromSecondsD(original_backoff_duration_double); + base::Time absolute_release_time = + base::Time::FromInternalValue(absolute_release_time_us); + base::TimeDelta backoff_duration = absolute_release_time - time_now; + // In cases where the system wall clock is rewound, use the redundant + // original_backoff_duration to ensure the backoff duration isn't longer + // than it was before serializing (note that it's not possible to protect + // against the clock being wound forward). + if (backoff_duration > original_backoff_duration) + backoff_duration = original_backoff_duration; + entry->SetCustomReleaseTime( + entry->BackoffDurationToReleaseTime(backoff_duration)); + + return entry; +} + +} // namespace net diff --git a/net/base/backoff_entry_serializer.h b/net/base/backoff_entry_serializer.h new file mode 100644 index 000000000000..837265727fb4 --- /dev/null +++ b/net/base/backoff_entry_serializer.h @@ -0,0 +1,55 @@ +// Copyright 2015 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 NET_BASE_BACKOFF_ENTRY_SERIALIZER_H_ +#define NET_BASE_BACKOFF_ENTRY_SERIALIZER_H_ + +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" +#include "base/time/time.h" +#include "net/base/backoff_entry.h" +#include "net/base/net_export.h" + +namespace base { +class Value; +class TickClock; +} + +namespace net { + +// Serialize or deserialize a BackoffEntry, so it can persist beyond the +// lifetime of the browser. +class NET_EXPORT BackoffEntrySerializer { + public: + // Serializes the release time and failure count into a ListValue that can + // later be passed to Deserialize to re-create the given BackoffEntry. The + // Policy is not serialized, instead callers must pass an identical Policy* + // when deserializing. |time_now| should be base::Time::Now(), except for + // tests that want to simulate time changes. The release time TimeTicks will + // be converted to an absolute timestamp, thus the time will continue counting + // down even whilst the device is powered off, and will be partially + // vulnerable to changes in the system clock time. + static scoped_ptr SerializeToValue(const BackoffEntry& entry, + base::Time time_now); + + // Deserializes a ListValue back to a BackoffEntry. |policy| MUST be the same + // Policy as the serialized entry had. |clock| may be NULL. Both |policy| and + // |clock| (if not NULL) must enclose lifetime of the returned BackoffEntry. + // |time_now| should be base::Time::Now(), except for tests that want to + // simulate time changes. The absolute timestamp that was serialized will be + // converted back to TimeTicks as best as possible. Returns NULL if + // deserialization was unsuccessful. + static scoped_ptr DeserializeFromValue( + const base::Value& serialized, + const BackoffEntry::Policy* policy, + base::TickClock* clock, + base::Time time_now); + + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(BackoffEntrySerializer); +}; + +} // namespace net + +#endif // NET_BASE_BACKOFF_ENTRY_SERIALIZER_H_ diff --git a/net/base/backoff_entry_serializer_unittest.cc b/net/base/backoff_entry_serializer_unittest.cc new file mode 100644 index 000000000000..9799c06e717f --- /dev/null +++ b/net/base/backoff_entry_serializer_unittest.cc @@ -0,0 +1,174 @@ +// Copyright 2015 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 "net/base/backoff_entry.h" + +#include "base/logging.h" +#include "base/macros.h" +#include "base/time/tick_clock.h" +#include "base/values.h" +#include "net/base/backoff_entry_serializer.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { + +namespace { + +using base::Time; +using base::TimeDelta; +using base::TimeTicks; + +BackoffEntry::Policy base_policy = { + 0 /* num_errors_to_ignore */, + 1000 /* initial_delay_ms */, + 2.0 /* multiply_factor */, + 0.0 /* jitter_factor */, + 20000 /* maximum_backoff_ms */, + 2000 /* entry_lifetime_ms */, + false /* always_use_initial_delay */ +}; + +class TestTickClock : public base::TickClock { + public: + TestTickClock() {} + ~TestTickClock() override {} + + TimeTicks NowTicks() override { return now_ticks_; } + void set_now(TimeTicks now) { now_ticks_ = now; } + + private: + TimeTicks now_ticks_; + + DISALLOW_COPY_AND_ASSIGN(TestTickClock); +}; + +TEST(BackoffEntrySerializerTest, SerializeNoFailures) { + Time original_time = Time::Now(); + TestTickClock original_ticks; + original_ticks.set_now(TimeTicks::Now()); + BackoffEntry original(&base_policy, &original_ticks); + scoped_ptr serialized = + BackoffEntrySerializer::SerializeToValue(original, original_time); + + scoped_ptr deserialized = + BackoffEntrySerializer::DeserializeFromValue(*serialized, &base_policy, + &original_ticks, + original_time); + ASSERT_TRUE(deserialized.get()); + EXPECT_EQ(original.failure_count(), deserialized->failure_count()); + EXPECT_EQ(original.GetReleaseTime(), deserialized->GetReleaseTime()); +} + +TEST(BackoffEntrySerializerTest, SerializeTimeOffsets) { + Time original_time = Time::FromJsTime(1430907555111); // May 2015 for realism + TestTickClock original_ticks; + BackoffEntry original(&base_policy, &original_ticks); + // 2 errors. + original.InformOfRequest(false); + original.InformOfRequest(false); + scoped_ptr serialized = + BackoffEntrySerializer::SerializeToValue(original, original_time); + + { + // Test that immediate deserialization round-trips. + scoped_ptr deserialized = + BackoffEntrySerializer::DeserializeFromValue(*serialized, &base_policy, + &original_ticks, + original_time); + ASSERT_TRUE(deserialized.get()); + EXPECT_EQ(original.failure_count(), deserialized->failure_count()); + EXPECT_EQ(original.GetReleaseTime(), deserialized->GetReleaseTime()); + } + + { + // Test deserialization when wall clock has advanced but TimeTicks::Now() + // hasn't (e.g. device was rebooted). + Time later_time = original_time + TimeDelta::FromDays(1); + scoped_ptr deserialized = + BackoffEntrySerializer::DeserializeFromValue(*serialized, &base_policy, + &original_ticks, + later_time); + ASSERT_TRUE(deserialized.get()); + EXPECT_EQ(original.failure_count(), deserialized->failure_count()); + // Remaining backoff duration continues decreasing while device is off. + // Since TimeTicks::Now() has not advanced, the absolute release time ticks + // will decrease accordingly. + EXPECT_GT(original.GetTimeUntilRelease(), + deserialized->GetTimeUntilRelease()); + EXPECT_EQ(original.GetReleaseTime() - TimeDelta::FromDays(1), + deserialized->GetReleaseTime()); + } + + { + // Test deserialization when TimeTicks::Now() has advanced but wall clock + // hasn't (e.g. it's an hour later, but a DST change cancelled that out). + TestTickClock later_ticks; + later_ticks.set_now(TimeTicks() + TimeDelta::FromDays(1)); + scoped_ptr deserialized = + BackoffEntrySerializer::DeserializeFromValue(*serialized, &base_policy, + &later_ticks, + original_time); + ASSERT_TRUE(deserialized.get()); + EXPECT_EQ(original.failure_count(), deserialized->failure_count()); + // According to the wall clock, no time has passed. So remaining backoff + // duration is preserved, hence the absolute release time ticks increases. + // This isn't ideal - by also serializing the current time and time ticks, + // it would be possible to detect that time has passed but the wall clock + // went backwards, and reduce the remaining backoff duration accordingly, + // however the current implementation does not do this as the benefit would + // be somewhat marginal. + EXPECT_EQ(original.GetTimeUntilRelease(), + deserialized->GetTimeUntilRelease()); + EXPECT_EQ(original.GetReleaseTime() + TimeDelta::FromDays(1), + deserialized->GetReleaseTime()); + } + + { + // Test deserialization when both wall clock and TimeTicks::Now() have + // advanced (e.g. it's just later than it used to be). + TestTickClock later_ticks; + later_ticks.set_now(TimeTicks() + TimeDelta::FromDays(1)); + Time later_time = original_time + TimeDelta::FromDays(1); + scoped_ptr deserialized = + BackoffEntrySerializer::DeserializeFromValue(*serialized, &base_policy, + &later_ticks, later_time); + ASSERT_TRUE(deserialized.get()); + EXPECT_EQ(original.failure_count(), deserialized->failure_count()); + // Since both have advanced by the same amount, the absolute release time + // ticks should be preserved; the remaining backoff duration will have + // decreased of course, since time has passed. + EXPECT_GT(original.GetTimeUntilRelease(), + deserialized->GetTimeUntilRelease()); + EXPECT_EQ(original.GetReleaseTime(), deserialized->GetReleaseTime()); + } + + { + // Test deserialization when wall clock has gone backwards but TimeTicks + // haven't (e.g. the system clock was fast but they fixed it). + EXPECT_LT(TimeDelta::FromSeconds(1), original.GetTimeUntilRelease()); + Time earlier_time = original_time - TimeDelta::FromSeconds(1); + scoped_ptr deserialized = + BackoffEntrySerializer::DeserializeFromValue(*serialized, &base_policy, + &original_ticks, + earlier_time); + ASSERT_TRUE(deserialized.get()); + EXPECT_EQ(original.failure_count(), deserialized->failure_count()); + // If only the absolute wall clock time was serialized, subtracting the + // (decreased) current wall clock time from the serialized wall clock time + // could give very large (incorrect) values for remaining backoff duration. + // But instead the implementation also serializes the remaining backoff + // duration, and doesn't allow the duration to increase beyond it's previous + // value during deserialization. Hence when the wall clock goes backwards + // the remaining backoff duration will be preserved. + EXPECT_EQ(original.GetTimeUntilRelease(), + deserialized->GetTimeUntilRelease()); + // Since TimeTicks::Now() hasn't changed, the absolute release time ticks + // will be equal too in this particular case. + EXPECT_EQ(original.GetReleaseTime(), deserialized->GetReleaseTime()); + } +} + +} // namespace + +} // namespace net diff --git a/net/net.gypi b/net/net.gypi index f5610d7c1dba..227428115da0 100644 --- a/net/net.gypi +++ b/net/net.gypi @@ -199,6 +199,8 @@ 'base/address_tracker_linux.h', 'base/backoff_entry.cc', 'base/backoff_entry.h', + 'base/backoff_entry_serializer.cc', + 'base/backoff_entry_serializer.h', 'base/cache_type.h', 'base/chunked_upload_data_stream.cc', 'base/chunked_upload_data_stream.h', @@ -1275,6 +1277,7 @@ 'android/network_change_notifier_android_unittest.cc', 'base/address_list_unittest.cc', 'base/address_tracker_linux_unittest.cc', + 'base/backoff_entry_serializer_unittest.cc', 'base/backoff_entry_unittest.cc', 'base/chunked_upload_data_stream_unittest.cc', 'base/data_url_unittest.cc', -- 2.11.4.GIT