Http cache: Add code to restart a network request when the...

Http cache: Add code to restart a network request when the
server doesn't revalidate a partially stored entry, in other
words, after we issued a conditional byte range request.

BUG=27276
TEST=unittests


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

[eroman, 2009-11-25 21:20:28]

lgtm

http://codereview.chromium.org/434052/diff/7/1008
File net/http/http_cache_transaction.cc (right):

http://codereview.chromium.org/434052/diff/7/1008#newcode915
net/http/http_cache_transaction.cc:915: if (partial_.get() &&
partial_->IsCurrentRangeCached())
was this a separate bug?

http://codereview.chromium.org/434052/diff/7/1008#newcode990
net/http/http_cache_transaction.cc:990: return true;
ok, so the net result of the changes in this function were to invert this return
value from what it was before (by flipping everything else).

http://codereview.chromium.org/434052/diff/7/1008#newcode997
net/http/http_cache_transaction.cc:997: // We cannot truncate this entry, it has
to be deleted.
This comment needs to be updated, since we are no longer deleting it right?

http://codereview.chromium.org/434052/diff/7/1008#newcode1279
net/http/http_cache_transaction.cc:1279: BeginNetworkRequest();
Is it possible to get caught in infinite loop?

[rvargas (doing something else), 2009-11-25 21:38:56]

thanks.

http://codereview.chromium.org/434052/diff/7/1008
File net/http/http_cache_transaction.cc (right):

http://codereview.chromium.org/434052/diff/7/1008#newcode915
net/http/http_cache_transaction.cc:915: if (partial_.get() &&
partial_->IsCurrentRangeCached())
On 2009/11/25 21:20:28, eroman wrote:
> was this a separate bug?

No. I thought this was the only problem, because we ended up sending
"If-None-Match: foo  If-Modified-Since: bar" and the server responded 206.

However, after fixing this, the server was still sending 206 for If-None-Match
and 200 for If-Modified-Since (I inverted the logic to see what it was doing),
even though the PDF was the same.

So we need to fix this and also implement the re-try.

http://codereview.chromium.org/434052/diff/7/1008#newcode990
net/http/http_cache_transaction.cc:990: return true;
On 2009/11/25 21:20:28, eroman wrote:
> ok, so the net result of the changes in this function were to invert this
return
> value from what it was before (by flipping everything else).

The change is to move the previous return value to an argument, an have a new
return value that is true all the time except when we find a serious validation
error and we want to restart the request.

http://codereview.chromium.org/434052/diff/7/1008#newcode997
net/http/http_cache_transaction.cc:997: // We cannot truncate this entry, it has
to be deleted.
On 2009/11/25 21:20:28, eroman wrote:
> This comment needs to be updated, since we are no longer deleting it right?

We are still deleting the cached entry, but we may also return false so that we
re-try the request (with mode_ = NONE)

http://codereview.chromium.org/434052/diff/7/1008#newcode1279
net/http/http_cache_transaction.cc:1279: BeginNetworkRequest();
On 2009/11/25 21:20:28, eroman wrote:
> Is it possible to get caught in infinite loop?

I hope not. The loop should break because ValidatePartialResponse should delete
partial_ whenever returns false, so the next time it has to return true. I added
a comment to ValidatePartialResponse.

[eroman, 2009-11-25 21:58:47]

lgtm