The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a27638a0c9a2677e92d024a733b6c4884d48f0e3

commit a27638a0c9a2677e92d024a733b6c4884d48f0e3
Author: toyoshim <toyoshim@chromium.org>
Date: Tue Dec 06 10:20:47 2016

Reload cache control test: add more tests

Add new test case for the same page navigation.
Also, add additional checks with DevTool shown/closed
for each test case.

BUG= 670232 ,  671545 
TEST=./out/Release/content_browsertests --gtest_filter='Reload*.*'
TEST=./out/Release/content_browsertests --gtest_filter='Reload*.*' --enable-browser-side-navigation

Review-Url: https://codereview.chromium.org/1991153002
Cr-Commit-Position: refs/heads/master@{#436559}

[modify] https://crrev.com/a27638a0c9a2677e92d024a733b6c4884d48f0e3/content/browser/loader/reload_cache_control_browsertest.cc

One of tests submitted at #c1 is disabled because of this issue.

toyoshim@: a nit on the change to reload_cache_control_browsertest.cc: prefer using the function available in content/public/common/browser_side_navigation_policy.h to check if PlzNavigate is enabled.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d8bd78d8de55ceb02be5ed9d6ad4a4d2f731d78

commit 4d8bd78d8de55ceb02be5ed9d6ad4a4d2f731d78
Author: toyoshim <toyoshim@chromium.org>
Date: Thu Dec 15 04:30:11 2016

Use content::IsBrowserSideNavigationEnabled() in ReloadCacheControlBrowserTest

To check if browser side navigation is enabled, use
content::IsBrowserSideNavigationEnabled() instead of checking command
line directly.

BUG= 671545 

Review-Url: https://codereview.chromium.org/2574973002
Cr-Commit-Position: refs/heads/master@{#438739}

[modify] https://crrev.com/4d8bd78d8de55ceb02be5ed9d6ad4a4d2f731d78/content/browser/loader/reload_cache_control_browsertest.cc

Note: CL at #c4 is for the comment at #c3.
Original issue isn't fixed yet.

clamy@, can you comment?

I'm not sure I understand the issue: same-page navigations are fragment navigations & pushState/popState. They are synchronous in the renderer, so I don't see how they would revalidate the main resource since no network request is made. Do you mean same-site navigations?

The same-page navigation isn't fragment navigations or pushState/popState here, but just do a usual navigation to the same URL page. In such case, we specially handle it as a reload variant, e.g. forcibly revalidate main resource, history isn't updated, scroll positions is restored, and so on.

Ah I see what you mean. We only call those same-page navigations in the scope of the NavigationController (otherwise same-page navigation tends to designate fragment and pushState/popState). ananta@ is working on relanding scottmg@'s patch which fixes the issue on PlzNavigate by removing SAME_PAGE from the NavigationController classification & also ensures we do a reload properly in that case. CL is in review at https://codereview.chromium.org/2580753002/.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea77fe5f9663714b8458b23dd996279baade45d6

commit ea77fe5f9663714b8458b23dd996279baade45d6
Author: toyoshim <toyoshim@chromium.org>
Date: Thu Mar 16 09:35:11 2017

Disable ReloadCacheControlBrowserTest if PlzNavigate is enabled

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
TBR=kinuko@chromium.org
BUG= 671545 ,  670237 

Review-Url: https://codereview.chromium.org/2752073005
Cr-Commit-Position: refs/heads/master@{#457384}

[modify] https://crrev.com/ea77fe5f9663714b8458b23dd996279baade45d6/content/browser/loader/reload_cache_control_browsertest.cc

toyoshim@, can we investigate why this test is failing? We are hoping to start field trials of PlzNavigate in the next week or so, which means that this behavior will be visible to users.

I think one of the reason is that FrameFetchContext does not handle PlzNavigate related FrameLoadType now.

One another clear reason is test side problems. It monitors network requests, but when PlzNavigate is enable, we can not ensure the resource request order between <img src> and <iframe src>. This should be fixed in test side.

I'm doing a related cleanup work; https://codereview.chromium.org/1867493003/ and I can try to fix this issue in the work.

Currently, FrameFetchContext::resourceRequestCachePolicy explores parent's FrameLoadType, but it skips if the parent is RemoteFrame. So, this looks the reason why BypassingCache flag is not used in iframes when PlzNavigate is enabled.

In my plan, I want to change propagating parent FrameLoadType to iframes when the parent loads iframes. WebLocalFrameImpl::createChildFrame() would be the right place to do it. But if I pass FrameLoadTypeReloadBypassingCache to the child frame there, it results in using ValidatingCacheData instead of BypassingCache. I will investigate why it happens in the next week.

So, while I'm investigating this issue, it reminds me some questions.

- What's FrameLoadType

Currently it contains multiple orthogonal concepts, navigation type to determine cache policy (Standard/BackFoward/Reload*), navitation type to manage history (ReplaceCurrentItem), and something for PlaNavigate (Initial*, but I do not understand them correctly. Please correct me if my understanding is wrong).

- What's FrameLoadRequest

Probably, relationship between FrameLoadRequest and ResourceRequest should be similar to the one of FetchRequest and ResourceRequest.

- Why FrameLoader::determineFrameLoadType() is needed?

FrameLoader::load() calls determineFrameLoadType(FrameLoadRequest) and overwrite the original FrameLoadType that is passed in the method call.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d48b3d14741301605c1715aef0c323c710b16a79

commit d48b3d14741301605c1715aef0c323c710b16a79
Author: toyoshim <toyoshim@chromium.org>
Date: Wed Mar 22 06:45:01 2017

Enable ResloadCacheControlBrowserTest.NavigateToSame for PlzNavigate

This was disabled for a while because it does not work if browser side
navigation is enabled. But the problem seems to be already fixed.

This patch just re-enables the NavigateToSame test. BypassingReload
test is still failing if I try enabling it. Fix will follow soon.

BUG= 671545 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2756153002
Cr-Commit-Position: refs/heads/master@{#458646}

[modify] https://crrev.com/d48b3d14741301605c1715aef0c323c710b16a79/content/browser/loader/reload_cache_control_browsertest.cc

toyoshim@, we are aiming to launch PlzNavigate in M60, which is the current milestone on trunk. Would you be able to look into this bug and fix it earlier rather than later? 
Thanks in advance!

I do not have enough bandwidth for a while, and am not sure I could work on and fix this in m60. If someone in PlzNavigate team can take a look, that would be nice. Otherwise, I will do my best.

I will take a look.

It looks like the cache flags are set in a few different places in the renderer, but the only place that matter at the end is in ResourceFetcher::InitializeResourceRequest() and it use the FrameFetchContext::ResourceRequestCachePolicy() to set the WebCachePolicy.

PlzNavigate doesn't use the ResourceFetcher for loading frames. That's why it doesn't work.
I will probably have to reuse/move the logic from the FrameFetchContext to another place in order to set the WebCachePolicy of the request before it uses the PlzNavigate path.

ReloadCacheControlBrowserTest is flaky especially on windows.

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_browsertests&tests=ReloadCacheControlBrowserTest

I'll close this bug as fixed, as the underlying issue is fixed. If the test itself is flaky, please file a separate bug and assign it to the test author.