Move cache mode override to DocumentLoader

Previously, we stored it in InternalDocumentStateData and applied in RenderFrameImpl.
This change moves the override to DocumentLoader, to consolidate navigation
related parameters in Blink.

This corresponds to the step 15.d from the doc linked to the bug.

Bug: 855189
Change-Id: Icfabe5cecec3deaab385d3617bc1d9f77c8c13b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1846795Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704630}

content/renderer/internal_document_state_data.cc

@@ -20,9 +20,7 @@ const char kUserDataKey[] = "InternalDocumentStateData";
 
 InternalDocumentStateData::InternalDocumentStateData()
     : is_overriding_user_agent_(false),
-      must_reset_scroll_and_scale_state_(false),
-      cache_policy_override_set_(false),
-      cache_policy_override_(blink::mojom::FetchCacheMode::kDefault) {}
+      must_reset_scroll_and_scale_state_(false) {}
 
 // static
 InternalDocumentStateData* InternalDocumentStateData::FromDocumentLoader(
@@ -52,8 +50,6 @@ void InternalDocumentStateData::CopyFrom(InternalDocumentStateData* other) {
   is_overriding_user_agent_ = other->is_overriding_user_agent_;
   must_reset_scroll_and_scale_state_ =
       other->must_reset_scroll_and_scale_state_;
-  cache_policy_override_set_ = other->cache_policy_override_set_;
-  cache_policy_override_ = other->cache_policy_override_;
   effective_connection_type_ = other->effective_connection_type_;
   request_id_ = other->request_id_;
 }

content/renderer/internal_document_state_data.h

@@ -50,24 +50,6 @@ class InternalDocumentStateData : public base::SupportsUserData::Data {
     must_reset_scroll_and_scale_state_ = state;
   }
 
-  // Sets the cache policy. The cache policy is only used if explicitly set and
-  // by default is not set. You can mark a NavigationState as not having a cache
-  // state by way of clear_cache_policy_override.
-  void set_cache_policy_override(blink::mojom::FetchCacheMode cache_policy) {
-    cache_policy_override_ = cache_policy;
-    cache_policy_override_set_ = true;
-  }
-  blink::mojom::FetchCacheMode cache_policy_override() const {
-    return cache_policy_override_;
-  }
-  void clear_cache_policy_override() {
-    cache_policy_override_set_ = false;
-    cache_policy_override_ = blink::mojom::FetchCacheMode::kDefault;
-  }
-  bool is_cache_policy_override_set() const {
-    return cache_policy_override_set_;
-  }
-
   net::EffectiveConnectionType effective_connection_type() const {
     return effective_connection_type_;
   }
@@ -89,8 +71,6 @@ class InternalDocumentStateData : public base::SupportsUserData::Data {
  private:
   bool is_overriding_user_agent_;
   bool must_reset_scroll_and_scale_state_;
-  bool cache_policy_override_set_;
-  blink::mojom::FetchCacheMode cache_policy_override_;
   net::EffectiveConnectionType effective_connection_type_ =
       net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
   int request_id_ = -1;

content/renderer/render_frame_impl.cc

@@ -913,21 +913,6 @@ std::unique_ptr<DocumentState> BuildDocumentStateFromParams(
   DCHECK(!common_params.navigation_start.is_null());
   DCHECK(!common_params.url.SchemeIs(url::kJavaScriptScheme));
 
-  if (common_params.navigation_type == mojom::NavigationType::RESTORE) {
-    // We're doing a load of a page that was restored from the last session.
-    // By default this prefers the cache over loading
-    // (LOAD_SKIP_CACHE_VALIDATION) which can result in stale data for pages
-    // that are set to expire. We explicitly override that by setting the
-    // policy here so that as necessary we load from the network.
-    //
-    // TODO(davidben): Remove this in favor of passing a cache policy to the
-    // loadHistoryItem call in OnNavigate. That requires not overloading
-    // UseProtocolCachePolicy to mean both "normal load" and "determine cache
-    // policy based on load type, etc".
-    internal_data->set_cache_policy_override(
-        blink::mojom::FetchCacheMode::kDefault);
-  }
-
   internal_data->set_is_overriding_user_agent(
       commit_params.is_overriding_user_agent);
   internal_data->set_must_reset_scroll_and_scale_state(
@@ -1047,6 +1032,21 @@ void FillMiscNavigationParams(
   }
   navigation_params->appcache_host_id =
       commit_params.appcache_host_id.value_or(base::UnguessableToken());
+
+  if (common_params.navigation_type == mojom::NavigationType::RESTORE) {
+    // We're doing a load of a page that was restored from the last session.
+    // By default this prefers the cache over loading
+    // (LOAD_SKIP_CACHE_VALIDATION) which can result in stale data for pages
+    // that are set to expire. We explicitly override that by setting the
+    // policy here so that as necessary we load from the network.
+    //
+    // TODO(davidben): Remove this in favor of passing a cache policy to the
+    // loadHistoryItem call in OnNavigate. That requires not overloading
+    // UseProtocolCachePolicy to mean both "normal load" and "determine cache
+    // policy based on load type, etc".
+    navigation_params->force_fetch_cache_mode =
+        blink::mojom::FetchCacheMode::kDefault;
+  }
 }
 
 // Fills in the origin policy associated with this response, if any is present.
@@ -5290,9 +5290,6 @@ void RenderFrameImpl::WillSendRequestInternal(
   if (!new_url.is_empty())
     request.SetUrl(WebURL(new_url));
 
-  if (internal_data->is_cache_policy_override_set())
-    request.SetCacheMode(internal_data->cache_policy_override());
-
   // The request's extra data may indicate that we should set a custom user
   // agent. This needs to be done here, after WebKit is through with setting the
   // user agent on its own.

third_party/blink/public/web/web_navigation_params.h

@@ -282,6 +282,9 @@ struct BLINK_EXPORT WebNavigationParams {
   WebHistoryItem history_item;
   // Whether this navigation is a result of client redirect.
   bool is_client_redirect = false;
+  // Cache mode to be used for subresources, instead of the one determined
+  // by |frame_load_type|.
+  base::Optional<blink::mojom::FetchCacheMode> force_fetch_cache_mode;
 
   // Miscellaneous parameters.
 

third_party/blink/renderer/core/loader/document_loader.cc

@@ -172,6 +172,7 @@ DocumentLoader::DocumentLoader(
   }
   ip_address_space_ = params_->ip_address_space;
   grant_load_local_resources_ = params_->grant_load_local_resources;
+  force_fetch_cache_mode_ = params_->force_fetch_cache_mode;
 
   WebNavigationTimings& timings = params_->navigation_timings;
   if (!timings.input_start.is_null())
@@ -1104,6 +1105,11 @@ const KURL& DocumentLoader::UnreachableURL() const {
   return unreachable_url_;
 }
 
+const base::Optional<blink::mojom::FetchCacheMode>&
+DocumentLoader::ForceFetchCacheMode() const {
+  return force_fetch_cache_mode_;
+}
+
 bool DocumentLoader::WillLoadUrlAsEmpty(const KURL& url) {
   if (url.IsEmpty())
     return true;

third_party/blink/renderer/core/loader/document_loader.h

@@ -149,6 +149,8 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
   const AtomicString& HttpMethod() const;
   const Referrer& GetReferrer() const;
   const KURL& UnreachableURL() const;
+  const base::Optional<blink::mojom::FetchCacheMode>& ForceFetchCacheMode()
+      const;
 
   void DidChangePerformanceTiming();
   void DidObserveLoadingBehavior(WebLoadingBehaviorFlag);
@@ -412,6 +414,7 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
   network::mojom::IPAddressSpace ip_address_space_ =
       network::mojom::IPAddressSpace::kUnknown;
   bool grant_load_local_resources_ = false;
+  base::Optional<blink::mojom::FetchCacheMode> force_fetch_cache_mode_;
 
   // Params are saved in constructor and are cleared after StartLoading().
   // TODO(dgozman): remove once StartLoading is merged with constructor.

third_party/blink/renderer/core/loader/frame_fetch_context.cc

@@ -424,6 +424,11 @@ void FrameFetchContext::PrepareRequest(
 
   if (GetResourceFetcherProperties().IsDetached())
     return;
+
+  DocumentLoader* document_loader = MasterDocumentLoader();
+  if (document_loader->ForceFetchCacheMode())
+    request.SetCacheMode(*document_loader->ForceFetchCacheMode());
+
   GetLocalFrameClient()->DispatchWillSendRequest(request);
   FrameScheduler* frame_scheduler = GetFrame()->GetFrameScheduler();
   if (!for_redirect && frame_scheduler) {
@@ -432,14 +437,13 @@ void FrameFetchContext::PrepareRequest(
         WebScopedVirtualTimePauser::VirtualTaskDuration::kNonInstant);
   }
 
-  probe::PrepareRequest(Probe(), MasterDocumentLoader(), request,
-                        initiator_info, resource_type);
+  probe::PrepareRequest(Probe(), document_loader, request, initiator_info,
+                        resource_type);
 
   // ServiceWorker hook ups.
-  if (MasterDocumentLoader()->GetServiceWorkerNetworkProvider()) {
+  if (document_loader->GetServiceWorkerNetworkProvider()) {
     WrappedResourceRequest webreq(request);
-    MasterDocumentLoader()->GetServiceWorkerNetworkProvider()->WillSendRequest(
-        webreq);
+    document_loader->GetServiceWorkerNetworkProvider()->WillSendRequest(webreq);
   }
 }