Skip to content

Commit e564867

Browse files
KurtCattiSchmidtchromium-wpt-export-bot
KurtCattiSchmidt
authored and
chromium-wpt-export-bot
committed
[CSS Modules] Update fetching strategy
This change updates the fetch behavior to match whatwg/html#12339. Note that this modifies the code for existing import, import(), and modulepreload for consistency, but the existing behavior for these API's does *not* change in any circumstance. The only observable change is in the shadowrootadoptedstylesheets fetching behavior. By modifying the CSS module fetching to first create the CSSStyleSheet before fetching and calling replaceSync upon completion, we are able to remove the placeholder stylesheets that were being inserted for shadowrootadoptedstylesheets. This simplifies the logic considerably and will be more performant. Unlike the other API's for fetching a CSS Module Script, shadowrootadoptedstylesheets will leave behind an empty CSSStyleSheet in adoptedStyleSheets if the fetch fails. This simplifies the logic and seems like a reasonable trade off for a rare condition. Earlier versions of this CL removed it upon failure, but the implementation was considerably more complex and had other side effects. Change-Id: If00445458983f5062a1ca2d83ed0253cc171d1ff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7709812 Reviewed-by: Dan Clark <daniec@microsoft.com> Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1629010}
1 parent 68076f1 commit e564867

7 files changed

Lines changed: 335 additions & 57 deletions

‎html/semantics/scripting-1/the-script-element/css-module/css-module-parse-error.html‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<!DOCTYPE html>
2+
<title>CSS module import: parse errors produce empty stylesheet</title>
3+
<meta name="author" title="Kurt Catti-Schmidt" href="mailto:kschmi@microsoft.com" />
4+
<link rel='help' href='https://github.com/whatwg/html/pull/11687'>
5+
<script src='/resources/testharness.js'></script>
6+
<script src='/resources/testharnessreport.js'></script>
7+
<body>
8+
<script type="module">
9+
promise_test(async (t) => {
10+
// A CSS module with parse errors should still import successfully,
11+
// producing a CSSStyleSheet with no valid rules.
12+
const sheet = await import("./resources/parse-error.css",
13+
{ with: { type: "css" } });
14+
assert_equals(sheet.default.cssRules.length, 0,
15+
"Parse-error CSS module should have no valid rules.");
16+
}, "CSS module with parse errors imports successfully with empty rules.");
17+
18+
promise_test(async (t) => {
19+
// A second import of the same parse-error module should return the
20+
// same result — not a stale pre-created sheet or a different object.
21+
const sheet1 = await import("./resources/parse-error.css",
22+
{ with: { type: "css" } });
23+
const sheet2 = await import("./resources/parse-error.css",
24+
{ with: { type: "css" } });
25+
assert_equals(sheet1.default, sheet2.default,
26+
"Repeated imports should return the same CSSStyleSheet.");
27+
assert_equals(sheet1.default.cssRules.length, 0,
28+
"Sheet should still have no valid rules on second import.");
29+
}, "Repeated import of parse-error CSS module returns same sheet.");
30+
31+
promise_test(async (t) => {
32+
// A CSS module with some malformed and some valid rules should import
33+
// successfully, with only the valid rules present.
34+
const sheet = await import("./resources/malformed.css",
35+
{ with: { type: "css" } });
36+
assert_true(sheet.default.cssRules.length > 0,
37+
"Malformed CSS module should have at least one valid rule.");
38+
}, "CSS module with partially malformed CSS imports with valid rules.");
39+
</script>
40+
</body>

‎shadow-dom/declarative/tentative/shadowrootadoptedstylesheets/shadowrootadoptedstylesheets-async-fetch-disconnect-iframe.html‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@
99
<script type="module">
1010
promise_test(async (t) => {
1111
// Host is inside an iframe that is destroyed during the fetch. Destroying
12-
// the iframe invalidates its browsing context. The completion callback for
13-
// the pending module fetch should detect that the context is gone and bail
14-
// out without crashing.
12+
// the iframe invalidates its browsing context, which causes the pending
13+
// module fetch to fail. The cleanup callback must handle this without
14+
// crashing; the orphaned shadow root retains its synchronously-adopted
15+
// empty sheet.
1516
const cssUrl = new URL("./support/styles-red.css?iframe", location.href).href;
1617

1718
const iframe = document.createElement("iframe");
@@ -34,23 +35,21 @@
3435
const shadowRoot = iframeHost.shadowRoot;
3536

3637
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
37-
"Before iframe destruction: expected 1 placeholder(s).");
38+
"Before iframe destruction: one entry exists synchronously.");
3839
assert_equals(shadowRoot.adoptedStyleSheets[0].cssRules.length, 0,
39-
"Before iframe destruction: placeholder at index 0 should be empty.");
40+
"Before iframe destruction: entry at index 0 is empty pre-fetch.");
4041

4142
iframe.remove();
4243

4344
await new Promise(resolve => step_timeout(resolve, 500));
4445
await new Promise(resolve => requestAnimationFrame(resolve));
4546

47+
// After the browsing context is invalidated, the failed fetch is
48+
// observed without crashing. The empty sheet is left in place.
4649
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
47-
"Orphaned shadow root should still have one entry after iframe destruction.");
48-
49-
// The browsing context was invalidated, so the placeholder should remain empty.
50-
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
51-
"After iframe destruction: expected 1 placeholder(s).");
50+
"After iframe destruction: empty sheet remains.");
5251
assert_equals(shadowRoot.adoptedStyleSheets[0].cssRules.length, 0,
53-
"After iframe destruction: placeholder at index 0 should be empty.");
52+
"After iframe destruction: sheet remains empty.");
5453

5554
// Re-inserting the iframe creates a new browsing context with a fresh document.
5655
const iframeReloaded = new Promise(
@@ -62,9 +61,7 @@
6261
assert_equals(iframe.contentDocument.getElementById("host"), null,
6362
"The new iframe document should not contain the original host element.");
6463
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
65-
"After iframe re-insertion: expected 1 placeholder(s).");
66-
assert_equals(shadowRoot.adoptedStyleSheets[0].cssRules.length, 0,
67-
"After iframe re-insertion: placeholder at index 0 should be empty.");
64+
"After iframe re-insertion: orphaned shadow root still holds sheet.");
6865
}, "Destroying iframe during async adoptedStyleSheets fetch does not crash.");
6966
</script>
7067
</body>

‎shadow-dom/declarative/tentative/shadowrootadoptedstylesheets/shadowrootadoptedstylesheets-async-fetch-failure-shared.html‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<!DOCTYPE html>
2+
<title>shadowrootadoptedstylesheets failed fetches are sticky across consumers</title>
3+
<meta name="author" title="Kurt Catti-Schmidt" href="mailto:kschmi@microsoft.com" />
4+
<link rel='help' href='https://github.com/whatwg/html/pull/11687'>
5+
<script src='/resources/testharness.js'></script>
6+
<script src='/resources/testharnessreport.js'></script>
7+
<script src='./support/helpers.js'></script>
8+
<body>
9+
<script type="module">
10+
// Verifies that a CSS module fetch failure originating from
11+
// `shadowrootadoptedstylesheets` leaves a sticky failed entry in the module
12+
// map, so subsequent declarative or imperative consumers of the same URL
13+
// observe the failure consistently rather than seeing a stale populated
14+
// sheet from the failed attempt.
15+
//
16+
// The first declarative consumer keeps its synchronously-adopted empty
17+
// placeholder sheet after failure (no cleanup runs). Later consumers that
18+
// observe the URL only after the entry has reached the finished-failed
19+
// state get nothing, since there is no pre-created sheet to adopt.
20+
21+
promise_test(async (t) => {
22+
const url = "./support/nonexistent-shared-1.css";
23+
24+
// First consumer: declarative shadow root initiates the fetch.
25+
const { shadowRoot: first } = createStylesheetHost(url, "first_host");
26+
// Synchronous empty sheet while fetching.
27+
assert_equals(first.adoptedStyleSheets.length, 1,
28+
"First consumer: empty sheet present synchronously.");
29+
30+
await fetchAndWait(url);
31+
32+
assert_equals(first.adoptedStyleSheets.length, 1,
33+
"First consumer: empty sheet remains after failed fetch.");
34+
assert_equals(first.adoptedStyleSheets[0].cssRules.length, 0,
35+
"First consumer: sheet remains empty after failed fetch.");
36+
37+
// Second consumer: another declarative shadow root for the same URL.
38+
// The module map already holds a finished failed entry; with no
39+
// pre-created sheet to adopt, the second shadow root contributes
40+
// nothing for this specifier.
41+
const { shadowRoot: second } = createStylesheetHost(url, "second_host");
42+
assert_equals(second.adoptedStyleSheets.length, 0,
43+
"Second declarative consumer of an already-failed URL: no sheet.");
44+
}, "Failed fetch initiated by shadowrootadoptedstylesheets stays failed for later declarative consumers.");
45+
46+
promise_test(async (t) => {
47+
const url = "./support/nonexistent-shared-2.css";
48+
49+
// Declarative consumer initiates the fetch and waits for it to fail.
50+
const { shadowRoot } = createStylesheetHost(url, "decl_then_imp_host");
51+
await fetchAndWait(url);
52+
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
53+
"Declarative consumer: empty sheet remains after failed fetch.");
54+
55+
// Imperative `import` of the same URL must reject from the cached
56+
// failure — same as if the original fetch had been imperative — even
57+
// though the declarative consumer still holds an empty placeholder.
58+
await promise_rejects_js(t, TypeError,
59+
import(url, { with: { type: "css" } }),
60+
"import() of an already-failed CSS module should reject.");
61+
}, "Declarative failure: subsequent import() of the same URL rejects (consistent with imperative-first failure).");
62+
63+
promise_test(async (t) => {
64+
const url = "./support/nonexistent-shared-3.css";
65+
66+
// Imperative consumer fails first.
67+
await promise_rejects_js(t, TypeError,
68+
import(url, { with: { type: "css" } }),
69+
"import() of a 404 CSS module should reject.");
70+
71+
// A subsequent declarative consumer must observe nothing — the sticky
72+
// failed entry has no pre-created sheet to adopt.
73+
const { shadowRoot } = createStylesheetHost(url, "imp_then_decl_host");
74+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
75+
"Declarative consumer after imperative failure: no sheet.");
76+
}, "Failed import() is observed by subsequent shadowrootadoptedstylesheets.");
77+
</script>
78+
</body>

‎shadow-dom/declarative/tentative/shadowrootadoptedstylesheets/shadowrootadoptedstylesheets-async-fetch-failure.html‎

Lines changed: 134 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,44 +6,157 @@
66
<script src='/resources/testharnessreport.js'></script>
77
<script src='./support/helpers.js'></script>
88
<body>
9+
<script type="importmap">
10+
{
11+
"imports": {
12+
"blocked-by-null": null
13+
}
14+
}
15+
</script>
916
<script type="module">
17+
// A failed CSS module fetch leaves its synchronously-adopted empty sheet
18+
// in `adoptedStyleSheets`. There is no observable cleanup when the fetch
19+
// fails: the empty sheet remains, but the module map's entry is sticky-
20+
// failed so subsequent imperative imports of the same URL still reject
21+
// and subsequent declarative consumers contribute no sheet for that URL.
22+
1023
promise_test(async (t) => {
1124
// --- Scenario 1: a specifier that resolves to a 404 URL. ---
12-
// The fetch should fail gracefully; the placeholder sheet should remain
13-
// empty and no crash should occur.
25+
// Synchronously, an empty sheet is adopted while the fetch is pending.
26+
// Once the fetch fails, that empty sheet is left behind as-is.
1427
const nonexistentUrl = "./support/nonexistent.css";
15-
const { shadowRoot, testElement } = createStylesheetHost(nonexistentUrl);
28+
const { shadowRoot } = createStylesheetHost(nonexistentUrl);
1629

1730
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
18-
"Before fetch settles: expected 1 placeholder(s).");
19-
assert_equals(shadowRoot.adoptedStyleSheets[0].cssRules.length, 0,
20-
"Before fetch settles: placeholder at index 0 should be empty.");
31+
"Before fetch settles: expected 1 (empty) sheet.");
32+
const placeholder = shadowRoot.adoptedStyleSheets[0];
33+
assert_equals(placeholder.cssRules.length, 0,
34+
"Before fetch settles: sheet at index 0 should be empty.");
2135

2236
await fetchAndWait(nonexistentUrl);
2337

2438
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
25-
"After failed fetch: expected 1 placeholder(s).");
39+
"After failed fetch: empty sheet remains in adoptedStyleSheets.");
40+
assert_equals(shadowRoot.adoptedStyleSheets[0], placeholder,
41+
"After failed fetch: sheet identity is unchanged.");
2642
assert_equals(shadowRoot.adoptedStyleSheets[0].cssRules.length, 0,
27-
"After failed fetch: placeholder at index 0 should be empty.");
43+
"After failed fetch: sheet remains empty.");
44+
}, "Async fetch failure: empty placeholder sheet remains in adoptedStyleSheets.");
2845

46+
promise_test(async (t) => {
2947
// --- Scenario 2: mixed valid and invalid specifiers. ---
30-
const nonexistentUrl2 = "./support/nonexistent-2.css";
48+
// The valid sheet is populated; the invalid one stays empty. Both
49+
// entries remain in the array.
50+
const nonexistentUrl = "./support/nonexistent-2.css";
3151
const validUrl = "./support/styles.css?failure";
32-
const { shadowRoot: shadowRoot2, testElement: testElement2 } =
33-
createStylesheetHost([nonexistentUrl2, validUrl]);
52+
const { shadowRoot, testElement } =
53+
createStylesheetHost([nonexistentUrl, validUrl]);
3454

35-
assert_equals(shadowRoot2.adoptedStyleSheets.length, 2,
36-
"Two entries should be present (one placeholder, one fetched or placeholder).");
55+
assert_equals(shadowRoot.adoptedStyleSheets.length, 2,
56+
"Two entries should be present synchronously (both empty pre-fetch).");
3757

38-
await fetchAndWait(nonexistentUrl2, validUrl);
58+
await fetchAndWait(nonexistentUrl, validUrl);
3959

40-
assert_equals(shadowRoot2.adoptedStyleSheets.length, 2,
41-
"adoptedStyleSheets should still have two entries.");
42-
assert_equals(shadowRoot2.adoptedStyleSheets[0].cssRules.length, 0,
43-
"The first entry (failed fetch) should be an empty placeholder.");
44-
assertSheetRule(shadowRoot2, 1, "span { color: blue; }", "Second entry (valid)");
45-
assert_equals(getComputedStyle(testElement2).color, "rgb(0, 0, 255)",
60+
assert_equals(shadowRoot.adoptedStyleSheets.length, 2,
61+
"After settled: both sheets remain (failed and valid).");
62+
assert_equals(shadowRoot.adoptedStyleSheets[0].cssRules.length, 0,
63+
"Failed entry remains empty.");
64+
assertSheetRule(shadowRoot, 1, "span { color: blue; }",
65+
"Valid entry");
66+
assert_equals(getComputedStyle(testElement).color, "rgb(0, 0, 255)",
4667
"The valid specifier's styles (blue) should be applied.");
47-
}, "Async fetch failure: 404 leaves placeholder empty, valid specifier still works.");
68+
}, "Async fetch failure: failed sheet remains empty; valid specifier still works.");
69+
70+
promise_test(async (t) => {
71+
// JS may insert additional references to the synchronously-created
72+
// placeholder before the fetch settles. Because nothing removes the
73+
// placeholder on failure, all duplicate references remain.
74+
const nonexistentUrl = "./support/nonexistent-3.css";
75+
const { shadowRoot } = createStylesheetHost(nonexistentUrl);
76+
77+
assert_equals(shadowRoot.adoptedStyleSheets.length, 1,
78+
"Sanity: declarative path produced one (empty) sheet synchronously.");
79+
const declarativeSheet = shadowRoot.adoptedStyleSheets[0];
80+
81+
shadowRoot.adoptedStyleSheets = [
82+
declarativeSheet, declarativeSheet, declarativeSheet];
83+
assert_equals(shadowRoot.adoptedStyleSheets.length, 3,
84+
"Sanity: JS-duplicated references produced three entries.");
85+
86+
await fetchAndWait(nonexistentUrl);
87+
88+
assert_equals(shadowRoot.adoptedStyleSheets.length, 3,
89+
"After failed fetch: all duplicate references remain.");
90+
for (let i = 0; i < 3; ++i) {
91+
assert_equals(shadowRoot.adoptedStyleSheets[i], declarativeSheet,
92+
`Entry ${i} should still reference the original empty sheet.`);
93+
assert_equals(shadowRoot.adoptedStyleSheets[i].cssRules.length, 0,
94+
`Entry ${i} should still be empty.`);
95+
}
96+
}, "Async fetch failure: JS-duplicated references all remain after failure.");
97+
98+
promise_test(async (t) => {
99+
// A bare module specifier (one that is not a relative URL and has no
100+
// matching import map entry) cannot be resolved, so no fetch is
101+
// initiated and no sheet is contributed for that specifier.
102+
// adoptedStyleSheets remains empty for the host. A subsequent imperative
103+
// `import()` of the same specifier should also reject.
104+
const bareSpecifier = "bare";
105+
const { shadowRoot } = createStylesheetHost(bareSpecifier);
106+
107+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
108+
"Bare specifier contributes no sheet to adoptedStyleSheets.");
109+
110+
await promise_rejects_js(t, TypeError,
111+
import(bareSpecifier, { with: { type: "css" } }),
112+
"Imperative import of a bare specifier should reject.");
113+
114+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
115+
"After import rejection: still no sheet adopted.");
116+
}, "Async fetch failure: bare module specifier contributes no sheet.");
117+
118+
promise_test(async (t) => {
119+
// An invalid URL (containing a lone surrogate, which is not valid in
120+
// any UTF-8-encoded URL) cannot be parsed, so module specifier
121+
// resolution fails for the same reason as the bare-specifier case
122+
// above. No fetch is initiated and no sheet is contributed.
123+
const invalidUrl = "\uD800"; // Lone leading surrogate, no low surrogate.
124+
const { shadowRoot } = createStylesheetHost(invalidUrl);
125+
126+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
127+
"Invalid URL (lone surrogate) contributes no sheet to " +
128+
"adoptedStyleSheets.");
129+
130+
await promise_rejects_js(t, TypeError,
131+
import(invalidUrl, { with: { type: "css" } }),
132+
"Imperative import of an invalid URL (lone surrogate) should reject.");
133+
134+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
135+
"After import rejection: still no sheet adopted.");
136+
}, "Async fetch failure: invalid URL (lone surrogate) contributes no sheet.");
137+
138+
promise_test(async (t) => {
139+
// The specifier here is by itself a valid bare specifier, AND the
140+
// import map declared at the top of the document does have an entry
141+
// for it -- but that entry is `null`, which the HTML spec defines as
142+
// "blocked by a null entry" and surfaces to ResolveModuleSpecifier as
143+
// an invalid resolved URL. This should be skipped for
144+
// shadowrootadoptedstylesheets processing (no fetch, no sheet
145+
// contributed). A subsequent imperative `import()` of the same specifier
146+
// rejects with TypeError.
147+
const blockedSpecifier = "blocked-by-null";
148+
const { shadowRoot } = createStylesheetHost(blockedSpecifier);
149+
150+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
151+
"Specifier blocked by null import map entry contributes no sheet.");
152+
153+
await promise_rejects_js(t, TypeError,
154+
import(blockedSpecifier, { with: { type: "css" } }),
155+
"Imperative import of a specifier mapped to null should reject.");
156+
157+
assert_equals(shadowRoot.adoptedStyleSheets.length, 0,
158+
"After import rejection: still no sheet adopted.");
159+
}, "Async fetch failure: import map mapping specifier to null contributes " +
160+
"no sheet.");
48161
</script>
49162
</body>

0 commit comments

Comments
 (0)