Use the ReloadBypassingCache policy

Use the ReloadBypassingCache policy

This CL makes use of the new ReloadBypassingCache policy and removes the
setting of cache HTTP headers in blink.

BUG=376025
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175515

[clamy, 2014-06-04 11:43:38]

@ppi: PTAL

[Nate Chapin, 2014-06-04 16:10:53]

Awesome, LGTM

[ppi, 2014-06-04 16:23:59]

I like where this is going! Please find some comments below.

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/core/loader/Do...
File Source/core/loader/DocumentLoader.cpp (right):

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/core/loader/Do...
Source/core/loader/DocumentLoader.cpp:374:
newRequest.setCachePolicy(ReloadBypassingCache);
Do we currently get a load with net::LOAD_BYPASS_CACHE here or does it result in
net::LOAD_PREFERRING_CACHE?

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/web/tests/WebF...
File Source/web/tests/WebFrameTest.cpp (right):

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/web/tests/WebF...
Source/web/tests/WebFrameTest.cpp:5452: TEST_F(WebFrameTest,
ReloadIgnoringCache)
This asserts on the resulting policy being "ReloadBypassingCache" - should the
test be called ReloadBypassingCache?

What is it that will cause the resulting policy be ReloadBypassingCache? Could
you indicate that in a comment?

[clamy, 2014-06-04 16:39:25]

@japhet: Thanks for the review! I actually have a question I forgot to ask: in
the case of a redirect after a post, should we do a normal reload or an
end-to-end reload?

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/core/loader/Do...
File Source/core/loader/DocumentLoader.cpp (right):

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/core/loader/Do...
Source/core/loader/DocumentLoader.cpp:374:
newRequest.setCachePolicy(ReloadBypassingCache);
On 2014/06/04 16:23:59, ppi wrote:
> Do we currently get a load with net::LOAD_BYPASS_CACHE here or does it result
in
> net::LOAD_PREFERRING_CACHE?

It would seem that we end up with a net::LOAD_VALIDATE_CACHE. However the
comments hint that we should do a reload from origin, which corresponds to a
net::LOAD_BYPASS_CACHE. Nate, which policy should be used here?

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/web/tests/WebF...
File Source/web/tests/WebFrameTest.cpp (right):

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/web/tests/WebF...
Source/web/tests/WebFrameTest.cpp:5452: TEST_F(WebFrameTest,
ReloadIgnoringCache)
On 2014/06/04 16:23:59, ppi wrote:
> This asserts on the resulting policy being "ReloadBypassingCache" - should the
> test be called ReloadBypassingCache?
> 
> What is it that will cause the resulting policy be ReloadBypassingCache? Could
> you indicate that in a comment?

This test checks that asking a reload on a frame with the option ignoringCache
set to true results in the policy being ReloadBypassingCache. I think the name
is right for the test, and added a comment.

[Nate Chapin, 2014-06-04 16:43:55]

https://codereview.chromium.org/318673002/diff/20001/Source/core/loader/Docum...
File Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/318673002/diff/20001/Source/core/loader/Docum...
Source/core/loader/DocumentLoader.cpp:374:
newRequest.setCachePolicy(ReloadBypassingCache);
We have used ReloadIgnoringCacheData as long as I can remember, but based on the
comment and my understanding of how web pages use these redirects,
ReloadBypassingCache probably makes more sense.

[ppi, 2014-06-04 19:25:38]

lgtm, thanks!

https://codereview.chromium.org/318673002/diff/1/Source/web/tests/WebFrameTes...
File Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/318673002/diff/1/Source/web/tests/WebFrameTes...
Source/web/tests/WebFrameTest.cpp:5452: TEST_F(WebFrameTest,
ReloadIgnoringCache)
On 2014/06/04 16:39:25, clamy wrote:
> On 2014/06/04 16:23:59, ppi wrote:
> > This asserts on the resulting policy being "ReloadBypassingCache" - should
the
> > test be called ReloadBypassingCache?
> > 
> > What is it that will cause the resulting policy be ReloadBypassingCache?
Could
> > you indicate that in a comment?
> 
> This test checks that asking a reload on a frame with the option ignoringCache
> set to true results in the policy being ReloadBypassingCache. I think the name
> is right for the test, and added a comment.

ReloadIgnoringCacheData is a name of caching policy that results in cache
revalidation, not bypassing, right? I think we might want to call this
"bypassing" whenever possible, even if existing helper functions don't.

[clamy, 2014-06-04 20:14:19]

@ppi: thanks!

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/web/tests/WebF...
File Source/web/tests/WebFrameTest.cpp (right):

https://chromiumcodereview.appspot.com/318673002/diff/1/Source/web/tests/WebF...
Source/web/tests/WebFrameTest.cpp:5452: TEST_F(WebFrameTest,
ReloadIgnoringCache)
On 2014/06/04 19:25:39, ppi wrote:
> On 2014/06/04 16:39:25, clamy wrote:
> > On 2014/06/04 16:23:59, ppi wrote:
> > > This asserts on the resulting policy being "ReloadBypassingCache" - should
> the
> > > test be called ReloadBypassingCache?
> > > 
> > > What is it that will cause the resulting policy be ReloadBypassingCache?
> Could
> > > you indicate that in a comment?
> > 
> > This test checks that asking a reload on a frame with the option
ignoringCache
> > set to true results in the policy being ReloadBypassingCache. I think the
name
> > is right for the test, and added a comment.
> 
> ReloadIgnoringCacheData is a name of caching policy that results in cache
> revalidation, not bypassing, right? I think we might want to call this
> "bypassing" whenever possible, even if existing helper functions don't.

Done.

[clamy, 2014-06-04 20:14:27]

The CQ bit was checked by clamy@chromium.org

[commit-bot: I haz the power, 2014-06-04 20:14:41]

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

[commit-bot: I haz the power, 2014-06-04 21:25:07]

Change committed as 175515