Set referrer string and referrer policy separately for module script fetching

There is no observable result of this change. Before this CL,
ModuleScriptFetchRequest stored a blink::Referrer and a
blink::ScriptFetchOptions. This effectively stored two blink::ReferrerPolicy
members under ModuleScriptFetchRequest. We also generated ModuleScriptFetchRequest's
Referrer in two different places in ModuleTreeLinker. Finally ModuleScriptFetchRequest's
Referrer member was used in ModuleScriptLoader::FetchInternal to set ResourceRequest's
referrer header.

After this CL, we store only a referrer string in ModuleScriptFetchRequest,
as the spec indicates, and do not generate a blink::Referrer in ModuleTreeLinker.
Then in ModuleScriptLoader::FetchInternal, we generate and set ResourceRequest's final
referrer. This also leaves a TODO, to stop storing ResourceRequest's referrer as
a header value.

R=kinuko@chromium.org, kouhei@chromium.org, nhiroki@chromium.org, yhirano@chromium.org

Bug: 863769
Change-Id: Id22467f5c93e59b33ca01571addeceb1a505a1af
Reviewed-on: https://chromium-review.googlesource.com/1157237
Commit-Queue: Dominic Farolino <domfarolino@gmail.com>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580804}

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

@@ -544,8 +544,7 @@ static void ModulePreloadIfNeeded(const LinkLoadParameters& params,
       ScriptFetchOptions(params.nonce, integrity_metadata, params.integrity,
                          kNotParserInserted, credentials_mode,
                          params.referrer_policy),
-      Referrer(Referrer::NoReferrer(), params.referrer_policy),
-      TextPosition::MinimumPosition());
+      Referrer::NoReferrer(), TextPosition::MinimumPosition());
 
   // Step 11. "Fetch a single module script given url, settings object,
   // destination, options, settings object, "client", and with the top-level

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

@@ -480,8 +480,8 @@ class ModulePreloadTestModulator final : public DummyModulator {
     EXPECT_EQ(kNotParserInserted, request.Options().ParserState());
     EXPECT_EQ(params_->expected_credentials_mode,
               request.Options().CredentialsMode());
-    EXPECT_EQ(Referrer::NoReferrer(), request.GetReferrer().referrer);
-    EXPECT_EQ(params_->referrer_policy, request.GetReferrer().referrer_policy);
+    EXPECT_EQ(Referrer::NoReferrer(), request.ReferrerString());
+    EXPECT_EQ(params_->referrer_policy, request.Options().GetReferrerPolicy());
     EXPECT_EQ(params_->integrity,
               request.Options().GetIntegrityAttributeValue());
     EXPECT_EQ(ModuleScriptCustomFetchType::kNone, custom_fetch_type);

third_party/blink/renderer/core/loader/modulescript/module_script_fetch_request.h

@@ -8,7 +8,6 @@
 #include "third_party/blink/public/platform/web_url_request.h"
 #include "third_party/blink/renderer/platform/loader/fetch/script_fetch_options.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
-#include "third_party/blink/renderer/platform/weborigin/referrer.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
@@ -24,32 +23,32 @@ class ModuleScriptFetchRequest final {
   ModuleScriptFetchRequest(const KURL& url,
                            WebURLRequest::RequestContext destination,
                            const ScriptFetchOptions& options,
-                           const Referrer& referrer,
+                           const String& referrer_string,
                            const TextPosition& referrer_position)
       : url_(url),
         destination_(destination),
         options_(options),
-        referrer_(referrer),
+        referrer_string_(referrer_string),
         referrer_position_(referrer_position) {}
 
   static ModuleScriptFetchRequest CreateForTest(const KURL& url) {
-    return ModuleScriptFetchRequest(url, WebURLRequest::kRequestContextScript,
-                                    ScriptFetchOptions(), Referrer(),
-                                    TextPosition::MinimumPosition());
+    return ModuleScriptFetchRequest(
+        url, WebURLRequest::kRequestContextScript, ScriptFetchOptions(),
+        Referrer::ClientReferrerString(), TextPosition::MinimumPosition());
   }
   ~ModuleScriptFetchRequest() = default;
 
   const KURL& Url() const { return url_; }
   WebURLRequest::RequestContext Destination() const { return destination_; }
   const ScriptFetchOptions& Options() const { return options_; }
-  const Referrer& GetReferrer() const { return referrer_; }
+  const String& ReferrerString() const { return referrer_string_; }
   const TextPosition& GetReferrerPosition() const { return referrer_position_; }
 
  private:
   const KURL url_;
   const WebURLRequest::RequestContext destination_;
   const ScriptFetchOptions options_;
-  const Referrer referrer_;
+  const String referrer_string_;
   const TextPosition referrer_position_;
 };
 

third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc

@@ -128,7 +128,7 @@ void ModuleScriptLoader::FetchInternal(
 
   if (level == ModuleGraphLevel::kDependentModuleFetch) {
     options.initiator_info.imported_module_referrer =
-        module_request.GetReferrer().referrer;
+        module_request.ReferrerString();
     options.initiator_info.position = module_request.GetReferrerPosition();
   }
 
@@ -145,10 +145,8 @@ void ModuleScriptLoader::FetchInternal(
   // cryptographic nonce, ..." [spec text]
   fetch_params.SetContentSecurityPolicyNonce(options_.Nonce());
 
-  // [SMSR] "... its referrer policy to options's referrer policy.
-  // TODO(domfarolino): Implement this so we can set ResourceRequest's referrer
-  // string and referrer policy separately, and have the final referrer string
-  // generated in BaseFetchContext. See https://crbug.com/863769.
+  // [SMSR] "... its referrer policy to options's referrer policy." [spec text]
+  // Note: For now this is done below with SetHTTPReferrer()
 
   // Step 5. "... mode is "cors", ..."
   // [SMSR] "... and its credentials mode to options's credentials mode."
@@ -158,10 +156,17 @@ void ModuleScriptLoader::FetchInternal(
       options_.CredentialsMode());
 
   // Step 5. "... referrer is referrer, ..." [spec text]
-  // TODO(domfarolino): Use ResourceRequest::SetReferrerString here instead
-  // of SetHTTPReferrer. See https://crbug.com/863769.
+  // Note: For now this is done below with SetHTTPReferrer()
+  String referrer_string = module_request.ReferrerString();
+  if (referrer_string == Referrer::ClientReferrerString())
+    referrer_string = fetch_client_settings_object->GetOutgoingReferrer();
+
+  // TODO(domfarolino): Stop storing ResourceRequest's referrer as a
+  // blink::Referrer (https://crbug.com/850813).
   fetch_params.MutableResourceRequest().SetHTTPReferrer(
-      module_request.GetReferrer());
+      SecurityPolicy::GenerateReferrer(
+          module_request.Options().GetReferrerPolicy(),
+          fetch_params.GetResourceRequest().Url(), referrer_string));
 
   // Step 5. "... and client is fetch client settings object." [spec text]
   // -> set by ResourceFetcher

third_party/blink/renderer/core/loader/modulescript/module_tree_linker.cc

@@ -188,20 +188,10 @@ void ModuleTreeLinker::FetchRoot(const KURL& original_url,
   visited_set_.insert(url);
 
   // Step 2. Perform the internal module script graph fetching procedure given
-  // ... with the top-level module fetch flag set. ...
-  // TODO(domfarolino): We should refrain from storing referrer as a generated
-  // blink::Referrer for a couple reasons (https://crbug.com/863769):
-  // 1.) It is awkward that now both ModuleScriptFetchRequest::Referrer and
-  // ScriptFetchOptions have a referrer_policy member from this point forward.
-  // 2.) We can instead just set the referrer string here to
-  // Referrer::ClientReferrerString, communicate this to the ResourceRequest in
-  // ModuleScriptLoader::FetchInternal, and generate the final referrer in
-  // BaseFetchContext::AddAdditionalRequestHeaders.
-  ModuleScriptFetchRequest request(
-      url, destination_, options,
-      SecurityPolicy::GenerateReferrer(
-          options.GetReferrerPolicy(), url,
-          fetch_client_settings_object_->GetOutgoingReferrer()),
+  // url, settings object, destination, options, settings object, visited set,
+  // "client", and with the top-level module fetch flag set.
+  ModuleScriptFetchRequest request(url, destination_, options,
+                                   Referrer::ClientReferrerString(),
                                    TextPosition::MinimumPosition());
 
   InitiateInternalModuleScriptGraphFetching(
@@ -394,14 +384,8 @@ void ModuleTreeLinker::FetchDescendants(ModuleScript* module_script) {
     // procedure given url, fetch client settings object, destination, options,
     // module script's settings object, visited set, module script's base URL,
     // and with the top-level module fetch flag unset. ...
-    // TODO(domfarolino): We should set ModuleScriptFetchRequest's referrer
-    // string without generating a full blink::Referrer. The final generated
-    // referrer string should be generated down in BaseFetchContext.
-    // See https://crbug.com/863769.
-    ModuleScriptFetchRequest request(
-        urls[i], destination_, options,
-        SecurityPolicy::GenerateReferrer(options.GetReferrerPolicy(), urls[i],
-                                         module_script->BaseURL().GetString()),
+    ModuleScriptFetchRequest request(urls[i], destination_, options,
+                                     module_script->BaseURL().GetString(),
                                      positions[i]);
     InitiateInternalModuleScriptGraphFetching(
         request, ModuleGraphLevel::kDependentModuleFetch);