Plumb ScriptFetchOptions to ClassicPendingScript

This CL plumbs ScriptFetchOptions from ScriptLoader::PrepareScript()
to ClassicPendingScript::options_ to ClassicScript, instead of
reconstructing in ClassicPendingScript::GetSource().

Thus this CL removes ScriptLoader::nonce_.

Bug: 771486
Change-Id: I3b74da2e65176ae0c04068abba05413b675a28b0
Reviewed-on: https://chromium-review.googlesource.com/736876
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512680}

third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp

@@ -19,9 +19,61 @@
 
 namespace blink {
 
-ClassicPendingScript* ClassicPendingScript::Fetch(ScriptElementBase* element,
-                                                  FetchParameters& params,
-                                                  Document& element_document) {
+ClassicPendingScript* ClassicPendingScript::Fetch(
+    const KURL& url,
+    Document& element_document,
+    const ScriptFetchOptions& options,
+    const IntegrityMetadataSet& integrity_metadata,
+    const WTF::TextEncoding& encoding,
+    ScriptElementBase* element,
+    FetchParameters::DeferOption defer) {
+  // Step 1. Let request be the result of creating a potential-CORS request
+  // given url, ... [spec text]
+  ResourceRequest resource_request(url);
+
+  // Step 1. ... "script", ... [spec text]
+  ResourceLoaderOptions resource_loader_options;
+  resource_loader_options.initiator_info.name = element->InitiatorName();
+  FetchParameters params(resource_request, resource_loader_options);
+
+  // Step 1. ... and CORS setting. [spec text]
+  //
+  // Instead of using CrossOriginAttributeValue that corresponds to |CORS
+  // setting|, we use ScriptFetchOptions::CredentialsMode().
+  // We shouldn't call SetCrossOriginAccessControl() if CredentialsMode() is
+  // kFetchCredentialsModeOmit, because in that case the request should be
+  // no-cors, while SetCrossOriginAccessControl(kFetchCredentialsModeOmit)
+  // would result in a cors request.
+  if (options.CredentialsMode() !=
+      network::mojom::FetchCredentialsMode::kOmit) {
+    params.SetCrossOriginAccessControl(element_document.GetSecurityOrigin(),
+                                       options.CredentialsMode());
+  }
+
+  // Step 3. Set up the classic script request given request and options. [spec
+  // text]
+  //
+  // https://html.spec.whatwg.org/multipage/webappapis.html#set-up-the-classic-script-request
+  // Set request's cryptographic nonce metadata to options's cryptographic
+  // nonce, [spec text]
+  params.SetContentSecurityPolicyNonce(options.Nonce());
+
+  // its integrity metadata to options's integrity metadata, [spec text]
+  //
+  // TODO(hiroshige): Move IntegrityMetadata to ScriptFetchOptions.
+  params.SetIntegrityMetadata(integrity_metadata);
+  params.MutableResourceRequest().SetFetchIntegrity(
+      element->IntegrityAttributeValue());
+
+  // and its parser metadata to options's parser metadata. [spec text]
+  params.SetParserDisposition(options.ParserState());
+
+  params.SetCharset(encoding);
+
+  // This DeferOption logic is only for classic scripts, as we always set
+  // |kLazyLoad| for module scripts in ModuleScriptLoader.
+  params.SetDefer(defer);
+
   // [Intervention]
   // 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
@@ -29,8 +81,8 @@ ClassicPendingScript* ClassicPendingScript::Fetch(ScriptElementBase* element,
   auto* client_for_intervention =
       MaybeDisallowFetchForDocWrittenScript(params, element_document);
 
-  ClassicPendingScript* pending_script =
-      new ClassicPendingScript(element, TextPosition(), true /* is_external */);
+  ClassicPendingScript* pending_script = new ClassicPendingScript(
+      element, TextPosition(), options, true /* is_external */);
   ScriptResource* resource =
       ScriptResource::Fetch(params, element_document.Fetcher());
   if (!resource)
@@ -46,8 +98,8 @@ ClassicPendingScript* ClassicPendingScript::CreateExternalForTest(
     ScriptElementBase* element,
     ScriptResource* resource) {
   DCHECK(resource);
-  ClassicPendingScript* pending_script =
-      new ClassicPendingScript(element, TextPosition(), true /* is_external */);
+  ClassicPendingScript* pending_script = new ClassicPendingScript(
+      element, TextPosition(), ScriptFetchOptions(), true /* is_external */);
   pending_script->SetResource(resource);
   pending_script->CheckState();
   return pending_script;
@@ -55,9 +107,10 @@ ClassicPendingScript* ClassicPendingScript::CreateExternalForTest(
 
 ClassicPendingScript* ClassicPendingScript::CreateInline(
     ScriptElementBase* element,
-    const TextPosition& starting_position) {
+    const TextPosition& starting_position,
+    const ScriptFetchOptions& options) {
   ClassicPendingScript* pending_script = new ClassicPendingScript(
-      element, starting_position, false /* is_external */);
+      element, starting_position, options, false /* is_external */);
   pending_script->CheckState();
   return pending_script;
 }
@@ -65,8 +118,10 @@ ClassicPendingScript* ClassicPendingScript::CreateInline(
 ClassicPendingScript::ClassicPendingScript(
     ScriptElementBase* element,
     const TextPosition& starting_position,
+    const ScriptFetchOptions& options,
     bool is_external)
     : PendingScript(element, starting_position),
+      options_(options),
       is_external_(is_external),
       ready_state_(is_external ? kWaitingForResource : kReady),
       integrity_failure_(false),
@@ -208,19 +263,10 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url,
   DCHECK(IsReady());
 
   error_occurred = ErrorOccurred();
-  const ScriptLoader* loader = GetElement()->Loader();
-  // Here, we use the nonce snapshot at "#prepare-a-script".
-  // Note that |loader| may be nullptr if ScriptStreamerTest.
-  const String& nonce = loader ? loader->Nonce() : String();
-  const ParserDisposition parser_state = (loader && loader->IsParserInserted())
-                                             ? kParserInserted
-                                             : kNotParserInserted;
-  ScriptFetchOptions fetch_options(nonce, parser_state,
-                                   network::mojom::FetchCredentialsMode::kOmit);
   if (!is_external_) {
     ScriptSourceCode source_code(GetElement()->TextFromChildren(), document_url,
                                  StartingPosition());
-    return ClassicScript::Create(source_code, fetch_options);
+    return ClassicScript::Create(source_code, options_);
   }
 
   DCHECK(GetResource()->IsLoaded());
@@ -228,7 +274,7 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url,
                         !streamer_->StreamingSuppressed();
   ScriptSourceCode source_code(streamer_ready ? streamer_ : nullptr,
                                GetResource());
-  return ClassicScript::Create(source_code, fetch_options);
+  return ClassicScript::Create(source_code, options_);
 }
 
 void ClassicPendingScript::SetStreamer(ScriptStreamer* streamer) {

third_party/WebKit/Source/core/dom/ClassicPendingScript.h

@@ -10,6 +10,7 @@
 #include "core/dom/PendingScript.h"
 #include "core/loader/resource/ScriptResource.h"
 #include "platform/MemoryCoordinator.h"
+#include "platform/loader/fetch/FetchParameters.h"
 #include "platform/loader/fetch/ResourceOwner.h"
 
 namespace blink {
@@ -30,11 +31,17 @@ class CORE_EXPORT ClassicPendingScript final
   USING_PRE_FINALIZER(ClassicPendingScript, Prefinalize);
 
  public:
+  // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
+  //
   // For a script from an external file, calls ScriptResource::Fetch() and
   // creates ClassicPendingScript. Returns nullptr if Fetch() returns nullptr.
-  static ClassicPendingScript* Fetch(ScriptElementBase*,
-                                     FetchParameters&,
-                                     Document&);
+  static ClassicPendingScript* Fetch(const KURL&,
+                                     Document&,
+                                     const ScriptFetchOptions&,
+                                     const IntegrityMetadataSet&,
+                                     const WTF::TextEncoding&,
+                                     ScriptElementBase*,
+                                     FetchParameters::DeferOption);
 
   // For a script from an external file, with a supplied ScriptResource.
   static ClassicPendingScript* CreateExternalForTest(ScriptElementBase*,
@@ -42,7 +49,8 @@ class CORE_EXPORT ClassicPendingScript final
 
   // For an inline script.
   static ClassicPendingScript* CreateInline(ScriptElementBase*,
-                                            const TextPosition&);
+                                            const TextPosition&,
+                                            const ScriptFetchOptions&);
 
   ~ClassicPendingScript() override;
 
@@ -83,6 +91,7 @@ class CORE_EXPORT ClassicPendingScript final
 
   ClassicPendingScript(ScriptElementBase*,
                        const TextPosition&,
+                       const ScriptFetchOptions&,
                        bool is_external);
   ClassicPendingScript() = delete;
 
@@ -105,6 +114,8 @@ class CORE_EXPORT ClassicPendingScript final
   // MemoryCoordinatorClient
   void OnPurgeMemory() override;
 
+  const ScriptFetchOptions options_;
+
   const bool is_external_;
   ReadyState ready_state_;
   bool integrity_failure_;

third_party/WebKit/Source/core/dom/ScriptLoader.cpp

@@ -338,7 +338,7 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
   // 17. "If the script element has a nonce attribute,
   //      then let cryptographic nonce be that attribute's value.
   //      Otherwise, let cryptographic nonce be the empty string."
-  nonce_ = element_->GetNonceForElement();
+  String nonce = element_->GetNonceForElement();
 
   // 18. is handled below.
 
@@ -358,6 +358,12 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
                               ? TextPosition()
                               : script_start_position;
 
+  // 20. "Let options be a script fetch options whose cryptographic nonce is
+  //      cryptographic nonce, integrity metadata is integrity metadata,
+  //      parser metadata is parser metadata, and credentials mode is module
+  //      script credentials mode." [spec text]
+  ScriptFetchOptions options(nonce, parser_state, credentials_mode);
+
   // 21. "If the element has a src content attribute, run these substeps:"
   if (element_->HasSourceAttribute()) {
     // 21.1. Let src be the value of the element's src attribute.
@@ -388,14 +394,6 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
       return false;
     }
 
-    DCHECK(!prepared_pending_script_);
-
-    // 20. "Let options be a script fetch options whose cryptographic nonce is
-    //      cryptographic nonce, integrity metadata is integrity metadata,
-    //      parser metadata is parser metadata, and credentials mode is module
-    //      script credentials mode." [spec text]
-    ScriptFetchOptions options(nonce_, parser_state, credentials_mode);
-
     // 21.6. "Switch on the script's type:"
     if (GetScriptType() == ScriptType::kClassic) {
       // - "classic":
@@ -479,7 +477,7 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
         //
         // TODO(hiroshige): Implement base URL and options.
         prepared_pending_script_ =
-            ClassicPendingScript::CreateInline(element_, position);
+            ClassicPendingScript::CreateInline(element_, position, options);
 
         // 2. Set the script's script to script.
         // 3. The script is ready.
@@ -500,8 +498,7 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
         Modulator* modulator = Modulator::From(
             ToScriptStateForMainWorld(context_document->GetFrame()));
         ModuleScript* module_script = ModuleScript::Create(
-            element_->TextFromChildren(), modulator, base_url,
-            ScriptFetchOptions(nonce_, parser_state, credentials_mode),
+            element_->TextFromChildren(), modulator, base_url, options,
             kSharableCrossOrigin, position);
 
         // 3. "If this returns null, set the script's script to null and abort
@@ -680,63 +677,14 @@ bool ScriptLoader::FetchClassicScript(
     const ScriptFetchOptions& options,
     const IntegrityMetadataSet& integrity_metadata,
     const WTF::TextEncoding& encoding) {
-  // TODO(hiroshige): remove nonce from the method argument.
-  DCHECK_EQ(nonce_, options.Nonce());
-
   FetchParameters::DeferOption defer = FetchParameters::kNoDefer;
   if (!parser_inserted_ || element_->AsyncAttributeValue() ||
       element_->DeferAttributeValue())
     defer = FetchParameters::kLazyLoad;
 
-  // Step 1. Let request be the result of creating a potential-CORS request
-  // given url, ... [spec text]
-  ResourceRequest resource_request(url);
-
-  // Step 1. ... "script", ... [spec text]
-  ResourceLoaderOptions resource_loader_options;
-  resource_loader_options.initiator_info.name = element_->InitiatorName();
-  FetchParameters params(resource_request, resource_loader_options);
-
-  // Step 1. ... and CORS setting. [spec text]
-  //
-  // Instead of using CrossOriginAttributeValue that corresponds to |CORS
-  // setting|, we use ScriptFetchOptions::CredentialsMode().
-  // We shouldn't call SetCrossOriginAccessControl() if CredentialsMode() is
-  // kFetchCredentialsModeOmit, because in that case the request should be
-  // no-cors, while SetCrossOriginAccessControl(kFetchCredentialsModeOmit)
-  // would result in a cors request.
-  if (options.CredentialsMode() !=
-      network::mojom::FetchCredentialsMode::kOmit) {
-    params.SetCrossOriginAccessControl(element_document.GetSecurityOrigin(),
-                                       options.CredentialsMode());
-  }
-
-  // Step 3. Set up the classic script request given request and options. [spec
-  // text]
-  //
-  // https://html.spec.whatwg.org/multipage/webappapis.html#set-up-the-classic-script-request
-  // Set request's cryptographic nonce metadata to options's cryptographic
-  // nonce, [spec text]
-  params.SetContentSecurityPolicyNonce(options.Nonce());
-
-  // its integrity metadata to options's integrity metadata, [spec text]
-  //
-  // TODO(hiroshige): Move IntegrityMetadata to ScriptFetchOptions.
-  params.SetIntegrityMetadata(integrity_metadata);
-  params.MutableResourceRequest().SetFetchIntegrity(
-      element_->IntegrityAttributeValue());
-
-  // and its parser metadata to options's parser metadata. [spec text]
-  params.SetParserDisposition(options.ParserState());
-
-  params.SetCharset(encoding);
-
-  // This DeferOption logic is only for classic scripts, as we always set
-  // |kLazyLoad| for module scripts in ModuleScriptLoader.
-  params.SetDefer(defer);
-
-  ClassicPendingScript* pending_script =
-      ClassicPendingScript::Fetch(element_, params, element_document);
+  ClassicPendingScript* pending_script = ClassicPendingScript::Fetch(
+      url, element_document, options, integrity_metadata, encoding, element_,
+      defer);
 
   if (!pending_script)
     return false;

third_party/WebKit/Source/core/dom/ScriptLoader.h

@@ -122,8 +122,6 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
 
   void SetFetchDocWrittenScriptDeferIdle();
 
-  const String& Nonce() const { return nonce_; }
-
   // To support script streaming, the ScriptRunner may need to access the
   // PendingScript. This breaks the intended layering, so please use with
   // care. (Method is virtual to support testing.)
@@ -232,8 +230,6 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
   // as ScriptLoader is alive. crbug.com/778799
   Member<ScriptResource> resource_keep_alive_;
 
-  String nonce_;
-
   // The context document at the time when PrepareScript() is executed.
   // This is only used to check whether the script element is moved between
   // documents and thus doesn't retain a strong reference.