Http cache: Allow caching of byte ranges without strong validators.

Http cache: Allow caching of byte ranges without strong validators.

In order to fetch a resource using multiple byte range requests, the cache
has to make sure that the resource has not changed, and strong validators
are needed for that.

However, if the cache only has to return data without talking to the server
again, strong validators are not required.

It used to be the case that the cache will not return content from sparse
entries without revalidating tnem with the server first. But that changed a
long time ago to allow regular reuse policies, even for sparse/range requests.

This CL allows caching of partial responses that don't have strong validators,
as they are useful as described above.

BUG=407923
Committed: https://crrev.com/439a05cd87fe1f880b896bd7b154454299355899
Cr-Commit-Position: refs/heads/master@{#292751}

[rvargas (doing something else), 2014-08-27 21:50:19]

rvargas@chromium.org changed reviewers:
+ davidben@chromium.org

[rvargas (doing something else), 2014-08-27 21:50:19]

PTAL

[davidben, 2014-08-29 18:03:24]

Some minor comment suggestions and one question.

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_tran...
File net/http/http_cache_transaction.cc (left):

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_tran...
net/http/http_cache_transaction.cc:1562: if (handling_206_ && !CanResume(false))
{
CanResume checks for a few things other than HasStrongValidators, like a missing
Content-Length. Should that still skip writing to the cache or will the rest of
the logic handle it?

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_unit...
File net/http/http_cache_unittest.cc (right):

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_unit...
net/http/http_cache_unittest.cc:3655: // Tests that we cache partial responses
that can't be validated.
Nit: Does this seem clearer/accurate?

  // Tests that partial responses which don't require revalidation
  // don't require strong validators.

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_unit...
net/http/http_cache_unittest.cc:3685: // Tests failures to validate cache
partial responses.
Nit: Does this seem clearer/accurate?

  // Tests failure to revalidate partial responses that lack
  // strong validators.

[rvargas (doing something else), 2014-08-29 19:34:27]

Thanks!

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_tran...
File net/http/http_cache_transaction.cc (left):

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_tran...
net/http/http_cache_transaction.cc:1562: if (handling_206_ && !CanResume(false))
{
On 2014/08/29 18:03:23, David Benjamin wrote:
> CanResume checks for a few things other than HasStrongValidators, like a
missing
> Content-Length. Should that still skip writing to the cache or will the rest
of
> the logic handle it?

The rest of the logic will handle that. CanResume checks all conditions needed
to issue a conditional byte-range request, which we now don't consider to save a
response. If matching the stored data with a requested range fails, we bail out
before getting to the revalidation logic (PartialData::UpdateFromStoredHeaders)

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_unit...
File net/http/http_cache_unittest.cc (right):

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_unit...
net/http/http_cache_unittest.cc:3655: // Tests that we cache partial responses
that can't be validated.
On 2014/08/29 18:03:23, David Benjamin wrote:
> Nit: Does this seem clearer/accurate?
> 
>   // Tests that partial responses which don't require revalidation
>   // don't require strong validators.

I used "Tests that we store partial responses that can't be validated, as they
can be used for requests that don't require revalidation" instead

https://codereview.chromium.org/509783002/diff/20001/net/http/http_cache_unit...
net/http/http_cache_unittest.cc:3685: // Tests failures to validate cache
partial responses.
On 2014/08/29 18:03:23, David Benjamin wrote:
> Nit: Does this seem clearer/accurate?
> 
>   // Tests failure to revalidate partial responses that lack
>   // strong validators.

Done.

[davidben, 2014-08-29 19:37:10]

lgtm

[rvargas (doing something else), 2014-08-29 23:40:10]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-08-29 23:54:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rvargas@chromium.org/509783002/40001

[commit-bot: I haz the power, 2014-08-30 01:09:05]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[commit-bot: I haz the power, 2014-08-30 01:44:38]

Committed patchset #3 (id:40001) as ecd598fa5e2202fec341604d02082d525a510c46

[commit-bot: I haz the power, 2014-09-10 03:12:59]

Patchset 3 (id:??) landed as
https://crrev.com/439a05cd87fe1f880b896bd7b154454299355899
Cr-Commit-Position: refs/heads/master@{#292751}