Apply DelayAsyncScriptExecution only for cross site scripts

This CL adds kDelayAsyncScriptExecutionCrossSiteOnlyParam feature param so that DelayAsyncScriptExecution is applied only for cross site scripts. Bug: 1340837 Change-Id: Ib6d83174e244f770af29c4c99350ec03cdc2ad83 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3816163 Reviewed-by: Shunya Shishido <sisidovski@chromium.org> Commit-Queue: Minoru Chikamune <chikamune@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/main@{#1034328}
chromium · Aug 12, 2022 · ff4716e · ff4716e
1 parent 28c67b8
commit ff4716e
Show file tree

Hide file tree

Showing 15 changed files with 83 additions and 31 deletions.
diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc
@@ -1510,6 +1510,9 @@ const base::FeatureParam<DelayAsyncScriptDelayType>
         DelayAsyncScriptDelayType::kFinishedParsing,
         &delay_async_script_execution_delay_types};
 
+const base::FeatureParam<bool> kDelayAsyncScriptExecutionCrossSiteOnlyParam{
+    &kDelayAsyncScriptExecution, "cross_site_only", false};
+
 const base::Feature kForceDeferScriptIntervention{
     "ForceDeferScriptIntervention", base::FEATURE_DISABLED_BY_DEFAULT};
 

diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h
@@ -750,6 +750,8 @@ enum class DelayAsyncScriptDelayType {
 };
 BLINK_COMMON_EXPORT extern const base::FeatureParam<DelayAsyncScriptDelayType>
     kDelayAsyncScriptExecutionDelayParam;
+BLINK_COMMON_EXPORT extern const base::FeatureParam<bool>
+    kDelayAsyncScriptExecutionCrossSiteOnlyParam;
 
 // If enabled, parser-blocking scripts are force-deferred.
 // https://crbug.com/1339112

diff --git a/third_party/blink/renderer/core/script/classic_pending_script.cc b/third_party/blink/renderer/core/script/classic_pending_script.cc
@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/script/classic_pending_script.h"
 
 #include "base/feature_list.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/script/script_type.mojom-blink-forward.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/bindings/core/v8/referrer_script_info.h"
@@ -60,6 +61,27 @@ namespace {
 KURL BaseUrl(const ScriptResource& resource) {
   return resource.GetResponse().ResponseUrl();
 }
+
+bool CheckIfEligibleForDelay(const KURL& url,
+                             const Document& element_document,
+                             const ScriptElementBase& element) {
+  if (!base::FeatureList::IsEnabled(features::kDelayAsyncScriptExecution))
+    return false;
+
+  if (element.IsPotentiallyRenderBlocking())
+    return false;
+
+  if (features::kDelayAsyncScriptExecutionCrossSiteOnlyParam.Get()) {
+    ExecutionContext* context = element_document.GetExecutionContext();
+    scoped_refptr<const SecurityOrigin> src_security_origin =
+        SecurityOrigin::Create(url);
+    if (src_security_origin->IsSameSiteWith(context->GetSecurityOrigin()))
+      return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 // <specdef href="https://html.spec.whatwg.org/C/#fetch-a-classic-script">
@@ -80,7 +102,8 @@ ClassicPendingScript* ClassicPendingScript::Fetch(
       MakeGarbageCollected<ClassicPendingScript>(
           element, TextPosition::MinimumPosition(), KURL(), KURL(), String(),
           ScriptSourceLocationType::kExternalFile, options,
-          true /* is_external */);
+          true /* is_external */,
+          CheckIfEligibleForDelay(url, element_document, *element));
 
   // [Intervention]
   // For users on slow connections, we want to avoid blocking the parser in
@@ -114,7 +137,8 @@ ClassicPendingScript* ClassicPendingScript::CreateInline(
   ClassicPendingScript* pending_script =
       MakeGarbageCollected<ClassicPendingScript>(
           element, starting_position, source_url, base_url, source_text,
-          source_location_type, options, false /* is_external */);
+          source_location_type, options, false /* is_external */,
+          false /* is_eligible_for_delay */);
   pending_script->CheckState();
   return pending_script;
 }
@@ -127,7 +151,8 @@ ClassicPendingScript::ClassicPendingScript(
     const String& source_text_for_inline_script,
     ScriptSourceLocationType source_location_type,
     const ScriptFetchOptions& options,
-    bool is_external)
+    bool is_external,
+    bool is_eligible_for_delay)
     : PendingScript(element, starting_position),
       options_(options),
       source_url_for_inline_script_(source_url_for_inline_script),
@@ -136,7 +161,8 @@ ClassicPendingScript::ClassicPendingScript(
       source_location_type_(source_location_type),
       is_external_(is_external),
       ready_state_(is_external ? kWaitingForResource : kReady),
-      integrity_failure_(false) {
+      integrity_failure_(false),
+      is_eligible_for_delay_(is_eligible_for_delay) {
   CHECK(GetElement());
 
   if (is_external_) {
@@ -219,8 +245,10 @@ bool ClassicPendingScript::IsEligibleForDelay() const {
   // We don't delay async scripts that have matched a resource in the preload
   // cache, because we're using <link rel=preload> as a signal that the script
   // is higher-than-usual priority, and therefore should be executed earlier
-  // rather than later.
-  return !GetResource()->IsLinkPreload();
+  // rather than later. IsLinkPreload() can't be checked in
+  // CheckIfEligibleForDelay() since ClassicPendingScript::Fetch() initialize
+  // the state.
+  return is_eligible_for_delay_ && !GetResource()->IsLinkPreload();
 }
 
 void ClassicPendingScript::NotifyFinished(Resource* resource) {

diff --git a/third_party/blink/renderer/core/script/classic_pending_script.h b/third_party/blink/renderer/core/script/classic_pending_script.h
@@ -60,7 +60,8 @@ class CORE_EXPORT ClassicPendingScript final : public PendingScript,
                        const String& source_text_for_inline_script,
                        ScriptSourceLocationType,
                        const ScriptFetchOptions&,
-                       bool is_external);
+                       bool is_external,
+                       bool is_eligible_for_delay);
   ~ClassicPendingScript() override;
 
   void Trace(Visitor*) const override;
@@ -137,6 +138,7 @@ class CORE_EXPORT ClassicPendingScript final : public PendingScript,
   const bool is_external_;
   ReadyState ready_state_;
   bool integrity_failure_;
+  const bool is_eligible_for_delay_;
 
   // The request is intervened by document.write() intervention.
   bool intervened_ = false;

diff --git a/third_party/blink/renderer/core/script/script_runner.cc b/third_party/blink/renderer/core/script/script_runner.cc
@@ -56,13 +56,10 @@ ScriptRunner::DelayReasons ScriptRunner::DetermineDelayReasonsToWait(
     PendingScript* pending_script) {
   DelayReasons reasons = static_cast<DelayReasons>(DelayReason::kLoad);
 
-  if (base::FeatureList::IsEnabled(features::kDelayAsyncScriptExecution)) {
-    if (pending_script->IsEligibleForDelay() &&
-        !pending_script->GetElement()->IsPotentiallyRenderBlocking() &&
-        (active_delay_reasons_ &
-         static_cast<DelayReasons>(DelayReason::kMilestone))) {
-      reasons |= static_cast<DelayReasons>(DelayReason::kMilestone);
-    }
+  if (pending_script->IsEligibleForDelay() &&
+      (active_delay_reasons_ &
+       static_cast<DelayReasons>(DelayReason::kMilestone))) {
+    reasons |= static_cast<DelayReasons>(DelayReason::kMilestone);
   }
 
   if (base::FeatureList::IsEnabled(features::kForceDeferScriptIntervention)) {

diff --git a/third_party/blink/web_tests/VirtualTestSuites b/third_party/blink/web_tests/VirtualTestSuites
@@ -1093,6 +1093,12 @@
     "bases": ["wpt_internal/async-script-scheduling"],
     "args": ["--disable-features=DelayAsyncScriptExecution"]
   },
+  {
+    "prefix": "async-script-scheduling-apply-to-cross-site-only",
+    "platforms": ["Linux", "Mac", "Win"],
+    "bases": ["wpt_internal/async-script-scheduling"],
+    "args": ["--enable-features=DelayAsyncScriptExecution:cross_site_only/true"]
+  },
   {
     "prefix": "async-script-scheduling-finished-parsing",
     "platforms": ["Linux", "Mac", "Win"],

diff --git a/...-to-cross-site-only/wpt_internal/async-script-scheduling/execution-order.sub-expected.txt b/...-to-cross-site-only/wpt_internal/async-script-scheduling/execution-order.sub-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Async Script Execution Order Uncaught Error: assert_equals: expected "async_preload;async_blocking_render;async;async;inline1;external;inline2;defer;DOMContentLoaded" but got "async_preload;async_blocking_render;async;inline1;external;inline2;defer;DOMContentLoaded;async"
+Harness: the test ran to completion.
+
diff --git a/...duling-finished-parsing/wpt_internal/async-script-scheduling/execution-order-expected.txt b/...duling-finished-parsing/wpt_internal/async-script-scheduling/execution-order-expected.txt
diff --git a/...ng-finished-parsing/wpt_internal/async-script-scheduling/execution-order.sub-expected.txt b/...ng-finished-parsing/wpt_internal/async-script-scheduling/execution-order.sub-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Async Script Execution Order Uncaught Error: assert_equals: expected "async_preload;async_blocking_render;async;async;inline1;external;inline2;defer;DOMContentLoaded" but got "async_preload;async_blocking_render;inline1;external;inline2;defer;DOMContentLoaded;async;async"
+Harness: the test ran to completion.
+
diff --git a/...int-or-finished-parsing/wpt_internal/async-script-scheduling/execution-order-expected.txt b/...int-or-finished-parsing/wpt_internal/async-script-scheduling/execution-order-expected.txt
diff --git a/...or-finished-parsing/wpt_internal/async-script-scheduling/execution-order.sub-expected.txt b/...or-finished-parsing/wpt_internal/async-script-scheduling/execution-order.sub-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Async Script Execution Order Uncaught Error: assert_equals: expected "async_preload;async_blocking_render;async;async;inline1;external;inline2;defer;DOMContentLoaded" but got "async_preload;async_blocking_render;inline1;external;inline2;defer;DOMContentLoaded;async;async"
+Harness: the test ran to completion.
+
diff --git a/...nk/web_tests/virtual/async-script-scheduling-apply-to-cross-site-only/README.md b/...nk/web_tests/virtual/async-script-scheduling-apply-to-cross-site-only/README.md
@@ -0,0 +1,5 @@
+# DelayAsyncScriptExecution
+This suite runs the tests in wpt_internal/async-script-scheduling/ with
+`--enable-features=DelayAsyncScriptExecution:cross_site_only/true`.
+
+See crbug.com/1340837.
diff --git a/...nc-script-scheduling/execution-order.html → ...cript-scheduling/execution-order.sub.html b/...nc-script-scheduling/execution-order.html → ...cript-scheduling/execution-order.sub.html
@@ -3,9 +3,8 @@
   <title>Async Script Execution Order</title>
   <script src="/resources/testharness.js"></script>
   <script src="/resources/testharnessreport.js"></script>
-  <link rel="preload" as="script" href="resources/script.js?4&preload">
-  <script async blocking=render src="resources/script.js?1&pipe=trickle(d1)"
-          data-label="async_blocking_render"></script>
+  <link rel="preload" as="script" href="resources/script.sub.js?label=async_preload">
+  <script async blocking=render src="resources/script.sub.js?pipe=trickle(d1)&label=async_blocking_render"></script>
 </head>
 <body>
   <!-- Note: This test serves mainly to document the current scheduling behavior
@@ -21,22 +20,28 @@
     }
     window.onload = e => {
       const actualOrder = logs.join(";");
-      const expectedOrder = "async_preload;async_blocking_render;async;inline1;external;inline2;defer;DOMContentLoaded";
+      const expectedOrder = "async_preload;async_blocking_render;async;async;inline1;external;inline2;defer;DOMContentLoaded";
       assert_equals(actualOrder, expectedOrder);
       done();
     };
     document.addEventListener('DOMContentLoaded', e => { logScript('DOMContentLoaded'); });
   </script>
 
-  <script defer src="resources/script.js?2" data-label="defer"></script>
-  <script async src="resources/script.js?3&pipe=trickle(d2)" data-label="async"></script>
-  <script async src="resources/script.js?4&preload" data-label="async_preload"></script>
+  <script defer src="resources/script.sub.js?label=defer"></script>
+  <!-- Note: The following two async scripts are named as
+       "label=async". These are intentionally named with the same name
+       because the ordering of these two scripts are not guaranteed even
+       if different trickle values are specified. -->
+  <script async src="resources/script.sub.js?pipe=trickle(d2)&label=async"></script>
+  <!-- cross site async script -->
+  <script async src="http://{{hosts[alt][]}}:{{ports[http][0]}}/wpt_internal/async-script-scheduling/resources/script.sub.js?pipe=trickle(d2)&label=async"></script>
+  <script async src="resources/script.sub.js?label=async_preload"></script>
   <link rel=stylesheet href=resources/main.css?pipe=trickle(d3)>
   <script>
     logScript('inline1');
     assert_equals('rgb(255, 255, 0)', getComputedStyle(document.body)['backgroundColor']);
   </script>
-  <script src="resources/script.js?5&pipe=trickle(d1)" data-label="external"></script>
+  <script src="resources/script.sub.js?pipe=trickle(d1)&label=external"></script>
   <script>
     logScript('inline2');
   </script>

diff --git a/third_party/blink/web_tests/wpt_internal/async-script-scheduling/resources/script.js b/third_party/blink/web_tests/wpt_internal/async-script-scheduling/resources/script.js
diff --git a/third_party/blink/web_tests/wpt_internal/async-script-scheduling/resources/script.sub.js b/third_party/blink/web_tests/wpt_internal/async-script-scheduling/resources/script.sub.js
@@ -0,0 +1 @@
+logScript("{{GET[label]}}");