Fix logic for handling reports of malformed bodies. To end up counting

one error, we need to count two when the malformed body is reported,
against the one success reported based on the response code from what
must have been a successful HTTP request.

Updated a unit test that previously should have caught this but was
making incorrect assumptions.

BUG=81587
TEST=net_unittests

Initially Committed: r84196
Reverted: r84200
Re-landed after fixing compile error.

Review URL: http://codereview.chromium.org/6932013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@84202 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 23 insertions, 9 deletions

diff --git a/net/base/backoff_entry.cc b/net/base/backoff_entry.cc
index 44bae52..e7b7b55 100644
--- a/net/base/backoff_entry.cc
+++ b/net/base/backoff_entry.cc
@@ -29,10 +29,18 @@ BackoffEntry::~BackoffEntry() {
 
 void BackoffEntry::InformOfRequest(bool succeeded) {
   if (!succeeded) {
-    failure_count_++;
+    ++failure_count_;
     exponential_backoff_release_time_ = CalculateReleaseTime();
   } else {
-    failure_count_ = 0;
+    // We slowly decay the number of times delayed instead of resetting it to 0
+    // in order to stay stable if we receive successes interleaved between lots
+    // of failures.
+    //
+    // TODO(joi): Revisit this; it might be most correct to go to zero
+    // but have a way to go back to "old error count +1" if there is
+    // another error soon after.
+    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
diff --git a/net/url_request/url_request_throttler_entry.cc b/net/url_request/url_request_throttler_entry.cc
index d07f054..b14bcbe 100644
--- a/net/url_request/url_request_throttler_entry.cc
+++ b/net/url_request/url_request_throttler_entry.cc
@@ -196,12 +196,13 @@ void URLRequestThrottlerEntry::UpdateWithResponse(
 }
 
 void URLRequestThrottlerEntry::ReceivedContentWasMalformed() {
-  // We keep this simple and just count it as a single error.
-  //
-  // If we wanted to get fancy, we would count two errors here, and decrease
-  // the error count only by one when we receive a successful (by status
-  // code) response. Instead, we keep things simple by always resetting the
-  // error count on success, and therefore counting only a single error here.
+  // 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);
 }
 
diff --git a/net/url_request/url_request_throttler_unittest.cc b/net/url_request/url_request_throttler_unittest.cc
index 5f8f2ee..f6227b3 100644
--- a/net/url_request/url_request_throttler_unittest.cc
+++ b/net/url_request/url_request_throttler_unittest.cc
@@ -325,8 +325,13 @@ TEST_F(URLRequestThrottlerEntryTest, MalformedContent) {
   TimeTicks release_after_failures = entry_->GetExponentialBackoffReleaseTime();
 
   // Inform the entry that a response body was malformed, which is supposed to
-  // increase the back-off time.
+  // increase the back-off time.  Note that we also submit a successful
+  // 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();
+  MockURLRequestThrottlerHeaderAdapter success_adapter(200);
+  entry_->UpdateWithResponse("", &success_adapter);
   EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), release_after_failures);
 }