Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Unified Diff: net/base/backoff_entry.cc

Issue 1076853003: Refactor net::BackoffEntry to not require subclassing (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address pneubeck's review comments Created 5 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/base/backoff_entry.h ('k') | net/base/backoff_entry_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/base/backoff_entry.cc
diff --git a/net/base/backoff_entry.cc b/net/base/backoff_entry.cc
index 0b3a06f1d559e181aa5fdd1157834b0384482ee6..bfcc9dd67d6b16f6e1708aad5ff33e5740595d98 100644
--- a/net/base/backoff_entry.cc
+++ b/net/base/backoff_entry.cc
@@ -12,11 +12,16 @@
#include "base/logging.h"
#include "base/numerics/safe_math.h"
#include "base/rand_util.h"
+#include "base/time/tick_clock.h"
namespace net {
-BackoffEntry::BackoffEntry(const BackoffEntry::Policy* const policy)
- : policy_(policy) {
+BackoffEntry::BackoffEntry(const BackoffEntry::Policy* policy)
+ : BackoffEntry(policy, nullptr) {}
+
+BackoffEntry::BackoffEntry(const BackoffEntry::Policy* policy,
+ base::TickClock* clock)
+ : policy_(policy), clock_(clock) {
DCHECK(policy_);
Reset();
}
@@ -41,7 +46,7 @@ void BackoffEntry::InformOfRequest(bool succeeded) {
--failure_count_;
// The reason why we are not just cutting the release time to
- // ImplGetTimeNow() is on the one hand, it would unset a release
+ // GetTimeTicksNow() is on the one hand, it would unset a release
// time set by SetCustomReleaseTime and on the other we would like
// to push every request up to our "horizon" when dealing with
// multiple in-flight requests. Ex: If we send three requests and
@@ -53,16 +58,16 @@ void BackoffEntry::InformOfRequest(bool succeeded) {
if (policy_->always_use_initial_delay)
delay = base::TimeDelta::FromMilliseconds(policy_->initial_delay_ms);
exponential_backoff_release_time_ = std::max(
- ImplGetTimeNow() + delay, exponential_backoff_release_time_);
+ GetTimeTicksNow() + delay, exponential_backoff_release_time_);
}
}
bool BackoffEntry::ShouldRejectRequest() const {
- return exponential_backoff_release_time_ > ImplGetTimeNow();
+ return exponential_backoff_release_time_ > GetTimeTicksNow();
}
base::TimeDelta BackoffEntry::GetTimeUntilRelease() const {
- base::TimeTicks now = ImplGetTimeNow();
+ base::TimeTicks now = GetTimeTicksNow();
if (exponential_backoff_release_time_ <= now)
return base::TimeDelta();
return exponential_backoff_release_time_ - now;
@@ -80,7 +85,7 @@ bool BackoffEntry::CanDiscard() const {
if (policy_->entry_lifetime_ms == -1)
return false;
- base::TimeTicks now = ImplGetTimeNow();
+ base::TimeTicks now = GetTimeTicksNow();
int64 unused_since_ms =
(now - exponential_backoff_release_time_).InMilliseconds();
@@ -103,19 +108,13 @@ bool BackoffEntry::CanDiscard() const {
void BackoffEntry::Reset() {
failure_count_ = 0;
-
- // We leave exponential_backoff_release_time_ unset, meaning 0. We could
- // initialize to ImplGetTimeNow() but because it's a virtual method it's
- // not safe to call in the constructor (and the constructor calls Reset()).
- // The effects are the same, i.e. ShouldRejectRequest() will return false
- // right after Reset().
+ // For legacy reasons, we reset exponential_backoff_release_time_ to the
+ // uninitialized state. It would also be reasonable to reset it to
+ // GetTimeTicksNow(). The effects are the same, i.e. ShouldRejectRequest()
+ // will return false right after Reset().
exponential_backoff_release_time_ = base::TimeTicks();
}
-base::TimeTicks BackoffEntry::ImplGetTimeNow() const {
- return base::TimeTicks::Now();
-}
-
base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
int effective_failure_count =
std::max(0, failure_count_ - policy_->num_errors_to_ignore);
@@ -128,7 +127,7 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
if (effective_failure_count == 0) {
// Never reduce previously set release horizon, e.g. due to Retry-After
// header.
- return std::max(ImplGetTimeNow(), exponential_backoff_release_time_);
+ return std::max(GetTimeTicksNow(), exponential_backoff_release_time_);
}
// The delay is calculated with this formula:
@@ -144,7 +143,7 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
// Do overflow checking in microseconds, the internal unit of TimeTicks.
const int64 kTimeTicksNowUs =
- (ImplGetTimeNow() - base::TimeTicks()).InMicroseconds();
+ (GetTimeTicksNow() - base::TimeTicks()).InMicroseconds();
base::internal::CheckedNumeric<int64> calculated_release_time_us =
delay_ms + 0.5;
calculated_release_time_us *= base::Time::kMicrosecondsPerMillisecond;
@@ -170,4 +169,8 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
exponential_backoff_release_time_);
}
+base::TimeTicks BackoffEntry::GetTimeTicksNow() const {
+ return clock_ ? clock_->NowTicks() : base::TimeTicks::Now();
+}
+
} // namespace net
« no previous file with comments | « net/base/backoff_entry.h ('k') | net/base/backoff_entry_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698