diff options
author | joi@chromium.org <joi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-27 18:51:53 +0000 |
---|---|---|
committer | joi@chromium.org <joi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-27 18:51:53 +0000 |
commit | 82c71ba49a999ffa2b9c57aefc664dfd9bbe8186 (patch) | |
tree | a119d96375aa37d2609cfc911ed5a9c84c594f4b /net | |
parent | 4d768302d3e793d3b717bedea907b6038e391bff (diff) | |
download | chromium_src-82c71ba49a999ffa2b9c57aefc664dfd9bbe8186.zip chromium_src-82c71ba49a999ffa2b9c57aefc664dfd9bbe8186.tar.gz chromium_src-82c71ba49a999ffa2b9c57aefc664dfd9bbe8186.tar.bz2 |
Anti-DDoS enhancements: Log to net log, UMA stats, improved policy.
Count only certain error codes as failures. Don't count malformed body
for codes already counted failures. Start with fresh entry when
outdated, which makes policy slightly less aggressive.
Add registration/unregistration to NetworkChangeNotifier, and reset
throttling state on network changes, to avoid degenerate corner cases.
Add UMA stats for throttling, and unit tests for them.
Add hash-location-only URL to unit test.
Enable testing of AJAX behavior via backoff_server.
BUG=83775
TEST=net_unittests
Review URL: http://codereview.chromium.org/6966038
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@87056 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/base/backoff_entry.cc | 50 | ||||
-rw-r--r-- | net/base/backoff_entry.h | 9 | ||||
-rw-r--r-- | net/base/backoff_entry_unittest.cc | 19 | ||||
-rw-r--r-- | net/base/net_log_event_type_list.h | 26 | ||||
-rw-r--r-- | net/base/net_log_source_type_list.h | 5 | ||||
-rw-r--r-- | net/tools/testserver/backoff_server.py | 71 | ||||
-rw-r--r-- | net/url_request/url_request_throttler_entry.cc | 224 | ||||
-rw-r--r-- | net/url_request/url_request_throttler_entry.h | 32 | ||||
-rw-r--r-- | net/url_request/url_request_throttler_entry_interface.h | 7 | ||||
-rw-r--r-- | net/url_request/url_request_throttler_manager.cc | 78 | ||||
-rw-r--r-- | net/url_request/url_request_throttler_manager.h | 34 | ||||
-rw-r--r-- | net/url_request/url_request_throttler_unittest.cc | 133 |
12 files changed, 607 insertions, 81 deletions
diff --git a/net/base/backoff_entry.cc b/net/base/backoff_entry.cc index e7b7b55..5fcaed9 100644 --- a/net/base/backoff_entry.cc +++ b/net/base/backoff_entry.cc @@ -13,12 +13,9 @@ namespace net { BackoffEntry::BackoffEntry(const BackoffEntry::Policy* const policy) - : failure_count_(0), - policy_(policy) { + : policy_(policy) { DCHECK(policy_); - - // Can't use GetTimeNow() as it's virtual. - exponential_backoff_release_time_ = base::TimeTicks::Now(); + Reset(); } BackoffEntry::~BackoffEntry() { @@ -42,21 +39,22 @@ void BackoffEntry::InformOfRequest(bool succeeded) { if (failure_count_ > 0) --failure_count_; - // The reason why we are not just cutting the release time to GetTimeNow() - // 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 we receive 2 failures and - // 1 success. The success that follows those failures will not reset the - // release time, further requests will then need to wait the delay caused - // by the 2 failures. + // The reason why we are not just cutting the release time to + // ImplGetTimeNow() 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 + // we receive 2 failures and 1 success. The success that follows + // those failures will not reset the release time, further + // requests will then need to wait the delay caused by the 2 + // failures. exponential_backoff_release_time_ = std::max( - GetTimeNow(), exponential_backoff_release_time_); + ImplGetTimeNow(), exponential_backoff_release_time_); } } bool BackoffEntry::ShouldRejectRequest() const { - return exponential_backoff_release_time_ > GetTimeNow(); + return exponential_backoff_release_time_ > ImplGetTimeNow(); } base::TimeTicks BackoffEntry::GetReleaseTime() const { @@ -71,7 +69,7 @@ bool BackoffEntry::CanDiscard() const { if (policy_->entry_lifetime_ms == -1) return false; - base::TimeTicks now = GetTimeNow(); + base::TimeTicks now = ImplGetTimeNow(); int64 unused_since_ms = (now - exponential_backoff_release_time_).InMilliseconds(); @@ -92,7 +90,18 @@ bool BackoffEntry::CanDiscard() const { return unused_since_ms >= policy_->entry_lifetime_ms; } -base::TimeTicks BackoffEntry::GetTimeNow() 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(). + exponential_backoff_release_time_ = base::TimeTicks(); +} + +base::TimeTicks BackoffEntry::ImplGetTimeNow() const { return base::TimeTicks::Now(); } @@ -102,7 +111,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(GetTimeNow(), exponential_backoff_release_time_); + return std::max(ImplGetTimeNow(), exponential_backoff_release_time_); } // The delay is calculated with this formula: @@ -119,8 +128,9 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const { // Never reduce previously set release horizon, e.g. due to Retry-After // header. - return std::max(GetTimeNow() + base::TimeDelta::FromMilliseconds(delay_int), - exponential_backoff_release_time_); + return std::max( + ImplGetTimeNow() + base::TimeDelta::FromMilliseconds(delay_int), + exponential_backoff_release_time_); } } // namespace net diff --git a/net/base/backoff_entry.h b/net/base/backoff_entry.h index 1a7d2dc..a3bca91 100644 --- a/net/base/backoff_entry.h +++ b/net/base/backoff_entry.h @@ -69,10 +69,15 @@ class NET_TEST BackoffEntry : NON_EXPORTED_BASE(public base::NonThreadSafe) { // had for Policy::entry_lifetime_ms_. bool CanDiscard() const; + // Resets this entry to a fresh (as if just constructed) state. + void Reset(); + + // Returns the failure count for this entry. + int failure_count() const { return failure_count_; } + protected: // Equivalent to TimeTicks::Now(), virtual so unit tests can override. - // TODO(joi): Switch to constructor-time dependency injection? - virtual base::TimeTicks GetTimeNow() const; + virtual base::TimeTicks ImplGetTimeNow() const; private: // Calculates when requests should again be allowed through. diff --git a/net/base/backoff_entry_unittest.cc b/net/base/backoff_entry_unittest.cc index cad40a6..320c9e3 100644 --- a/net/base/backoff_entry_unittest.cc +++ b/net/base/backoff_entry_unittest.cc @@ -25,7 +25,7 @@ class TestBackoffEntry : public BackoffEntry { virtual ~TestBackoffEntry() {} - virtual TimeTicks GetTimeNow() const { + virtual TimeTicks ImplGetTimeNow() const OVERRIDE { return now_; } @@ -108,29 +108,30 @@ TEST(BackoffEntryTest, ReleaseTimeCalculation) { // With zero errors, should return "now". TimeTicks result = entry.GetReleaseTime(); - EXPECT_EQ(entry.GetTimeNow(), result); + EXPECT_EQ(entry.ImplGetTimeNow(), result); // 1 error. entry.InformOfRequest(false); result = entry.GetReleaseTime(); - EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(1000), result); + EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(1000), result); // 2 errors. entry.InformOfRequest(false); result = entry.GetReleaseTime(); - EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(2000), result); + EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(2000), result); // 3 errors. entry.InformOfRequest(false); result = entry.GetReleaseTime(); - EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(4000), result); + EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(4000), result); // 6 errors (to check it doesn't pass maximum). entry.InformOfRequest(false); entry.InformOfRequest(false); entry.InformOfRequest(false); result = entry.GetReleaseTime(); - EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(20000), result); + EXPECT_EQ( + entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(20000), result); } TEST(BackoffEntryTest, ReleaseTimeCalculationWithJitter) { @@ -144,8 +145,10 @@ TEST(BackoffEntryTest, ReleaseTimeCalculationWithJitter) { entry.InformOfRequest(false); entry.InformOfRequest(false); TimeTicks result = entry.GetReleaseTime(); - EXPECT_LE(entry.GetTimeNow() + TimeDelta::FromMilliseconds(3200), result); - EXPECT_GE(entry.GetTimeNow() + TimeDelta::FromMilliseconds(4000), result); + EXPECT_LE( + entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(3200), result); + EXPECT_GE( + entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(4000), result); } } diff --git a/net/base/net_log_event_type_list.h b/net/base/net_log_event_type_list.h index 8d7b20a..5d9bf22 100644 --- a/net/base/net_log_event_type_list.h +++ b/net/base/net_log_event_type_list.h @@ -1019,3 +1019,29 @@ EVENT_TYPE(APPCACHE_DELIVERING_ERROR_RESPONSE) // This event is emitted whenever NetworkChangeNotifier determines that the // underlying network has changed. EVENT_TYPE(NETWORK_IP_ADDRESSES_CHANGED) + +// ------------------------------------------------------------------------ +// Exponential back-off throttling events +// ------------------------------------------------------------------------ + +// Emitted when back-off is disabled for a given host, or the first time +// a localhost URL is used (back-off is always disabled for localhost). +// { +// "host": <The hostname back-off was disabled for> +// } +EVENT_TYPE(THROTTLING_DISABLED_FOR_HOST) + +// Emitted when a request is denied due to exponential back-off throttling. +// { +// "url": <URL that was being requested>, +// "num_failures": <Failure count for the URL>, +// "release_after_ms": <Number of milliseconds until URL will be unblocked> +// } +EVENT_TYPE(THROTTLING_REJECTED_REQUEST) + +// Emitted when throttling entry receives an X-Retry-After header. +// { +// "url": <URL that was being requested>, +// "retry_after_ms": <Milliseconds until retry-after expires> +// } +EVENT_TYPE(THROTTLING_GOT_CUSTOM_RETRY_AFTER) diff --git a/net/base/net_log_source_type_list.h b/net/base/net_log_source_type_list.h index d5fed96..e0af952 100644 --- a/net/base/net_log_source_type_list.h +++ b/net/base/net_log_source_type_list.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -18,5 +18,6 @@ SOURCE_TYPE(HOST_RESOLVER_IMPL_JOB, 8) SOURCE_TYPE(DISK_CACHE_ENTRY, 9) SOURCE_TYPE(MEMORY_CACHE_ENTRY, 10) SOURCE_TYPE(HTTP_STREAM_JOB, 11) +SOURCE_TYPE(EXPONENTIAL_BACKOFF_THROTTLING, 12) -SOURCE_TYPE(COUNT, 12) // Always keep this as the last entry. +SOURCE_TYPE(COUNT, 13) // Always keep this as the last entry. diff --git a/net/tools/testserver/backoff_server.py b/net/tools/testserver/backoff_server.py index 4188cd1..783fd83 100644 --- a/net/tools/testserver/backoff_server.py +++ b/net/tools/testserver/backoff_server.py @@ -13,8 +13,50 @@ import sys import urlparse
+AJAX_TEST_PAGE = '''
+<html>
+<head>
+<script>
+
+function reportResult(txt) {
+ var element = document.createElement('p');
+ element.innerHTML = txt;
+ document.body.appendChild(element);
+}
+
+function fetch() {
+ var response_code = document.getElementById('response_code');
+
+ xmlhttp = new XMLHttpRequest();
+ xmlhttp.open("GET",
+ "http://%s:%d/%s?code=" + response_code.value,
+ true);
+ xmlhttp.onreadystatechange = function() {
+ reportResult(
+ 'readyState=' + xmlhttp.readyState + ', status=' + xmlhttp.status);
+ }
+ try {
+ xmlhttp.send(null);
+ } catch (e) {
+ reportResult('Exception: ' + e);
+ }
+}
+
+</script>
+</head>
+<body>
+<form action="javascript:fetch()">
+ Response code to get: <input id="response_code" type="text" value="503">
+ <input type="submit">
+</form>
+</body>
+</html>'''
+
+
class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
keep_running = True
+ local_ip = ''
+ port = 0
def do_GET(self):
if self.path == '/quitquitquit':
@@ -25,22 +67,43 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): RequestHandler.keep_running = False
return
+ if self.path.startswith('/ajax/'):
+ self.send_response(200)
+ self.send_header('Content-Type', 'text/html')
+ self.end_headers()
+ self.wfile.write(AJAX_TEST_PAGE % (self.local_ip,
+ self.port,
+ self.path[6:]))
+ return
+
params = urlparse.parse_qs(urlparse.urlparse(self.path).query)
+ def SendCustomRetryIfRequested():
+ if params and 'custom-retry-after' in params:
+ custom_retry = params['custom-retry-after'][0]
+ self.send_header('X-Retry-After', custom_retry)
+
if not params or not 'code' in params or params['code'][0] == '200':
self.send_response(200)
self.send_header('Content-Type', 'text/plain')
+ SendCustomRetryIfRequested()
self.end_headers()
self.wfile.write('OK')
else:
- self.send_error(int(params['code'][0]))
+ status_code = int(params['code'][0])
+ self.send_response(status_code)
+ SendCustomRetryIfRequested()
+ self.end_headers()
+ self.wfile.write('Error %d' % int(status_code))
def main():
- if len(sys.argv) != 2:
- print "Usage: %s PORT" % sys.argv[0]
+ if len(sys.argv) != 3:
+ print "Usage: %s LOCAL_IP PORT" % sys.argv[0]
sys.exit(1)
- port = int(sys.argv[1])
+ RequestHandler.local_ip = sys.argv[1]
+ port = int(sys.argv[2])
+ RequestHandler.port = port
print "To stop the server, go to http://localhost:%d/quitquitquit" % port
httpd = BaseHTTPServer.HTTPServer(('', port), RequestHandler)
while RequestHandler.keep_running:
diff --git a/net/url_request/url_request_throttler_entry.cc b/net/url_request/url_request_throttler_entry.cc index b14bcbe..45dc619 100644 --- a/net/url_request/url_request_throttler_entry.cc +++ b/net/url_request/url_request_throttler_entry.cc @@ -7,8 +7,12 @@ #include <cmath> #include "base/logging.h" +#include "base/metrics/field_trial.h" +#include "base/metrics/histogram.h" #include "base/rand_util.h" #include "base/string_number_conversions.h" +#include "base/values.h" +#include "net/base/net_log.h" #include "net/url_request/url_request_throttler_header_interface.h" #include "net/url_request/url_request_throttler_manager.h" @@ -30,9 +34,10 @@ const int URLRequestThrottlerEntry::kDefaultMaxSendThreshold = 20; // unavailable at the end of each back-off period for a total of about // 48 minutes. // -// Ignoring the first 4 errors helps avoid back-off from kicking in on -// flaky connections. -const int URLRequestThrottlerEntry::kDefaultNumErrorsToIgnore = 4; +// Ignoring the first couple of errors is just a conservative measure to +// avoid false positives. It should help avoid back-off from kicking in e.g. +// on flaky connections. +const int URLRequestThrottlerEntry::kDefaultNumErrorsToIgnore = 2; const int URLRequestThrottlerEntry::kDefaultInitialBackoffMs = 700; const double URLRequestThrottlerEntry::kDefaultMultiplyFactor = 1.4; const double URLRequestThrottlerEntry::kDefaultJitterFactor = 0.4; @@ -44,20 +49,71 @@ const char URLRequestThrottlerEntry::kExponentialThrottlingHeader[] = const char URLRequestThrottlerEntry::kExponentialThrottlingDisableValue[] = "disable"; +// NetLog parameters when a request is rejected by throttling. +class RejectedRequestParameters : public NetLog::EventParameters { + public: + RejectedRequestParameters(const std::string& url_id, + int num_failures, + int release_after_ms) + : url_id_(url_id), + num_failures_(num_failures), + release_after_ms_(release_after_ms) { + } + + virtual Value* ToValue() const {
+ DictionaryValue* dict = new DictionaryValue();
+ dict->SetString("url", url_id_);
+ dict->SetInteger("num_failures", num_failures_);
+ dict->SetInteger("release_after_ms", release_after_ms_);
+ return dict;
+ }
+ + private:
+ std::string url_id_;
+ int num_failures_; + int release_after_ms_; +}; + +// NetLog parameters when a response contains an X-Retry-After header. +class RetryAfterParameters : public NetLog::EventParameters { + public: + RetryAfterParameters(const std::string& url_id, + int retry_after_ms) + : url_id_(url_id), + retry_after_ms_(retry_after_ms) { + } + + virtual Value* ToValue() const {
+ DictionaryValue* dict = new DictionaryValue();
+ dict->SetString("url", url_id_);
+ dict->SetInteger("retry_after_ms", retry_after_ms_);
+ return dict;
+ }
+ + private:
+ std::string url_id_;
+ int retry_after_ms_; +}; + URLRequestThrottlerEntry::URLRequestThrottlerEntry( - URLRequestThrottlerManager* manager) + URLRequestThrottlerManager* manager, + const std::string& url_id) : sliding_window_period_( base::TimeDelta::FromMilliseconds(kDefaultSlidingWindowPeriodMs)), max_send_threshold_(kDefaultMaxSendThreshold), is_backoff_disabled_(false), backoff_entry_(&backoff_policy_), - manager_(manager) { + manager_(manager), + url_id_(url_id), + net_log_(BoundNetLog::Make( + manager->net_log(), NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING)) { DCHECK(manager_); Initialize(); } URLRequestThrottlerEntry::URLRequestThrottlerEntry( URLRequestThrottlerManager* manager, + const std::string& url_id, int sliding_window_period_ms, int max_send_threshold, int initial_backoff_ms, @@ -69,7 +125,8 @@ URLRequestThrottlerEntry::URLRequestThrottlerEntry( max_send_threshold_(max_send_threshold), is_backoff_disabled_(false), backoff_entry_(&backoff_policy_), - manager_(manager) { + manager_(manager), + url_id_(url_id) { DCHECK_GT(sliding_window_period_ms, 0); DCHECK_GT(max_send_threshold_, 0); DCHECK_GE(initial_backoff_ms, 0); @@ -106,7 +163,7 @@ bool URLRequestThrottlerEntry::IsEntryOutdated() const { // If there are send events in the sliding window period, we still need this // entry. if (!send_log_.empty() && - send_log_.back() + sliding_window_period_ > GetTimeNow()) { + send_log_.back() + sliding_window_period_ > ImplGetTimeNow()) { return false; } @@ -122,15 +179,37 @@ void URLRequestThrottlerEntry::DetachManager() { } bool URLRequestThrottlerEntry::IsDuringExponentialBackoff() const { - if (is_backoff_disabled_) - return false; + bool reject_request = false; + if (!is_backoff_disabled_ && GetBackoffEntry()->ShouldRejectRequest()) { + int num_failures = GetBackoffEntry()->failure_count(); + int release_after_ms = + (GetBackoffEntry()->GetReleaseTime() - base::TimeTicks::Now()) + .InMilliseconds(); + + net_log_.AddEvent( + NetLog::TYPE_THROTTLING_REJECTED_REQUEST, + make_scoped_refptr( + new RejectedRequestParameters(url_id_, + num_failures, + release_after_ms))); + + reject_request = true; + } + + int reject_count = reject_request ? 1 : 0; + UMA_HISTOGRAM_ENUMERATION( + "Throttling.RequestThrottled", reject_count, 2); + if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) { + UMA_HISTOGRAM_ENUMERATION(base::FieldTrial::MakeName( + "Throttling.RequestThrottled", "ThrottlingEnabled"), reject_count, 2); + } - return GetBackoffEntry()->ShouldRejectRequest(); + return reject_request; } int64 URLRequestThrottlerEntry::ReserveSendingTimeForNextRequest( const base::TimeTicks& earliest_time) { - base::TimeTicks now = GetTimeNow(); + base::TimeTicks now = ImplGetTimeNow(); // If a lot of requests were successfully made recently, // sliding_window_release_time_ may be greater than @@ -171,7 +250,7 @@ base::TimeTicks // wrong thing to do (i.e. it would likely be too long). Therefore, we // return "now" so that retries are not delayed. if (is_backoff_disabled_) - return GetTimeNow(); + return ImplGetTimeNow(); return GetBackoffEntry()->GetReleaseTime(); } @@ -179,7 +258,10 @@ base::TimeTicks void URLRequestThrottlerEntry::UpdateWithResponse( const std::string& host, const URLRequestThrottlerHeaderInterface* response) { - if (response->GetResponseCode() >= 500) { + int response_code = response->GetResponseCode(); + HandleMetricsTracking(response_code); + + if (IsConsideredError(response_code)) { GetBackoffEntry()->InformOfRequest(false); } else { GetBackoffEntry()->InformOfRequest(true); @@ -195,15 +277,20 @@ void URLRequestThrottlerEntry::UpdateWithResponse( } } -void URLRequestThrottlerEntry::ReceivedContentWasMalformed() { +void URLRequestThrottlerEntry::ReceivedContentWasMalformed(int response_code) { // A malformed body can only occur when the request to fetch a resource // was successful. Therefore, in such a situation, we will receive one - // call to ReceivedContentWasMalformed() and one call to UpdateWithResponse() - // with a response categorized as "good". To end up counting one failure, - // we need to count two failures here against the one success in - // UpdateWithResponse(). - GetBackoffEntry()->InformOfRequest(false); - GetBackoffEntry()->InformOfRequest(false); + // call to ReceivedContentWasMalformed() and one call to + // UpdateWithResponse() with a response categorized as "good". To end + // up counting one failure, we need to count two failures here against + // the one success in UpdateWithResponse(). + // + // We do nothing for a response that is already being considered an error + // based on its status code (otherwise we would count 3 errors instead of 1). + if (!IsConsideredError(response_code)) { + GetBackoffEntry()->InformOfRequest(false); + GetBackoffEntry()->InformOfRequest(false); + } } URLRequestThrottlerEntry::~URLRequestThrottlerEntry() { @@ -217,9 +304,38 @@ void URLRequestThrottlerEntry::Initialize() { backoff_policy_.jitter_factor = kDefaultJitterFactor; backoff_policy_.maximum_backoff_ms = kDefaultMaximumBackoffMs; backoff_policy_.entry_lifetime_ms = kDefaultEntryLifetimeMs; + + // We pretend we just had a successful response so that we have a + // starting point to our tracking. This is called from the + // constructor so we do not use the virtual ImplGetTimeNow(). + last_successful_response_time_ = base::TimeTicks::Now(); + last_response_was_success_ = true; +} + +bool URLRequestThrottlerEntry::IsConsideredError(int response_code) { + // We throttle only for the status codes most likely to indicate the server + // is failing because it is too busy or otherwise are likely to be + // because of DDoS. + // + // 500 is the generic error when no better message is suitable, and + // as such does not necessarily indicate a temporary state, but + // other status codes cover most of the permanent error states. + // 503 is explicitly documented as a temporary state where the server + // is either overloaded or down for maintenance. + // 509 is the (non-standard but widely implemented) Bandwidth Limit Exceeded + // status code, which might indicate DDoS. + // + // We do not back off on 502 or 504, which are reported by gateways + // (proxies) on timeouts or failures, because in many cases these requests + // have not made it to the destination server and so we do not actually + // know that it is down or busy. One degenerate case could be a proxy on + // localhost, where you are not actually connected to the network. + return (response_code == 500 || + response_code == 503 || + response_code == 509); } -base::TimeTicks URLRequestThrottlerEntry::GetTimeNow() const { +base::TimeTicks URLRequestThrottlerEntry::ImplGetTimeNow() const { return base::TimeTicks::Now(); } @@ -241,8 +357,22 @@ void URLRequestThrottlerEntry::HandleCustomRetryAfter( if (value_ms < 0) return; - GetBackoffEntry()->SetCustomReleaseTime( - GetTimeNow() + base::TimeDelta::FromMilliseconds(value_ms)); + net_log_.AddEvent( + NetLog::TYPE_THROTTLING_GOT_CUSTOM_RETRY_AFTER, + make_scoped_refptr(new RetryAfterParameters(url_id_, value_ms))); + + base::TimeDelta value = base::TimeDelta::FromMilliseconds(value_ms); + GetBackoffEntry()->SetCustomReleaseTime(ImplGetTimeNow() + value); + + UMA_HISTOGRAM_CUSTOM_TIMES( + "Throttling.CustomRetryAfterMs", value, + base::TimeDelta::FromSeconds(1), base::TimeDelta::FromHours(12), 50); + if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) { + UMA_HISTOGRAM_CUSTOM_TIMES( + base::FieldTrial::MakeName("Throttling.CustomRetryAfterMs", + "ThrottlingEnabled"), value, + base::TimeDelta::FromSeconds(1), base::TimeDelta::FromHours(12), 50); + } } void URLRequestThrottlerEntry::HandleThrottlingHeader( @@ -257,6 +387,54 @@ void URLRequestThrottlerEntry::HandleThrottlingHeader( } } +void URLRequestThrottlerEntry::HandleMetricsTracking(int response_code) { + // This is essentially the same as the "Net.HttpResponseCode" UMA stat + // but we are tracking it separately here for the throttling experiment + // to make sure we count only the responses seen by throttling. + // TODO(joi): Remove after experiment. + UMA_HISTOGRAM_ENUMERATION("Throttling.HttpResponseCode", response_code, 600); + if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) { + UMA_HISTOGRAM_ENUMERATION( + base::FieldTrial::MakeName("Throttling.HttpResponseCode", + "ThrottlingEnabled"), + response_code, 600); + } + + // Note that we are not interested in whether the code is considered + // an error for the backoff logic, but whether it is a 5xx error in + // general. This is because here, we are tracking the apparent total + // downtime of a server. + if (response_code >= 500) { + last_response_was_success_ = false; + } else { + base::TimeTicks now = ImplGetTimeNow(); + if (!last_response_was_success_) { + // We are transitioning from failure to success, so generate our stats. + base::TimeDelta down_time = now - last_successful_response_time_; + int failure_count = GetBackoffEntry()->failure_count(); + + UMA_HISTOGRAM_COUNTS("Throttling.FailureCountAtSuccess", failure_count); + UMA_HISTOGRAM_CUSTOM_TIMES( + "Throttling.PerceivedDowntime", down_time, + base::TimeDelta::FromMilliseconds(10), + base::TimeDelta::FromHours(6), 50); + + if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) { + UMA_HISTOGRAM_COUNTS(base::FieldTrial::MakeName( + "Throttling.FailureCountAtSuccess", "ThrottlingEnabled"), + failure_count); + UMA_HISTOGRAM_CUSTOM_TIMES(base::FieldTrial::MakeName( + "Throttling.PerceivedDowntime", "ThrottlingEnabled"), down_time, + base::TimeDelta::FromMilliseconds(10), + base::TimeDelta::FromHours(6), 50); + } + } + + last_successful_response_time_ = now; + last_response_was_success_ = true; + } +} + const BackoffEntry* URLRequestThrottlerEntry::GetBackoffEntry() const { return &backoff_entry_; } diff --git a/net/url_request/url_request_throttler_entry.h b/net/url_request/url_request_throttler_entry.h index 7cc4133..75e6aa0 100644 --- a/net/url_request/url_request_throttler_entry.h +++ b/net/url_request/url_request_throttler_entry.h @@ -10,7 +10,9 @@ #include <string> #include "base/basictypes.h" +#include "base/time.h" #include "net/base/backoff_entry.h" +#include "net/base/net_log.h" #include "net/url_request/url_request_throttler_entry_interface.h" namespace net { @@ -69,12 +71,14 @@ class NET_API URLRequestThrottlerEntry static const char kExponentialThrottlingDisableValue[]; // The manager object's lifetime must enclose the lifetime of this object. - explicit URLRequestThrottlerEntry(URLRequestThrottlerManager* manager); + explicit URLRequestThrottlerEntry(URLRequestThrottlerManager* manager, + const std::string& url_id); // The life span of instances created with this constructor is set to // infinite, and the number of initial errors to ignore is set to 0. // It is only used by unit tests. URLRequestThrottlerEntry(URLRequestThrottlerManager* manager, + const std::string& url_id, int sliding_window_period_ms, int max_send_threshold, int initial_backoff_ms, @@ -100,15 +104,19 @@ class NET_API URLRequestThrottlerEntry virtual void UpdateWithResponse( const std::string& host, const URLRequestThrottlerHeaderInterface* response); - virtual void ReceivedContentWasMalformed(); + virtual void ReceivedContentWasMalformed(int response_code); protected: virtual ~URLRequestThrottlerEntry(); void Initialize(); + // Returns true if the given response code is considered an error for + // throttling purposes. + bool IsConsideredError(int response_code); + // Equivalent to TimeTicks::Now(), virtual to be mockable for testing purpose. - virtual base::TimeTicks GetTimeNow() const; + virtual base::TimeTicks ImplGetTimeNow() const; // Used internally to increase release time following a retry-after header. void HandleCustomRetryAfter(const std::string& header_value); @@ -117,6 +125,10 @@ class NET_API URLRequestThrottlerEntry void HandleThrottlingHeader(const std::string& header_value, const std::string& host); + // Used internally to keep track of failure->success transitions and + // generate statistics about them. + void HandleMetricsTracking(int response_code); + // Retrieves the back-off entry object we're using. Used to enable a // unit testing seam for dependency injection in tests. virtual const BackoffEntry* GetBackoffEntry() const; @@ -154,9 +166,23 @@ class NET_API URLRequestThrottlerEntry // Access it through GetBackoffEntry() to allow a unit test seam. BackoffEntry backoff_entry_; + // The time of the last successful response, plus knowledge of whether + // the last response was successful or not, let us generate statistics on + // the length of perceived downtime for a given URL, and the error count + // when such transitions occur. This is useful for experiments with + // throttling but will likely become redundant after they are finished. + // TODO(joi): Remove when the time is right + base::TimeTicks last_successful_response_time_; + bool last_response_was_success_; + // Weak back-reference to the manager object managing us. URLRequestThrottlerManager* manager_; + // Canonicalized URL string that this entry is for; used for logging only. + std::string url_id_; + + BoundNetLog net_log_; + DISALLOW_COPY_AND_ASSIGN(URLRequestThrottlerEntry); }; diff --git a/net/url_request/url_request_throttler_entry_interface.h b/net/url_request/url_request_throttler_entry_interface.h index 259c5cf..a910e1a 100644 --- a/net/url_request/url_request_throttler_entry_interface.h +++ b/net/url_request/url_request_throttler_entry_interface.h @@ -49,9 +49,10 @@ class NET_API URLRequestThrottlerEntryInterface // Lets higher-level modules, that know how to parse particular response // bodies, notify of receiving malformed content for the given URL. This will - // be handled by the throttler as if an HTTP 5xx response had been received to - // the request, i.e. it will count as a failure. - virtual void ReceivedContentWasMalformed() = 0; + // be handled by the throttler as if an HTTP 503 response had been received to + // the request, i.e. it will count as a failure, unless the HTTP response code + // indicated is already one of those that will be counted as an error. + virtual void ReceivedContentWasMalformed(int response_code) = 0; protected: friend class base::RefCountedThreadSafe<URLRequestThrottlerEntryInterface>; diff --git a/net/url_request/url_request_throttler_manager.cc b/net/url_request/url_request_throttler_manager.cc index 07c0605..e138bba 100644 --- a/net/url_request/url_request_throttler_manager.cc +++ b/net/url_request/url_request_throttler_manager.cc @@ -5,7 +5,10 @@ #include "net/url_request/url_request_throttler_manager.h" #include "base/logging.h" +#include "base/metrics/field_trial.h" +#include "base/metrics/histogram.h" #include "base/string_util.h" +#include "net/base/net_log.h" #include "net/base/net_util.h" namespace net { @@ -27,10 +30,20 @@ scoped_refptr<URLRequestThrottlerEntryInterface> // Periodically garbage collect old entries. GarbageCollectEntriesIfNecessary(); - // Find the entry in the map or create it. + // Find the entry in the map or create a new NULL entry. scoped_refptr<URLRequestThrottlerEntry>& entry = url_entries_[url_id]; + + // If the entry exists but could be garbage collected at this point, we + // start with a fresh entry so that we possibly back off a bit less + // aggressively (i.e. this resets the error count when the entry's URL + // hasn't been requested in long enough). + if (entry.get() && entry->IsEntryOutdated()) { + entry = NULL; + } + + // Create the entry if needed. if (entry.get() == NULL) { - entry = new URLRequestThrottlerEntry(this); + entry = new URLRequestThrottlerEntry(this, url_id); // We only disable back-off throttling on an entry that we have // just constructed. This is to allow unit tests to explicitly override @@ -40,6 +53,13 @@ scoped_refptr<URLRequestThrottlerEntryInterface> std::string host = url.host(); if (opt_out_hosts_.find(host) != opt_out_hosts_.end() || IsLocalhost(host)) { + if (!logged_for_localhost_disabled_ && IsLocalhost(host)) { + logged_for_localhost_disabled_ = true; + net_log_->AddEvent( + NetLog::TYPE_THROTTLING_DISABLED_FOR_HOST, + make_scoped_refptr(new NetLogStringParameter("host", host))); + } + // TODO(joi): Once sliding window is separate from back-off throttling, // we can simply return a dummy implementation of // URLRequestThrottlerEntryInterface here that never blocks anything (and @@ -57,7 +77,18 @@ void URLRequestThrottlerManager::AddToOptOutList(const std::string& host) { // after there are already one or more entries in url_entries_ for that // host, the pre-existing entries may still perform back-off throttling. // In practice, this would almost never occur. - opt_out_hosts_.insert(host); + if (opt_out_hosts_.find(host) == opt_out_hosts_.end()) { + UMA_HISTOGRAM_COUNTS("Throttling.SiteOptedOut", 1); + if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) { + UMA_HISTOGRAM_COUNTS(base::FieldTrial::MakeName( + "Throttling.SiteOptedOut", "ThrottlingEnabled"), 1); + } + + net_log_->EndEvent( + NetLog::TYPE_THROTTLING_DISABLED_FOR_HOST, + make_scoped_refptr(new NetLogStringParameter("host", host))); + opt_out_hosts_.insert(host); + } } void URLRequestThrottlerManager::OverrideEntryForTests( @@ -94,11 +125,31 @@ bool URLRequestThrottlerManager::enforce_throttling() { return enforce_throttling_; } +void URLRequestThrottlerManager::set_net_log(NetLog* net_log) { + DCHECK(net_log); + NetLog::Source source(NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING, + net_log->NextID()); + net_log_.reset(new BoundNetLog(source, net_log)); +} + +NetLog* URLRequestThrottlerManager::net_log() const { + return net_log_->net_log(); +} + +void URLRequestThrottlerManager::OnIPAddressChanged() { + OnNetworkChange(); +} + +void URLRequestThrottlerManager::OnOnlineStateChanged(bool online) { + OnNetworkChange(); +} + // TODO(joi): Turn throttling on by default when appropriate. URLRequestThrottlerManager::URLRequestThrottlerManager() : requests_since_last_gc_(0), enforce_throttling_(false), - enable_thread_checks_(false) { + enable_thread_checks_(false), + logged_for_localhost_disabled_(false) { // Construction/destruction is on main thread (because BrowserMain // retrieves an instance to call InitializeOptions), but is from then on // used on I/O thread. @@ -108,12 +159,22 @@ URLRequestThrottlerManager::URLRequestThrottlerManager() url_id_replacements_.ClearUsername(); url_id_replacements_.ClearQuery(); url_id_replacements_.ClearRef(); + + // Make sure there is always a net_log_ instance, even if it logs to + // nowhere. + net_log_.reset(new BoundNetLog()); + + NetworkChangeNotifier::AddIPAddressObserver(this); + NetworkChangeNotifier::AddOnlineStateObserver(this); } URLRequestThrottlerManager::~URLRequestThrottlerManager() { // Destruction is on main thread (AtExit), but real use is on I/O thread. DetachFromThread(); + NetworkChangeNotifier::RemoveIPAddressObserver(this); + NetworkChangeNotifier::RemoveOnlineStateObserver(this); + // Since, for now, the manager object might conceivably go away before // the entries, detach the entries' back-pointer to the manager. // @@ -163,4 +224,13 @@ void URLRequestThrottlerManager::GarbageCollectEntries() { } } +void URLRequestThrottlerManager::OnNetworkChange() { + // Remove all entries. Any entries that in-flight requests have a reference + // to will live until those requests end, and these entries may be + // inconsistent with new entries for the same URLs, but since what we + // want is a clean slate for the new connection state, this is OK. + url_entries_.clear(); + requests_since_last_gc_ = 0; +} + } // namespace net diff --git a/net/url_request/url_request_throttler_manager.h b/net/url_request/url_request_throttler_manager.h index 4b9b21f..846b682 100644 --- a/net/url_request/url_request_throttler_manager.h +++ b/net/url_request/url_request_throttler_manager.h @@ -16,10 +16,14 @@ #include "base/threading/non_thread_safe.h" #include "googleurl/src/gurl.h" #include "net/base/net_api.h" +#include "net/base/network_change_notifier.h" #include "net/url_request/url_request_throttler_entry.h" namespace net { +class BoundNetLog; +class NetLog; + // Class that registers URL request throttler entries for URLs being accessed // in order to supervise traffic. URL requests for HTTP contents should // register their URLs in this manager on each request. @@ -34,7 +38,9 @@ namespace net { // although to allow it to be used as a singleton, construction and destruction // can occur on a separate thread. class NET_API URLRequestThrottlerManager - : NON_EXPORTED_BASE(public base::NonThreadSafe) { + : NON_EXPORTED_BASE(public base::NonThreadSafe), + public NetworkChangeNotifier::IPAddressObserver, + public NetworkChangeNotifier::OnlineStateObserver { public: static URLRequestThrottlerManager* GetInstance(); @@ -71,6 +77,16 @@ class NET_API URLRequestThrottlerManager void set_enforce_throttling(bool enforce); bool enforce_throttling(); + // Sets the NetLog instance to use. + void set_net_log(NetLog* net_log); + NetLog* net_log() const; + + // IPAddressObserver interface. + virtual void OnIPAddressChanged() OVERRIDE; + + // OnlineStateObserver interface. + virtual void OnOnlineStateChanged(bool online) OVERRIDE; + protected: URLRequestThrottlerManager(); ~URLRequestThrottlerManager(); @@ -90,6 +106,15 @@ class NET_API URLRequestThrottlerManager // Method that does the actual work of garbage collecting. void GarbageCollectEntries(); + // When we switch from online to offline or change IP addresses, we + // clear all back-off history. This is a precaution in case the change in + // online state now lets us communicate without error with servers that + // we were previously getting 500 or 503 responses from (perhaps the + // responses are from a badly-written proxy that should have returned a + // 502 or 504 because it's upstream connection was down or it had no route + // to the server). + void OnNetworkChange(); + // Used by tests. int GetNumberOfEntriesForTests() const { return url_entries_.size(); } @@ -136,6 +161,13 @@ class NET_API URLRequestThrottlerManager // workaround. bool enable_thread_checks_; + // Initially false, switches to true once we have logged because of back-off + // being disabled for localhost. + bool logged_for_localhost_disabled_; + + // NetLog to use, or NULL if none configured. + scoped_ptr<BoundNetLog> net_log_; + DISALLOW_COPY_AND_ASSIGN(URLRequestThrottlerManager); }; diff --git a/net/url_request/url_request_throttler_unittest.cc b/net/url_request/url_request_throttler_unittest.cc index f6227b3..354adb9 100644 --- a/net/url_request/url_request_throttler_unittest.cc +++ b/net/url_request/url_request_throttler_unittest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "base/memory/scoped_ptr.h" +#include "base/metrics/histogram.h" #include "base/pickle.h" #include "base/stringprintf.h" #include "base/string_number_conversions.h" @@ -19,6 +20,10 @@ using base::TimeTicks; namespace net { namespace { + +using base::Histogram; +using base::StatisticsRecorder; + class MockURLRequestThrottlerManager; class MockBackoffEntry : public BackoffEntry { @@ -29,7 +34,7 @@ class MockBackoffEntry : public BackoffEntry { virtual ~MockBackoffEntry() {} - TimeTicks GetTimeNow() const { + virtual TimeTicks ImplGetTimeNow() const OVERRIDE { return fake_now_; } @@ -45,7 +50,7 @@ class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry { public : explicit MockURLRequestThrottlerEntry( net::URLRequestThrottlerManager* manager) - : net::URLRequestThrottlerEntry(manager), + : net::URLRequestThrottlerEntry(manager, ""), mock_backoff_entry_(&backoff_policy_) { InitPolicy(); } @@ -54,7 +59,7 @@ class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry { const TimeTicks& exponential_backoff_release_time, const TimeTicks& sliding_window_release_time, const TimeTicks& fake_now) - : net::URLRequestThrottlerEntry(manager), + : net::URLRequestThrottlerEntry(manager, ""), fake_time_now_(fake_now), mock_backoff_entry_(&backoff_policy_) { InitPolicy(); @@ -86,13 +91,13 @@ class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry { fake_time_now_ = time_now; mock_backoff_entry_.SetFakeNow(time_now); - GetBackoffEntry()->InformOfRequest(true); // Sets failure count to 0. + GetBackoffEntry()->Reset(); GetBackoffEntry()->SetCustomReleaseTime(time_now); set_sliding_window_release_time(time_now); } // Overridden for tests. - virtual TimeTicks GetTimeNow() const { return fake_time_now_; } + virtual TimeTicks ImplGetTimeNow() const OVERRIDE { return fake_time_now_; } void set_exponential_backoff_release_time( const base::TimeTicks& release_time) { @@ -219,15 +224,66 @@ struct GurlAndString { class URLRequestThrottlerEntryTest : public testing::Test { protected: virtual void SetUp(); + + // After calling this function, histogram snapshots in |samples_| contain + // only the delta caused by the test case currently running. + void CalculateHistogramDeltas(); + TimeTicks now_; MockURLRequestThrottlerManager manager_; // Dummy object, not used. scoped_refptr<MockURLRequestThrottlerEntry> entry_; + + std::map<std::string, Histogram::SampleSet> original_samples_; + std::map<std::string, Histogram::SampleSet> samples_; +}; + +// List of all histograms we care about in these unit tests. +const char* kHistogramNames[] = { + "Throttling.CustomRetryAfterMs", + "Throttling.FailureCountAtSuccess", + "Throttling.HttpResponseCode", + "Throttling.PerceivedDowntime", + "Throttling.RequestThrottled", + "Throttling.SiteOptedOut", }; void URLRequestThrottlerEntryTest::SetUp() { now_ = TimeTicks::Now(); entry_ = new MockURLRequestThrottlerEntry(&manager_); entry_->ResetToBlank(now_); + + for (size_t i = 0; i < arraysize(kHistogramNames); ++i) { + // Must retrieve original samples for each histogram for comparison + // as other tests may affect them. + const char* name = kHistogramNames[i]; + Histogram::SampleSet& original = original_samples_[name]; + Histogram* histogram; + if (StatisticsRecorder::FindHistogram(name, &histogram)) { + histogram->SnapshotSample(&original); + } + } +} + +void URLRequestThrottlerEntryTest::CalculateHistogramDeltas() { + for (size_t i = 0; i < arraysize(kHistogramNames); ++i) { + const char* name = kHistogramNames[i]; + Histogram::SampleSet& original = original_samples_[name]; + Histogram::SampleSet& sample = samples_[name]; + + Histogram* histogram; + if (StatisticsRecorder::FindHistogram(name, &histogram)) { + ASSERT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags()); + + histogram->SnapshotSample(&sample); + // Ensure |original| size is same as |sample|, then subtract original + // values. + original.Resize(*histogram); + sample.Subtract(original); + } + } + + // Ensure we don't accidentally use the originals in our tests. + original_samples_.clear(); } std::ostream& operator<<(std::ostream& out, const base::TimeTicks& time) { @@ -238,6 +294,10 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceDuringExponentialBackoff) { entry_->set_exponential_backoff_release_time( entry_->fake_time_now_ + TimeDelta::FromMilliseconds(1)); EXPECT_TRUE(entry_->IsDuringExponentialBackoff()); + + CalculateHistogramDeltas(); + ASSERT_EQ(0, samples_["Throttling.RequestThrottled"].counts(0)); + ASSERT_EQ(1, samples_["Throttling.RequestThrottled"].counts(1)); } TEST_F(URLRequestThrottlerEntryTest, InterfaceNotDuringExponentialBackoff) { @@ -246,6 +306,10 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceNotDuringExponentialBackoff) { entry_->set_exponential_backoff_release_time( entry_->fake_time_now_ - TimeDelta::FromMilliseconds(1)); EXPECT_FALSE(entry_->IsDuringExponentialBackoff()); + + CalculateHistogramDeltas(); + ASSERT_EQ(2, samples_["Throttling.RequestThrottled"].counts(0)); + ASSERT_EQ(0, samples_["Throttling.RequestThrottled"].counts(1)); } TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateRetryAfter) { @@ -258,13 +322,18 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateRetryAfter) { "increase release_time"; entry_->ResetToBlank(now_); - header_w_delay_header.fake_retry_value_ = "-5.5"; + MockURLRequestThrottlerHeaderAdapter header_w_negative_header( + "-5.5", "", 200); + entry_->UpdateWithResponse("", &header_w_negative_header); EXPECT_EQ(entry_->GetExponentialBackoffReleaseTime(), entry_->fake_time_now_) << "When given a negative value, it should not change the release_time"; + + CalculateHistogramDeltas(); + ASSERT_EQ(1, samples_["Throttling.CustomRetryAfterMs"].TotalCount()); } TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateFailure) { - MockURLRequestThrottlerHeaderAdapter failure_response(505); + MockURLRequestThrottlerHeaderAdapter failure_response(503); entry_->UpdateWithResponse("", &failure_response); EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), entry_->fake_time_now_) << "A failure should increase the release_time"; @@ -278,12 +347,19 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateSuccess) { } TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateSuccessThenFailure) { - MockURLRequestThrottlerHeaderAdapter failure_response(500); + MockURLRequestThrottlerHeaderAdapter failure_response(503); MockURLRequestThrottlerHeaderAdapter success_response(200); entry_->UpdateWithResponse("", &success_response); entry_->UpdateWithResponse("", &failure_response); EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), entry_->fake_time_now_) << "This scenario should add delay"; + entry_->UpdateWithResponse("", &success_response); + + CalculateHistogramDeltas(); + ASSERT_EQ(1, samples_["Throttling.HttpResponseCode"].counts(503)); + ASSERT_EQ(2, samples_["Throttling.HttpResponseCode"].counts(200)); + ASSERT_EQ(1, samples_["Throttling.FailureCountAtSuccess"].counts(1)); + ASSERT_EQ(1, samples_["Throttling.PerceivedDowntime"].TotalCount()); } TEST_F(URLRequestThrottlerEntryTest, IsEntryReallyOutdated) { @@ -308,7 +384,7 @@ TEST_F(URLRequestThrottlerEntryTest, IsEntryReallyOutdated) { TEST_F(URLRequestThrottlerEntryTest, MaxAllowedBackoff) { for (int i = 0; i < 30; ++i) { - MockURLRequestThrottlerHeaderAdapter response_adapter(505); + MockURLRequestThrottlerHeaderAdapter response_adapter(503); entry_->UpdateWithResponse("", &response_adapter); } @@ -318,7 +394,7 @@ TEST_F(URLRequestThrottlerEntryTest, MaxAllowedBackoff) { } TEST_F(URLRequestThrottlerEntryTest, MalformedContent) { - MockURLRequestThrottlerHeaderAdapter response_adapter(505); + MockURLRequestThrottlerHeaderAdapter response_adapter(503); for (int i = 0; i < 5; ++i) entry_->UpdateWithResponse("", &response_adapter); @@ -329,7 +405,7 @@ TEST_F(URLRequestThrottlerEntryTest, MalformedContent) { // UpdateWithResponse to pair with ReceivedContentWasMalformed() since that // is what happens in practice (if a body is received, then a non-500 // response must also have been received). - entry_->ReceivedContentWasMalformed(); + entry_->ReceivedContentWasMalformed(200); MockURLRequestThrottlerHeaderAdapter success_adapter(200); entry_->UpdateWithResponse("", &success_adapter); EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), release_after_failures); @@ -386,6 +462,9 @@ TEST(URLRequestThrottlerManager, IsUrlStandardised) { GurlAndString(GURL("http://www.example.com/index.php?code=1#superEntry"), std::string("http://www.example.com/index.php"), __LINE__), + GurlAndString(GURL("http://www.example.com/index.php#superEntry"), + std::string("http://www.example.com/index.php"), + __LINE__), GurlAndString(GURL("http://www.example.com:1234/"), std::string("http://www.example.com:1234/"), __LINE__)}; @@ -485,4 +564,36 @@ TEST(URLRequestThrottlerManager, OptOutHeader) { ExpectEntryAllowsAllOnErrorIfOptedOut(localhost_entry, true); } +TEST(URLRequestThrottlerManager, ClearOnNetworkChange) { + for (int i = 0; i < 3; ++i) { + MockURLRequestThrottlerManager manager; + scoped_refptr<net::URLRequestThrottlerEntryInterface> entry_before = + manager.RegisterRequestUrl(GURL("http://www.example.com/")); + MockURLRequestThrottlerHeaderAdapter failure_adapter(503); + for (int j = 0; j < 10; ++j) { + // Host doesn't really matter in this scenario so we skip it. + entry_before->UpdateWithResponse("", &failure_adapter); + } + EXPECT_TRUE(entry_before->IsDuringExponentialBackoff()); + + switch (i) { + case 0: + manager.OnIPAddressChanged(); + break; + case 1: + manager.OnOnlineStateChanged(true); + break; + case 2: + manager.OnOnlineStateChanged(false); + break; + default: + FAIL(); + } + + scoped_refptr<net::URLRequestThrottlerEntryInterface> entry_after = + manager.RegisterRequestUrl(GURL("http://www.example.com/")); + EXPECT_FALSE(entry_after->IsDuringExponentialBackoff()); + } +} + } // namespace net |