Bug 1861061: change internal priority to low for async/deferred scripts with fetchpriority=auto and to high for module-scripts and scripts from head. r=smaug

For web-compatibility; match Chromium's behavior. Differential Revision: https://phabricator.services.mozilla.com/D196954
2024-01-02 14:12:50 +00:00
parent 43ea78fedd
commit baff4c93ad
3 changed files with 92 additions and 34 deletions
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -731,34 +731,49 @@ void AdjustPriorityForNonLinkPreloadScripts(nsIChannel* aChannel,
          do_QueryInterface(aChannel)) {
    LOG(("Is not <link rel=[module]preload"));
    const RequestPriority fetchPriority = aRequest->FetchPriority();
+
    // The spec defines the priority to be set in an implementation defined
    // manner (<https://fetch.spec.whatwg.org/#concept-fetch>, step 15 and
    // <https://html.spec.whatwg.org/#concept-script-fetch-options-fetch-priority>).
-    // For web-compatibility, the fetch priority mapping from
-    // <https://web.dev/articles/fetch-priority#browser_priority_and_fetchpriority>
-    // is taken.
-    const Maybe<int32_t> supportsPriorityValue = [&]() -> Maybe<int32_t> {
+    // <testing/web-platform/mozilla/tests/fetch/fetchpriority/support/script-tests-data.js>
+    // provides more context for the priority mapping.
+    const int32_t supportsPriorityValue = [&]() {
      switch (fetchPriority) {
-        case RequestPriority::Auto:
-          return Nothing{};
+        case RequestPriority::Auto: {
+          if (aRequest->IsModuleRequest()) {
+            return nsISupportsPriority::PRIORITY_HIGH;
+          }
+
+          const ScriptLoadContext* scriptLoadContext =
+              aRequest->GetScriptLoadContext();
+          if (scriptLoadContext->IsAsyncScript() ||
+              scriptLoadContext->IsDeferredScript()) {
+            return nsISupportsPriority::PRIORITY_LOW;
+          }
+
+          if (scriptLoadContext->mScriptFromHead) {
+            return nsISupportsPriority::PRIORITY_HIGH;
+          }
+
+          return nsISupportsPriority::PRIORITY_NORMAL;
+        }
        case RequestPriority::Low: {
-          return Some(nsISupportsPriority::PRIORITY_LOW);
+          return nsISupportsPriority::PRIORITY_LOW;
        }
        case RequestPriority::High: {
-          return Some(nsISupportsPriority::PRIORITY_HIGH);
+          return nsISupportsPriority::PRIORITY_HIGH;
        }
        default: {
          MOZ_ASSERT_UNREACHABLE();
-          return Nothing{};
+          return nsISupportsPriority::PRIORITY_NORMAL;
        }
      }
    }();

    if (supportsPriorityValue) {
      LogPriorityMapping(ScriptLoader::gScriptLoaderLog,
-                         ToFetchPriority(fetchPriority),
-                         *supportsPriorityValue);
-      supportsPriority->SetPriority(*supportsPriorityValue);
+                         ToFetchPriority(fetchPriority), supportsPriorityValue);
+      supportsPriority->SetPriority(supportsPriorityValue);
    }
  }
 }
--- a/testing/web-platform/mozilla/meta/fetch/fetchpriority/fetchpriority.h2.html.ini
+++ b/testing/web-platform/mozilla/meta/fetch/fetchpriority/fetchpriority.h2.html.ini
@@ -18,6 +18,3 @@

  [image-dynamic-load.h2.html: test different 'fetchpriority' values]
    expected: FAIL
-
-  [script-initial-load-body.h2.html: test different 'fetchpriority' values]
-    expected: FAIL
--- a/testing/web-platform/mozilla/tests/fetch/fetchpriority/support/script-tests-data.js
+++ b/testing/web-platform/mozilla/tests/fetch/fetchpriority/support/script-tests-data.js
@@ -3,6 +3,12 @@ const kFetchPriorityHighRequestFileNameAndSuffix = "dummy.js?2";
 const kFetchPriorityAutoRequestFileNameAndSuffix = "dummy.js?3";
 const kNoFetchPriorityRequestFileNameAndSuffix = "dummy.js?4";

+// Mapping fetchpriority's values to internal priorities is specified as
+// implementation-defined (https://fetch.spec.whatwg.org/#concept-fetch, step
+// 15). For web-compatibility, Chromium's desired mapping is chosen, see
+// <https://web.dev/articles/fetch-priority#browser_priority_and_fetchpriority>.
+// Exceptions are commented below.
+
 const kExpectedRequestsForScriptsInHead = [
    {   fileNameAndSuffix: kFetchPriorityLowRequestFileNameAndSuffix,
        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
@@ -11,21 +17,13 @@ const kExpectedRequestsForScriptsInHead = [
        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
    },
    {   fileNameAndSuffix: kFetchPriorityAutoRequestFileNameAndSuffix,
-        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_NORMAL
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
    },
    {   fileNameAndSuffix: kNoFetchPriorityRequestFileNameAndSuffix,
-        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_NORMAL
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
    }
 ];

-// Mapping fetchpriority's values to internal priorities is specified as
-// implementation-defined (https://fetch.spec.whatwg.org/#concept-fetch, step
-// 15). For web-compatibility, Chromium's mapping is chosen, see
-// <https://web.dev/articles/fetch-priority#browser_priority_and_fetchpriority>.
-//
-// The difference of the internal priorities for late- and early-in-body scripts
-// is considered important for optimizing the LCP
-// (https://developer.mozilla.org/en-US/docs/Glossary/Largest_contentful_paint).
 const kExpectedRequestsForScriptsInBody = [
    {   fileNameAndSuffix: "dummy.js?1",
        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
@@ -33,11 +31,12 @@ const kExpectedRequestsForScriptsInBody = [
    {   fileNameAndSuffix: "dummy.js?2",
        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
    },
+    // Bug 1872654: Chromium's behavior here differs.
    {   fileNameAndSuffix: "dummy.js?3",
-        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_NORMAL
    },
    {   fileNameAndSuffix: "dummy.js?4",
-        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_NORMAL
    },
    {   fileNameAndSuffix: "dummy.js?5",
        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
@@ -55,6 +54,53 @@ const kExpectedRequestsForScriptsInBody = [

 export const kTestFolderName = "script-tests";

+const kExpectedRequestsForNonModuleAsyncAndDeferredScripts = [
+    {   fileNameAndSuffix: "dummy.js?1",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+    {   fileNameAndSuffix: "dummy.js?2",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+    },
+    {   fileNameAndSuffix: "dummy.js?3",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+    {   fileNameAndSuffix: "dummy.js?4",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+]
+
+const kExpectedRequestsForDynamicNonModuleScripts = [
+    {   fileNameAndSuffix: "dummy.js?1",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+    {   fileNameAndSuffix: "dummy.js?2",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+    },
+    {   fileNameAndSuffix: "dummy.js?3",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+    {   fileNameAndSuffix: "dummy.js?4",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+];
+
+// Chromium's desired behavior is under discussion:
+// <https://bugs.chromium.org/p/chromium/issues/detail?id=1475635>.
+const kExpectedRequestsForModuleScripts = [
+    {   fileNameAndSuffix: "dummy.js?1",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_LOW
+    },
+    {   fileNameAndSuffix: "dummy.js?2",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+    },
+    {   fileNameAndSuffix: "dummy.js?3",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+    },
+    {   fileNameAndSuffix: "dummy.js?4",
+        internalPriority: SpecialPowers.Ci.nsISupportsPriority.PRIORITY_HIGH
+    },
+]
+
 export const kTestData = [
    {   testFileName: "script-initial-load-head.h2.html",
        expectedRequests: kExpectedRequestsForScriptsInHead
@@ -63,24 +109,24 @@ export const kTestData = [
        expectedRequests: kExpectedRequestsForScriptsInBody
    },
    {   testFileName: "async-script-initial-load.h2.html",
-        expectedRequests: kExpectedRequestsForScriptsInHead
+        expectedRequests: kExpectedRequestsForNonModuleAsyncAndDeferredScripts
    },
    {   testFileName: "deferred-script-initial-load.h2.html",
-        expectedRequests: kExpectedRequestsForScriptsInHead
+        expectedRequests: kExpectedRequestsForNonModuleAsyncAndDeferredScripts
    },
    {   testFileName: "module-script-initial-load.h2.html",
-        expectedRequests: kExpectedRequestsForScriptsInHead
+        expectedRequests: kExpectedRequestsForModuleScripts
    },
    {   testFileName: "async-module-script-initial-load.h2.html",
-        expectedRequests: kExpectedRequestsForScriptsInHead
+        expectedRequests: kExpectedRequestsForModuleScripts
    },
    // Dynamic insertion executes non-speculative-parsing
    // (https://developer.mozilla.org/en-US/docs/Glossary/speculative_parsing)
-    // code paths.
+    // code paths. Moreover such inserted scripts are loaded asynchronously.
    {   testFileName: "script-dynamic-insertion.h2.html",
-        expectedRequests: kExpectedRequestsForScriptsInHead
+        expectedRequests: kExpectedRequestsForNonModuleAsyncAndDeferredScripts
    },
    {   testFileName: "module-script-dynamic-insertion.h2.html",
-        expectedRequests: kExpectedRequestsForScriptsInHead
+        expectedRequests: kExpectedRequestsForModuleScripts
    }
 ];