Introduce WebCachePolicy to merge cache policy enums

Introduce WebCachePolicy to merge cache policy enums

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
Committed: https://crrev.com/f3f7f688d56f5c9cf0be4c77c7052cec0419897f
Cr-Commit-Position: refs/heads/master@{#386016}

[Takashi Toyoshima, 2016-04-04 06:53:21]

Description was changed from

==========
Introduce WebCachePolicy to merge three cache policy enum

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy

BUG=599364
==========

to

==========
Introduce WebCachePolicy to merge three cache policy enum

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- remove unnecessary cast between them

BUG=599364
==========

[Takashi Toyoshima, 2016-04-04 07:31:07]

Description was changed from

==========
Introduce WebCachePolicy to merge three cache policy enum

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- remove unnecessary cast between them

BUG=599364
==========

to

==========
Introduce WebCachePolicy to merge three cache policy enum

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
==========

[Takashi Toyoshima, 2016-04-04 07:57:34]

Description was changed from

==========
Introduce WebCachePolicy to merge three cache policy enum

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
==========

to

==========
Introduce WebCachePolicy to merge cache policy enums

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
==========

[Takashi Toyoshima, 2016-04-04 07:59:04]

toyoshim@chromium.org changed reviewers:
+ japhet@chromium.org, jochen@chromium.org

[Takashi Toyoshima, 2016-04-04 08:02:00]

Can you take a look?
Actually, I want to merge one more enum, core/fetch/CachePolicy.h, but I feel I
should make a separate change for that because this change is already scattered
to many files.

[dcheng, 2016-04-04 08:17:10]

dcheng@chromium.org changed reviewers:
+ clamy@chromium.org, dcheng@chromium.org

[dcheng, 2016-04-04 08:17:11]

+clamy, since there's a comment about removing these methods once RenderFrame
only calls load().

https://codereview.chromium.org/1858533002/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/web/WebFrame.h (right):

https://codereview.chromium.org/1858533002/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/web/WebFrame.h:356: WebCachePolicy =
WebCachePolicy::UseProtocolCachePolicy)
I know your patch doesn't introduce this, but why is this a public API? It looks
like there are no callers from the embedder, just callers from inside Blink
itself.

[Takashi Toyoshima, 2016-04-04 11:15:34]

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
>    // TODO(clamy): Remove the reload, reloadWithOverrideURL, loadHistoryItem
>    // loadRequest functions in WebFrame once RenderFrame only calls
loadRequest.

Is this the comment you said here, right?
I guess loadData relaced all callers of loadHistoryItem, but clamy or creis
would know.

I also had a question about this comment. Should we have this comment in
WebFrame.h instead of WebLocalFrame?

[clamy, 2016-04-04 12:51:06]

https://codereview.chromium.org/1858533002/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/web/WebFrame.h (right):

https://codereview.chromium.org/1858533002/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/web/WebFrame.h:356: WebCachePolicy =
WebCachePolicy::UseProtocolCachePolicy)
On 2016/04/04 08:17:10, dcheng wrote:
> I know your patch doesn't introduce this, but why is this a public API? It
looks
> like there are no callers from the embedder, just callers from inside Blink
> itself.

I think this comes from the changes we made to only use load inside
RenderFrame::NavigateInternal, instead of specific subfunctions. At the time teh
change was made, the goal was to remove all other subfunctions, but there were
still used by the embedder (I think this one may have been used in the
HistoryController). If they no longer have callers, then they should be removed
from the public interface.

[Takashi Toyoshima, 2016-04-05 06:13:23]

Thanks clamy.
I can make a separate CL to remove it.

Nate: I will address the point dcheng commented in a separate CL, so can you
continue taking a look on this change?

I want to finish cleanup changes before I add a new reload flag to ship new pull
to refresh feature for Android Chrome.

[Takashi Toyoshima, 2016-04-07 03:19:53]

toyoshim@chromium.org changed reviewers:
+ hiroshige@chromium.org

[Takashi Toyoshima, 2016-04-07 03:21:48]

+hiroshige
Nate seems to be besy for having many review requests, so can you take a quick
look before getting owner review from him?

[jochen (gone - plz use gerrit), 2016-04-07 03:35:51]

lgtm

[hiroshige, 2016-04-07 06:30:32]

https://codereview.chromium.org/1858533002/diff/40001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (left):

https://codereview.chromium.org/1858533002/diff/40001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:1148: // Check whether the
request should be allowed. If not allowed, we reset the
nit: spaces here shouldn't be removed?

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
content/renderer/render_frame_impl.cc:5580: WebCachePolicy cache_policy) {
Can we remove |is_reload| and |cache_policy| if they are unused?

[Nate Chapin, 2016-04-07 20:47:13]

lgtm

[Takashi Toyoshima, 2016-04-08 04:41:48]

https://codereview.chromium.org/1858533002/diff/40001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (left):

https://codereview.chromium.org/1858533002/diff/40001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:1148: // Check whether the
request should be allowed. If not allowed, we reset the
Oh, thanks!
It seems "git cl format" was confused.

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
content/renderer/render_frame_impl.cc:5580: WebCachePolicy cache_policy) {
I think it's WebKit style, but we usually do not remove the name even in such
case in chromium.

[Takashi Toyoshima, 2016-04-08 04:42:21]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-04-08 04:42:22]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1858533002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-08 04:42:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1858533002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1858533002/80001

[hiroshige, 2016-04-08 05:50:40]

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
content/renderer/render_frame_impl.cc:5580: WebCachePolicy cache_policy) {
On 2016/04/08 04:41:48, toyoshim wrote:
> I think it's WebKit style, but we usually do not remove the name even in such
> case in chromium.

Sorry, I meant: can we remove the two parameters from this method, not just
names?

[Takashi Toyoshima, 2016-04-08 06:06:47]

The CQ bit was unchecked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-04-08 06:12:08]

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1858533002/diff/40001/content/renderer/render...
content/renderer/render_frame_impl.cc:5580: WebCachePolicy cache_policy) {
Oops, I see. You are right. I can just remove these arguments.
I cancel the CQ, and preparing to upload a new CL.

[Takashi Toyoshima, 2016-04-08 06:24:58]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-04-08 06:24:58]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1858533002/#ps100001
(title: "hiroshige review")

[commit-bot: I haz the power, 2016-04-08 06:25:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1858533002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1858533002/100001

[hiroshige, 2016-04-08 07:44:56]

lgtm

[commit-bot: I haz the power, 2016-04-08 07:48:04]

Description was changed from

==========
Introduce WebCachePolicy to merge cache policy enums

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
==========

to

==========
Introduce WebCachePolicy to merge cache policy enums

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
==========

[commit-bot: I haz the power, 2016-04-08 07:48:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-04-08 07:49:47]

Description was changed from

==========
Introduce WebCachePolicy to merge cache policy enums

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364
==========

to

==========
Introduce WebCachePolicy to merge cache policy enums

- Replace blink::WebURLRequest::CachePolicy with blink::WebCachePolicy
- Merge blink::ResourceRequestCachePolicy to blink::WebCachePolicy
- Remove unnecessary cast between them
- Replace CachePolicy* with CachePolicy in RenderFrameImpl

BUG=599364

Committed: https://crrev.com/f3f7f688d56f5c9cf0be4c77c7052cec0419897f
Cr-Commit-Position: refs/heads/master@{#386016}
==========

[commit-bot: I haz the power, 2016-04-08 07:49:49]

Patchset 6 (id:??) landed as
https://crrev.com/f3f7f688d56f5c9cf0be4c77c7052cec0419897f
Cr-Commit-Position: refs/heads/master@{#386016}