[CSS Modules] Bypass fetch for Blob URL's [chromium/src : main]

0 views

Skip to first unread message

Kurt Catti-Schmidt (Gerrit)

unread,

Nov 4, 2025, 8:22:21 PM (2 days ago) Nov 4

to Blink W3C Test Autoroller, Mason Freed, Kouhei Ueno, Nate Chapin, Chromium LUCI CQ, AyeAye, Menard, Alexis, chromium...@chromium.org, blink-revie...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, hiroshig...@chromium.org, kinuko...@chromium.org, kouhei...@chromium.org, dom+...@chromium.org, loading-re...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, kinuko+...@chromium.org

Attention needed from Kouhei Ueno and Mason Freed

Kurt Catti-Schmidt voted and added 2 comments

Votes added by Kurt Catti-Schmidt

Commit-Queue

2 comments

Patchset-level comments

File-level comment, Patchset 16 (Latest):

Kurt Catti-Schmidt . resolved

@mas...@chromium.org - PTAL - this version is able to avoid the sync fetch entirely!

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

Line 220, Patchset 11: if (module_request.Options().GetSync()) {

Kouhei Ueno . resolved

Would it be possible to centralize all the change here? I don't follow why plumbing through ScriptFetchOptions are necessary.
can we peek at ModuleScriptFetchRequest and see if that's a CSS module that we are loading from blob/data url to trigger this?

Kurt Catti-Schmidt

I added the `SetSync` and `GetSync` because it's possible to create an imperative CSS module today and use a blob URL or dataURI, and I only want to change the behavior for the new declarative version.
e.g. https://codepen.io/Kurt-Catti-Schmidt/pen/vELVLoE for Blob and https://codepen.io/Kurt-Catti-Schmidt/pen/KwVGVJX for dataURI.
An alternative is to add a declarative/imperative flag, and change behaviors here based on that, but I think sync terminology makes the intent clearer, and I don't intend to add more differences between the imperative and declarative versions. Or I can change the behavior of the existing imperative version, but that doesn't feel like a safe change to make. What do you think?

Kurt Catti-Schmidt

Also, what do you think of this earlier version that kept a map of Blob URL's and then used a FileReader instead? https://chromium-review.googlesource.com/c/chromium/src/+/7093679/2

Kurt Catti-Schmidt

The latest version avoids the sync fetch entirely and adds a way to add entries to the module map. I think this will be much more performant, as it avoids the fetch entirely. Let me know what you think.

Open in Gerrit

Related details

Attention is currently required from:

Kouhei Ueno
Mason Freed

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Nov 5, 2025, 7:32:43 PM (13 hours ago) Nov 5

to Kurt Catti-Schmidt, Blink W3C Test Autoroller, Kouhei Ueno, Nate Chapin, Chromium LUCI CQ, AyeAye, Menard, Alexis, chromium...@chromium.org, blink-revie...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, hiroshig...@chromium.org, kinuko...@chromium.org, kouhei...@chromium.org, dom+...@chromium.org, loading-re...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, kinuko+...@chromium.org

Attention needed from Kouhei Ueno and Kurt Catti-Schmidt

Mason Freed voted and added 1 comment

Votes added by Mason Freed

Code-Review

1 comment

Patchset-level comments

File-level comment, Patchset 17 (Latest):

Mason Freed . resolved

Still LGTM

Open in Gerrit

Related details

Attention is currently required from:

Kouhei Ueno
Kurt Catti-Schmidt

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages