FrameFetchContext should not always respect conditional requests

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

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 203 matching lines @@
           is2G) ||
          (document.settings()
               ->getDisallowFetchForDocWrittenScriptsInMainFrameIfEffectively2G() &&
           isConnectionEffectively2G(effectiveConnection));
 }
 
 enum class RequestMethod { kIsPost, kIsNotPost };
 enum class RequestType { kIsConditional, kIsNotConditional };
 enum class ResourceType { kIsMainResource, kIsNotMainResource };
 
+// Determines WebCachePolicy for a main resource, or WebCachePolicy that is
+// corresponding to FrameLoadType.
+// TODO(toyoshim): Probably, we should split FrameLoadType to WebCachePolicy
+// conversion logic into a separate function once other TODOs in this function
+// are resolved.
 WebCachePolicy determineWebCachePolicy(RequestMethod method,
                                        RequestType requestType,
                                        ResourceType resourceType,
                                        FrameLoadType loadType) {
   switch (loadType) {
     case FrameLoadTypeStandard:
       return (requestType == RequestType::kIsConditional ||
               method == RequestMethod::kIsPost)
                  ? WebCachePolicy::ValidatingCacheData
                  : WebCachePolicy::UseProtocolCachePolicy;
@@ skipping 29 matching lines @@
       return (resourceType == ResourceType::kIsMainResource &&
               (requestType == RequestType::kIsConditional ||
                method == RequestMethod::kIsPost))
                  ? WebCachePolicy::ValidatingCacheData
                  : WebCachePolicy::BypassingCache;
   }
   NOTREACHED();
   return WebCachePolicy::UseProtocolCachePolicy;
 }
 
-// TODO(toyoshim): Remove |resourceType|. See comments in
-// resourceRequestCachePolicy().
+// Determines WebCachePolicy for |frame|. This WebCachePolicy should be a base
+// policy to consider one of each resource belonging to the frame, and should
+// not count resource specific conditions in.
+// TODO(toyoshim): Remove |resourceType| to realize the design described above.
+// See also comments in resourceRequestCachePolicy().
 WebCachePolicy determineFrameWebCachePolicy(Frame* frame,
                                             ResourceType resourceType) {
   if (!frame)
     return WebCachePolicy::UseProtocolCachePolicy;
   if (!frame->isLocalFrame())
     return determineFrameWebCachePolicy(frame->tree().parent(), resourceType);
 
   // Does not propagate cache policy for subresources after the load event.
   // TODO(toyoshim): We should be able to remove following parents' policy check
   // if each frame has a relevant FrameLoadType for reload and history
@@ skipping 122 matching lines @@
 
   // 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())
+  const WebCachePolicy cachePolicy =
+      determineFrameWebCachePolicy(frame(), ResourceType::kIsNotMainResource);
+
+  // TODO(toyoshim): Revisit to consider if this clause can be merged to
+  // determineWebCachePolicy or determineFrameWebCachePolicy.
+  if (cachePolicy == WebCachePolicy::UseProtocolCachePolicy &&
+      request.isConditional()) {
     return WebCachePolicy::ValidatingCacheData;
-
-  return determineFrameWebCachePolicy(frame(),
-                                      ResourceType::kIsNotMainResource);
+  }
+  return cachePolicy;
 }
 
 // The |m_documentLoader| is null in the FrameFetchContext of an imported
 // document.
 // FIXME(http://crbug.com/274173): This means Inspector, which uses
 // DocumentLoader as a grouping entity, cannot see imported documents.
 inline DocumentLoader* FrameFetchContext::masterDocumentLoader() const {
   if (m_documentLoader)
     return m_documentLoader.get();
 
@@ skipping 638 matching lines @@
                                                     response);
 }
 
 DEFINE_TRACE(FrameFetchContext) {
   visitor->trace(m_document);
   visitor->trace(m_documentLoader);
   FetchContext::trace(visitor);
 }
 
 }  // namespace blink