Bug 2056795 - Don't start deferred loads immediately if aren't supposed to. r=layout-reviewers,firefox-style-system-reviewers,dshin

Differential Revision: https://phabricator.services.mozilla.com/D313580
2026-07-23 01:22:49 +00:00
parent a2bfbab8c1
commit 7c78d5b8d2
4 changed files with 84 additions and 9 deletions
@@ -1573,14 +1573,19 @@ void Loader::NotifyObservers(SheetLoadData& aData, nsresult aStatus,
  RefPtr loadDispatcher = aData.PrepareLoadEventIfNeeded();
  if (aData.mURI) {
    aData.NotifyStop(aStatus);
-    // NOTE(emilio): This needs to happen before notifying observers, as
-    // FontFaceSet for example checks for pending sheet loads from the
-    // StyleSheetLoaded callback.
+    // NOTE(emilio): DecrementOngoingLoadCountAndMaybeUnblockOnload() needs to
+    // happen before notifying observers, as FontFaceSet for example checks for
+    // pending sheet loads from the StyleSheetLoaded callback.
    if (aData.BlocksLoadEvent()) {
      DecrementOngoingLoadCountAndMaybeUnblockOnload();
      if (mPendingLoadCount && mPendingLoadCount == mOngoingLoadCount) {
        LOG(("  No more loading sheets; starting deferred loads"));
-        StartDeferredLoads();
+        if (aCanFireEvents) {
+          StartDeferredLoads();
+        } else {
+          NS_DispatchToMainThread(NewRunnableMethod(
+              "Loader::StartDeferredLoads", this, &Loader::StartDeferredLoads));
+        }
      }
    }
  }
@@ -58,11 +58,18 @@ void SharedStyleSheetCache::LoadCompleted(SharedStyleSheetCache* aCache,
    } while ((data = data->mNext));
  }

-  // The only way of getting a load with mMustNotify = false before one with
-  // mMustNotify = true should be when we try to kick off a deferred load from a
-  // non-deferred one, and the load fails right away. In that case, it's not
-  // sound to try to fire events synchronously.
-  const bool canFireEvents = aData.mMustNotify;
+  // The only way of getting a load with mMustNotify = false before or inside
+  // one with mMustNotify = true should be when we try to kick off a deferred
+  // load from a non-deferred one, and the load fails right away. In that case,
+  // it's not sound to try to fire events synchronously.
+  const bool canFireEvents = [&] {
+    for (auto* data = &aData; data; data = data->mParentData) {
+      if (!data->mMustNotify) {
+        return false;
+      }
+    }
+    return true;
+  }();

  // 8 is probably big enough for all our common cases.  It's not likely that
  // imports will nest more than 8 deep, and multiple sheets with the same URI
@@ -0,0 +1,63 @@
+<!doctype html>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/#the-link-element">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<script>
+  async_test(function (t) {
+    let reached_end = false;
+    const BAD = "file:///nope-link-load-event-alt-002.css";
+    const BAD_CHILD = "file:///nope-link-load-event-alt-002-child.css";
+    let pendingErrors = 2;
+
+    function addLink(doc, rel, href, title) {
+      const l = doc.createElement("link");
+      l.rel = rel;
+      l.href = href;
+      if (title) l.title = title;
+      doc.head.appendChild(l);
+      return l;
+    }
+
+    function onAltError(t) {
+      assert_true(reached_end, "Shouldn't fire error event synchronously");
+      if (--pendingErrors == 0) {
+        t.done();
+      }
+    }
+
+    const f2 = document.createElement("iframe");
+    document.body.appendChild(f2);
+    const d2 = f2.contentDocument;
+    const f1 = document.createElement("iframe");
+    document.body.appendChild(f1);
+    const d1 = f1.contentDocument;
+
+    // Two ongoing loads on d2 are needed to trigger the deferral codepath for
+    // the alternate stylesheet below.
+    addLink(d2, "stylesheet", "style.css?link-load-event-alt-002-1").onerror =
+      t.unreached_func("Shouldn't error");
+    addLink(d2, "stylesheet", "style.css?link-load-event-alt-002-2").onerror =
+      t.unreached_func("Shouldn't error");
+
+    // This gets deferred, since there are two other loads ongoing in d2.
+    const alt2 = addLink(d2, "alternate stylesheet", BAD, "aaa");
+    alt2.onload = t.unreached_func("Should not load");
+    alt2.onerror = t.step_func(() => onAltError(t));
+
+    // A load for the same URL from a different document coalesces onto d2's
+    // pending entry.
+    const alt1 = addLink(d1, "alternate stylesheet", BAD, "bbb");
+    alt1.onload = t.unreached_func("Should not load");
+    alt1.onerror = t.step_func(() => onAltError(t));
+
+    // An inline stylesheet whose @import fails to open a channel
+    // synchronously. This reentrantly completes the coalesced load above
+    // while d1's loader is still in the middle of parsing this very sheet.
+    const s = d1.createElement("style");
+    s.textContent = '@import url("' + BAD_CHILD + '");';
+    d1.head.appendChild(s);
+
+    reached_end = true;
+  });
+</script>