remove FetchContext::getCachePolicy()

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

Index: third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
index f05b6b4cd689537d5f19ed947e748b848c2c955d..e6dc243e11e2c718d625a58e76d8ad7216a554c8 100644
--- a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
@@ -215,6 +215,76 @@ bool shouldDisallowFetchForMainFrameScript(ResourceRequest& request,
           isConnectionEffectively2G(effectiveConnection));
 }
 
+enum class RequestMethod { kIsPost, kIsNotPost };

[Nate Chapin, 2017/03/09 20:02:47]

I'm behind on the style guide: is kEnumValue preferred now over EnumValue?

[Takashi Toyoshima, 2017/03/13 11:54:29]

Yeah, I also recently noticed this style change through a code review.

https://www.chromium.org/blink/coding-style

According to the rule 12, "kInterCaps is preferable for new code."

+enum class RequestType { kIsConditional, kIsNotConditional };
+enum class ResourceType { kIsMainResource, kIsNotMainResource };
+
+WebCachePolicy determineWebCachePolicy(RequestMethod method,
+                                       RequestType requestType,
+                                       ResourceType resourceType,
+                                       FrameLoadType loadType) {
+  switch (loadType) {
+    case FrameLoadTypeStandard:
+    case FrameLoadTypeReplaceCurrentItem:
+    case FrameLoadTypeInitialInChildFrame:
+      return (requestType == RequestType::kIsConditional ||
+              method == RequestMethod::kIsPost)
+                 ? WebCachePolicy::ValidatingCacheData
+                 : WebCachePolicy::UseProtocolCachePolicy;
+    case FrameLoadTypeInitialHistoryLoad:
+    case FrameLoadTypeBackForward:
+      // Mutates the policy for POST requests to avoid form resubmission.
+      return method == RequestMethod::kIsPost
+                 ? WebCachePolicy::ReturnCacheDataDontLoad
+                 : WebCachePolicy::ReturnCacheDataElseLoad;
+    case FrameLoadTypeReload:
+      return WebCachePolicy::ValidatingCacheData;
+    case FrameLoadTypeReloadMainResource:
+      return resourceType == ResourceType::kIsMainResource
+                 ? WebCachePolicy::ValidatingCacheData
+                 : WebCachePolicy::UseProtocolCachePolicy;
+    case FrameLoadTypeReloadBypassingCache:
+      return WebCachePolicy::BypassingCache;
+  }
+  NOTREACHED();
+  return WebCachePolicy::UseProtocolCachePolicy;
+}
+
+// TODO(toyoshim): Remove |type|. See comments in resourceRequestCachePolicy().
+WebCachePolicy determineFrameWebCachePolicy(Frame* frame, Resource::Type type) {

[Nate Chapin, 2017/03/09 20:02:46]

Can we use the local ResourceType instead of Resource::Type here?

[Takashi Toyoshima, 2017/03/13 11:54:30]

Ah, yes. Sorry, I just forgot to revert this type change.
When I changed this function to a class static method, I changed it to
Resource::Type, but now it's an anonymous function and can be back to
ResourceType.

+  if (!frame)
+    return WebCachePolicy::UseProtocolCachePolicy;
+  if (!frame->isLocalFrame())

[Nate Chapin, 2017/03/09 20:02:46]

Should this have a TODO to remove when |type| is removed?

[Takashi Toyoshima, 2017/03/13 11:54:29]

Ah, yes. Subresources will not need to check parents' policy too.

+    return determineFrameWebCachePolicy(frame->tree().parent(), type);
+
+  // Does not propagate cache policy for subresources after the load event.
+  if (type != Resource::MainResource &&
+      toLocalFrame(frame)->document()->fetcher()->context().isLoadComplete()) {

[Nate Chapin, 2017/03/09 20:02:47]

Optional: Just check loadEventFinished() directly instead of routing to the
FetchContext?

[Takashi Toyoshima, 2017/03/13 11:54:30]

Done.

+    return WebCachePolicy::UseProtocolCachePolicy;
+  }
+
+  // Respects BypassingCache rather than parent's policy.
+  // TODO(toyoshim): Adopt BypassingCache even for MainResource.

[Nate Chapin, 2017/03/09 20:02:46]

Why is the MainResource special case necessary?

[Takashi Toyoshima, 2017/03/13 11:54:29]

Just because the old behavior is so.
I already confirmed that all tests pass even if I remove this main resource
exception, but I just want to exclude behavior changes from this CL as I can as
possible so that I can easily revert behavior change case by case when I need.

+  FrameLoadType loadType =
+      toLocalFrame(frame)->loader().documentLoader()->loadType();
+  if (type != Resource::MainResource &&
+      loadType == FrameLoadTypeReloadBypassingCache) {
+    return WebCachePolicy::BypassingCache;
+  }
+
+  // Respects parent's policy if it has a special one.
+  WebCachePolicy parentPolicy =
+      determineFrameWebCachePolicy(frame->tree().parent(), type);
+  if (parentPolicy != WebCachePolicy::UseProtocolCachePolicy)
+    return parentPolicy;
+
+  // Otherwise, follows FrameLoadType. Use kIsNotPost, kIsNotConditional, and
+  // kIsNotMainResource to obtain a representative policy for the frame.
+  return determineWebCachePolicy(RequestMethod::kIsNotPost,
+                                 RequestType::kIsNotConditional,
+                                 ResourceType::kIsNotMainResource, loadType);
+}
+
 }  // namespace
 
 FrameFetchContext::FrameFetchContext(DocumentLoader* loader, Document* document)
@@ -285,118 +355,44 @@ void FrameFetchContext::addAdditionalRequestHeaders(ResourceRequest& request,
     request.setHTTPHeaderField("Save-Data", "on");
 }
 
-CachePolicy FrameFetchContext::getCachePolicy() const {
-  if (m_document && m_document->loadEventFinished())
-    return CachePolicyVerify;
-
-  FrameLoadType loadType = masterDocumentLoader()->loadType();
-  if (loadType == FrameLoadTypeReloadBypassingCache)
-    return CachePolicyReload;
-
-  Frame* parentFrame = frame()->tree().parent();
-  if (parentFrame && parentFrame->isLocalFrame()) {
-    CachePolicy parentCachePolicy = toLocalFrame(parentFrame)
-                                        ->document()
-                                        ->fetcher()
-                                        ->context()
-                                        .getCachePolicy();
-    if (parentCachePolicy != CachePolicyVerify)
-      return parentCachePolicy;
-  }
-
-  if (loadType == FrameLoadTypeReload)
-    return CachePolicyRevalidate;
-
-  if (m_documentLoader &&
-      m_documentLoader->getRequest().getCachePolicy() ==
-          WebCachePolicy::ReturnCacheDataElseLoad)
-    return CachePolicyHistoryBuffer;
-
-  // Returns CachePolicyVerify for other cases, mainly FrameLoadTypeStandard and
-  // FrameLoadTypeReloadMainResource. See public/web/WebFrameLoadType.h to know
-  // how these load types work.
-  return CachePolicyVerify;
-}
-
-static WebCachePolicy memoryCachePolicyToResourceRequestCachePolicy(
-    const CachePolicy policy) {
-  if (policy == CachePolicyVerify)
-    return WebCachePolicy::UseProtocolCachePolicy;
-  if (policy == CachePolicyRevalidate)
-    return WebCachePolicy::ValidatingCacheData;
-  if (policy == CachePolicyReload)
-    return WebCachePolicy::BypassingCache;
-  if (policy == CachePolicyHistoryBuffer)
-    return WebCachePolicy::ReturnCacheDataElseLoad;
-  return WebCachePolicy::UseProtocolCachePolicy;
-}
-
-static WebCachePolicy frameLoadTypeToWebCachePolicy(FrameLoadType type) {
-  if (type == FrameLoadTypeBackForward)
-    return WebCachePolicy::ReturnCacheDataElseLoad;
-  if (type == FrameLoadTypeReloadBypassingCache)
-    return WebCachePolicy::BypassingCache;
-  if (type == FrameLoadTypeReload)
-    return WebCachePolicy::ValidatingCacheData;
-  return WebCachePolicy::UseProtocolCachePolicy;
-}
-
 WebCachePolicy FrameFetchContext::resourceRequestCachePolicy(
     ResourceRequest& request,
     Resource::Type type,
     FetchRequest::DeferOption defer) const {
   DCHECK(frame());
   if (type == Resource::MainResource) {
-    FrameLoadType frameLoadType = masterDocumentLoader()->loadType();
-    if (request.httpMethod() == "POST" &&
-        frameLoadType == FrameLoadTypeBackForward)
-      return WebCachePolicy::ReturnCacheDataDontLoad;
-    if (frameLoadType == FrameLoadTypeReloadMainResource ||
-        request.isConditional() || request.httpMethod() == "POST")
-      return WebCachePolicy::ValidatingCacheData;
-
-    WebCachePolicy policy = frameLoadTypeToWebCachePolicy(frameLoadType);
-    if (policy != WebCachePolicy::UseProtocolCachePolicy)
-      return policy;
-
-    for (Frame* f = frame()->tree().parent(); f; f = f->tree().parent()) {
-      if (!f->isLocalFrame())
-        continue;
-      policy = frameLoadTypeToWebCachePolicy(
-          toLocalFrame(f)->loader().documentLoader()->loadType());
-      if (policy != WebCachePolicy::UseProtocolCachePolicy)
-        return policy;
-    }
-    // Returns UseProtocolCachePolicy for other cases, parent frames not having
-    // special kinds of FrameLoadType as they are checked inside the for loop
-    // above, or |frameLoadType| being FrameLoadTypeStandard. See
-    // public/web/WebFrameLoadType.h to know how these load types work.
-    return WebCachePolicy::UseProtocolCachePolicy;
+    const WebCachePolicy cachePolicy = determineWebCachePolicy(
+        request.httpMethod() == "POST" ? RequestMethod::kIsPost
+                                       : RequestMethod::kIsNotPost,
+        request.isConditional() ? RequestType::kIsConditional
+                                : RequestType::kIsNotConditional,
+        ResourceType::kIsMainResource, masterDocumentLoader()->loadType());
+    // Follows the parent frame's policy.
+    // TODO(toyoshim): Probably, FrameLoadType for each frame should have a
+    // right type for reload or history navigations, and should not need to
+    // check parent's frame policy here. Once it has a right FrameLoadType,
+    // we can remove Resource::Type argument from determineFrameWebCachePolicy.
+    // See also crbug.com/332602.
+    if (cachePolicy != WebCachePolicy::UseProtocolCachePolicy)
+      return cachePolicy;
+    return determineFrameWebCachePolicy(frame()->tree().parent(), type);
   }
 
   // For users on slow connections, we want to avoid blocking the parser in
   // the main frame on script loads inserted via document.write, since it can
   // add significant delays before page content is displayed on the screen.
+  // TODO(toyoshim): Move following logic that rewrites ResourceRequest to
+  // somewhere that should be relevant to the script resource handling.
   if (type == Resource::Script && isMainFrame() && m_document &&
       shouldDisallowFetchForMainFrameScript(request, defer, *m_document))
     return WebCachePolicy::ReturnCacheDataDontLoad;
 
+  // TODO(toyoshim): We should check isConditional() and use ValidatingCacheData
+  // only when |cachePolicy| below is UseProtocolCachePolicy.
   if (request.isConditional())
     return WebCachePolicy::ValidatingCacheData;
 
-  if (m_documentLoader && m_document && !m_document->loadEventFinished()) {
-    // For POST requests, we mutate the main resource's cache policy to avoid
-    // form resubmission. This policy should not be inherited by subresources.
-    WebCachePolicy mainResourceCachePolicy =
-        m_documentLoader->getRequest().getCachePolicy();
-    if (m_documentLoader->getRequest().httpMethod() == "POST") {
-      if (mainResourceCachePolicy == WebCachePolicy::ReturnCacheDataDontLoad)
-        return WebCachePolicy::ReturnCacheDataElseLoad;
-      return WebCachePolicy::UseProtocolCachePolicy;
-    }
-    return memoryCachePolicyToResourceRequestCachePolicy(getCachePolicy());
-  }
-  return WebCachePolicy::UseProtocolCachePolicy;
+  return determineFrameWebCachePolicy(frame(), type);
 }
 
 // The |m_documentLoader| is null in the FrameFetchContext of an imported