More reduction of the dependency to ResourceRequest in FetchContext and MixedContentChecker

- Remove one of the ShouldBlockFetch() method from MixedContentChecker.
  We can just pass the parameters directly to the other one extracted
  from ResourceRequest at the caller.

Bonus: Mark BaseFetchContext::CanRequestInternal() as private.

Bug: 736308
Change-Id: I6fe66e5df86cc97698d95ce37002a6ec7601dd02
Reviewed-on: https://chromium-review.googlesource.com/594128
Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491304}

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

@@ -206,12 +206,14 @@ ResourceRequestBlockedReason BaseFetchContext::CanRequestInternal(
       break;
   }
 
+  WebURLRequest::RequestContext request_context =
+      resource_request.GetRequestContext();
+
   // We check the 'report-only' headers before upgrading the request (in
   // populateResourceRequest). We check the enforced headers here to ensure we
   // block things we ought to block.
   if (CheckCSPForRequestInternal(
-          resource_request.GetRequestContext(), url, options, reporting_policy,
-          redirect_status,
+          request_context, url, options, reporting_policy, redirect_status,
           ContentSecurityPolicy::CheckHeaderType::kCheckEnforce) ==
       ResourceRequestBlockedReason::kCSP) {
     return ResourceRequestBlockedReason::kCSP;
@@ -231,10 +233,11 @@ ResourceRequestBlockedReason BaseFetchContext::CanRequestInternal(
       !url.ProtocolIsData())
     return ResourceRequestBlockedReason::kOrigin;
 
+  WebURLRequest::FrameType frame_type = resource_request.GetFrameType();
+
   // Measure the number of legacy URL schemes ('ftp://') and the number of
   // embedded-credential ('http://user:password@...') resources embedded as
   // subresources.
-  WebURLRequest::FrameType frame_type = resource_request.GetFrameType();
   if (frame_type != WebURLRequest::kFrameTypeTopLevel) {
     bool is_subresource = frame_type == WebURLRequest::kFrameTypeNone;
     const SecurityOrigin* embedding_origin =
@@ -258,8 +261,9 @@ ResourceRequestBlockedReason BaseFetchContext::CanRequestInternal(
   // Check for mixed content. We do this second-to-last so that when folks block
   // mixed content via CSP, they don't get a mixed content warning, but a CSP
   // warning instead.
-  if (ShouldBlockFetchByMixedContentCheck(resource_request, url,
-                                          reporting_policy))
+  if (ShouldBlockFetchByMixedContentCheck(request_context, frame_type,
+                                          resource_request.GetRedirectStatus(),
+                                          url, reporting_policy))
     return ResourceRequestBlockedReason::kMixedContent;
 
   if (url.PotentiallyDanglingMarkup() && url.ProtocolIsInHTTPFamily()) {
@@ -272,8 +276,8 @@ ResourceRequestBlockedReason BaseFetchContext::CanRequestInternal(
   // proceed.
   if (GetSubresourceFilter() && type != Resource::kMainResource &&
       type != Resource::kImportResource) {
-    if (!GetSubresourceFilter()->AllowLoad(
-            url, resource_request.GetRequestContext(), reporting_policy)) {
+    if (!GetSubresourceFilter()->AllowLoad(url, request_context,
+                                           reporting_policy)) {
       return ResourceRequestBlockedReason::kSubresourceFilter;
     }
   }

third_party/WebKit/Source/core/loader/BaseFetchContext.h

@@ -14,6 +14,7 @@
 #include "platform/weborigin/ReferrerPolicy.h"
 #include "platform/wtf/Optional.h"
 #include "public/platform/WebAddressSpace.h"
+#include "public/platform/WebURLRequest.h"
 
 namespace blink {
 
@@ -67,7 +68,9 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
   virtual bool ShouldBypassMainWorldCSP() const = 0;
   virtual bool IsSVGImageChromeClient() const = 0;
   virtual bool ShouldBlockFetchByMixedContentCheck(
-      const ResourceRequest&,
+      WebURLRequest::RequestContext,
+      WebURLRequest::FrameType,
+      ResourceRequest::RedirectStatus,
       const KURL&,
       SecurityViolationReportingPolicy) const = 0;
   virtual bool ShouldBlockFetchAsCredentialedSubresource(const ResourceRequest&,
@@ -85,6 +88,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
   void PrintAccessDeniedMessage(const KURL&) const;
   void AddCSPHeaderIfNecessary(Resource::Type, ResourceRequest&);
 
+ private:
   // Utility methods that are used in default implement for CanRequest,
   // CanFollowRedirect and AllowResponse.
   ResourceRequestBlockedReason CanRequestInternal(
@@ -96,7 +100,6 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
       FetchParameters::OriginRestriction,
       ResourceRequest::RedirectStatus) const;
 
- private:
   ResourceRequestBlockedReason CheckCSPForRequestInternal(
       WebURLRequest::RequestContext,
       const KURL&,

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

@@ -58,7 +58,9 @@ class MockBaseFetchContext final : public BaseFetchContext {
   void CountUsage(WebFeature) const override {}
   void CountDeprecation(WebFeature) const override {}
   bool ShouldBlockFetchByMixedContentCheck(
-      const ResourceRequest&,
+      WebURLRequest::RequestContext,
+      WebURLRequest::FrameType,
+      ResourceRequest::RedirectStatus,
       const KURL&,
       SecurityViolationReportingPolicy) const override {
     return false;

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

@@ -940,15 +940,18 @@ void FrameFetchContext::CountDeprecation(WebFeature feature) const {
 }
 
 bool FrameFetchContext::ShouldBlockFetchByMixedContentCheck(
-    const ResourceRequest& resource_request,
+    WebURLRequest::RequestContext request_context,
+    WebURLRequest::FrameType frame_type,
+    ResourceRequest::RedirectStatus redirect_status,
     const KURL& url,
     SecurityViolationReportingPolicy reporting_policy) const {
   if (IsDetached()) {
     // TODO(yhirano): Implement the detached case.
     return false;
   }
-  return MixedContentChecker::ShouldBlockFetch(GetFrame(), resource_request,
-                                               url, reporting_policy);
+  return MixedContentChecker::ShouldBlockFetch(GetFrame(), request_context,
+                                               frame_type, redirect_status, url,
+                                               reporting_policy);
 }
 
 bool FrameFetchContext::ShouldBlockFetchAsCredentialedSubresource(

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

@@ -193,7 +193,9 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext {
   void CountUsage(WebFeature) const override;
   void CountDeprecation(WebFeature) const override;
   bool ShouldBlockFetchByMixedContentCheck(
-      const ResourceRequest&,
+      WebURLRequest::RequestContext,
+      WebURLRequest::FrameType,
+      ResourceRequest::RedirectStatus,
       const KURL&,
       SecurityViolationReportingPolicy) const override;
   bool ShouldBlockFetchAsCredentialedSubresource(const ResourceRequest&,

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

@@ -426,7 +426,9 @@ bool MixedContentChecker::ShouldBlockFetch(
 bool MixedContentChecker::ShouldBlockFetchOnWorker(
     WorkerOrWorkletGlobalScope* global_scope,
     WebWorkerFetchContext* worker_fetch_context,
-    const ResourceRequest& request,
+    WebURLRequest::RequestContext request_context,
+    WebURLRequest::FrameType frame_type,
+    ResourceRequest::RedirectStatus redirect_status,
     const KURL& url,
     SecurityViolationReportingPolicy reporting_policy) {
   if (!MixedContentChecker::IsMixedContent(global_scope->GetSecurityOrigin(),
@@ -437,7 +439,7 @@ bool MixedContentChecker::ShouldBlockFetchOnWorker(
   UseCounter::Count(global_scope, WebFeature::kMixedContentPresent);
   UseCounter::Count(global_scope, WebFeature::kMixedContentBlockable);
   if (ContentSecurityPolicy* policy = global_scope->GetContentSecurityPolicy())
-    policy->ReportMixedContent(url, request.GetRedirectStatus());
+    policy->ReportMixedContent(url, redirect_status);
 
   // Blocks all mixed content request from worklets.
   // TODO(horo): Revise this when the spec is updated.
@@ -477,7 +479,7 @@ bool MixedContentChecker::ShouldBlockFetchOnWorker(
 
   if (reporting_policy == SecurityViolationReportingPolicy::kReport) {
     LogToConsoleAboutFetch(global_scope, global_scope->Url(), url,
-                           request.GetRequestContext(), allowed, nullptr);
+                           request_context, allowed, nullptr);
   }
   return !allowed;
 }
@@ -691,7 +693,7 @@ WebMixedContentContextType MixedContentChecker::ContextTypeForInspector(
   if (!mixed_frame)
     return WebMixedContentContextType::kNotMixedContent;
 
-  // See comment in shouldBlockFetch() about loading the main resource of a
+  // See comment in ShouldBlockFetch() about loading the main resource of a
   // subframe.
   if (request.GetFrameType() == WebURLRequest::kFrameTypeNested &&
       !SchemeRegistry::ShouldTreatURLSchemeAsCORSEnabled(

third_party/WebKit/Source/core/loader/MixedContentChecker.h

@@ -72,20 +72,12 @@ class CORE_EXPORT MixedContentChecker final {
                                const KURL&,
                                SecurityViolationReportingPolicy =
                                    SecurityViolationReportingPolicy::kReport);
-  static bool ShouldBlockFetch(
-      LocalFrame* frame,
-      const ResourceRequest& request,
-      const KURL& url,
-      SecurityViolationReportingPolicy reporting_policy =
-          SecurityViolationReportingPolicy::kReport) {
-    return ShouldBlockFetch(frame, request.GetRequestContext(),
-                            request.GetFrameType(), request.GetRedirectStatus(),
-                            url, reporting_policy);
-  }
 
   static bool ShouldBlockFetchOnWorker(WorkerOrWorkletGlobalScope*,
                                        WebWorkerFetchContext*,
-                                       const ResourceRequest&,
+                                       WebURLRequest::RequestContext,
+                                       WebURLRequest::FrameType,
+                                       ResourceRequest::RedirectStatus,
                                        const KURL&,
                                        SecurityViolationReportingPolicy);
 

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

@@ -148,12 +148,14 @@ void WorkerFetchContext::CountDeprecation(WebFeature feature) const {
 }
 
 bool WorkerFetchContext::ShouldBlockFetchByMixedContentCheck(
-    const ResourceRequest& resource_request,
+    WebURLRequest::RequestContext request_context,
+    WebURLRequest::FrameType frame_type,
+    ResourceRequest::RedirectStatus redirect_status,
     const KURL& url,
     SecurityViolationReportingPolicy reporting_policy) const {
   return MixedContentChecker::ShouldBlockFetchOnWorker(
-      global_scope_, web_context_.get(), resource_request, url,
-      reporting_policy);
+      global_scope_, web_context_.get(), request_context, frame_type,
+      redirect_status, url, reporting_policy);
 }
 
 bool WorkerFetchContext::ShouldBlockFetchAsCredentialedSubresource(

third_party/WebKit/Source/core/loader/WorkerFetchContext.h

@@ -49,7 +49,9 @@ class WorkerFetchContext final : public BaseFetchContext {
   void CountUsage(WebFeature) const override;
   void CountDeprecation(WebFeature) const override;
   bool ShouldBlockFetchByMixedContentCheck(
-      const ResourceRequest&,
+      WebURLRequest::RequestContext,
+      WebURLRequest::FrameType,
+      ResourceRequest::RedirectStatus,
       const KURL&,
       SecurityViolationReportingPolicy) const override;
   bool ShouldBlockFetchAsCredentialedSubresource(const ResourceRequest&,