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

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.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=84202

[Jói, 2011-05-04 17:45:53]

Hi Sanjeev,

This should fix the issue we discussed yesterday.  Let me know if you had
created a bug for it and I'll update the change description.

Cheers,
Jói

[sanjeevr, 2011-05-04 18:51:50]

LGTM

http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc
File net/base/backoff_entry.cc (right):

http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc#newco...
net/base/backoff_entry.cc:37: exponential_backoff_release_time_ =
CalculateReleaseTime();
I don't know the code well enough to tell if calling CalculateReleaseTime here
is the right thing to do. I am assuming it is :)

[sanjeevr, 2011-05-04 18:52:30]

I have not filed a bug yet for this. Also, can this merged to M12 or is that too
painful?

On 2011/05/04 18:51:50, sanjeevr wrote:
> LGTM
> 
> http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc
> File net/base/backoff_entry.cc (right):
> 
>
http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc#newco...
> net/base/backoff_entry.cc:37: exponential_backoff_release_time_ =
> CalculateReleaseTime();
> I don't know the code well enough to tell if calling CalculateReleaseTime here
> is the right thing to do. I am assuming it is :)

[Jói, 2011-05-04 19:53:24]

I think the merge to M12 should be pretty painless.  Since we're going
to merge, do you mind opening a bug to document the issue?

Oh, and you caught a small bug in your comment, I will upload a new
version in a couple of minutes.

Cheers,
Jói


On Wed, May 4, 2011 at 2:52 PM,  <sanjeevr@chromium.org> wrote:
> I have not filed a bug yet for this. Also, can this merged to M12 or is that
> too
> painful?
>
> On 2011/05/04 18:51:50, sanjeevr wrote:
>>
>> LGTM
>
>> http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc
>> File net/base/backoff_entry.cc (right):
>
>
>
http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc#newco...
>>
>> net/base/backoff_entry.cc:37: exponential_backoff_release_time_ =
>> CalculateReleaseTime();
>> I don't know the code well enough to tell if calling CalculateReleaseTime
>> here
>> is the right thing to do. I am assuming it is :)
>
>
>
> http://codereview.chromium.org/6932013/
>

[Jói, 2011-05-04 20:04:29]

PTAL.

http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc
File net/base/backoff_entry.cc (right):

http://codereview.chromium.org/6932013/diff/1/net/base/backoff_entry.cc#newco...
net/base/backoff_entry.cc:37: exponential_backoff_release_time_ =
CalculateReleaseTime();
On 2011/05/04 18:51:50, sanjeevr wrote:
> I don't know the code well enough to tell if calling CalculateReleaseTime here
> is the right thing to do. I am assuming it is :)

Nope, it's a small bug to call it in the success case. Thanks for asking.

[sanjeevr, 2011-05-04 20:23:09]

LGTM

http://codereview.chromium.org/6932013/diff/9/net/base/backoff_entry.cc
File net/base/backoff_entry.cc (right):

http://codereview.chromium.org/6932013/diff/9/net/base/backoff_entry.cc#newco...
net/base/backoff_entry.cc:38: failure_count_ = std::max(0, --failure_count_);
I think we need to think of a better fix in the long term for the malformed
case. Ideally you do want to reset the failure count to 0, but also remember the
last value so we revert to that + 1 if the upper layer reports a malformed body.
But I think this is OK for now.

[Jói, 2011-05-04 20:30:14]

I've added BUG=81587 to the change description and a TODO for your last comment.

http://codereview.chromium.org/6932013/diff/9/net/base/backoff_entry.cc
File net/base/backoff_entry.cc (right):

http://codereview.chromium.org/6932013/diff/9/net/base/backoff_entry.cc#newco...
net/base/backoff_entry.cc:38: failure_count_ = std::max(0, --failure_count_);
On 2011/05/04 20:23:09, sanjeevr wrote:
> I think we need to think of a better fix in the long term for the malformed
> case. Ideally you do want to reset the failure count to 0, but also remember
the
> last value so we revert to that + 1 if the upper layer reports a malformed
body.
> But I think this is OK for now.

Added a TODO to revisit this.

[Jói, 2011-05-04 20:36:50]

Adding Ricardo for net/OWNERS approval.

[rvargas (doing something else), 2011-05-04 20:42:08]

RBSTMP LGTM