Repository navigation
refactor(web): use singular requestFileHandle() and stream Wasm through COS - #188010
auto-submit[bot] merged 16 commits into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the WASM instantiator to support Cross-Origin Storage (COS) by attempting to retrieve and cache WASM modules using their hashes. Feedback points out potential TypeError risks: one when checking filename.includes('/') if filename is undefined, and another when using the in operator on navigator.crossOriginStorage if it is null or undefined. Both issues can be resolved using optional chaining as suggested.
|
Looks like we need to do a rebase dance |
Switch to the new singular API as adopted in: WICG/cross-origin-storage#61
ac9a210 to
e3f3b0d
Compare
|
Thanks, @kevmoo. Rebased. |
|
When will there be a chrome version that supports this? |
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/188010, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
I don't know, but there's a lot of work happening to make it happen |
|
Do we risk CRASHES in the overlap? That we certainly want to avoid. @tomayac |
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/188010, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Replace the NotAllowedError / NotFoundError branches with a single bare catch that silently falls through to the network fetch path. This makes the code robust against any future renaming of the error (see WICG/cross-origin-storage#62).
On a cache miss, tee the network response body so one branch is compiled as it arrives while the other is piped into COS, instead of buffering the whole module into a Blob and writing it in one go. On a cache hit, read the stored file as a stream rather than as a Blob. `pipeTo()` closes the writable stream once the body is exhausted, which is when the user agent verifies the bytes against the declared hash, so there is no explicit `write()`/`close()` to make. The store branch is canceled when the write cannot start, otherwise `tee()` keeps buffering the whole module for a reader that will never read it. This is the shape the explainer recommends: https://github.com/WICG/cross-origin-storage#example-streaming-a-file-into-cos-while-using-it Also restores the retrieval error messages that the previous commit rewrote, since the rename did not require changing them.
tomayac
changed the title
The previous commit reverted the defensive `filename?.includes('/')`,
`navigator.crossOriginStorage?.requestFileHandle` and `err && err.name`
forms that earlier review feedback on this PR asked for. Restore them, so
that the streaming change is the only thing this commit series adds on top
of the rename.
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/188010, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
flutter-dashboard
Bot
removed
the
autosubmit
5 participants
Two changes to the Cross-Origin Storage (COS) path in
instantiate_wasm.js.1. Singular
requestFileHandle()Switches from the deprecated plural
requestFileHandles([hash])to the singularrequestFileHandle(hash), which returns aFileSystemFileHandledirectly rather than a single-element array, and updates the feature detection accordingly. The rename was adopted in the COS spec: WICG/cross-origin-storage#61origins: '*'is passed on create. CanvasKit and Skwasm are the same bytes for every app built against a given engine revision, and without it the stored file is readable only by same-site origins, which is the one case where a shared cache buys nothing.2. Stream the module instead of buffering it
The store path used to
clone()the response, await.blob()on the clone, and write thatBlobin a singlewrite(). Nothing can be written until the last byte has arrived, and the whole module sits in memory first (5.4 MB forchromium/canvaskit.wasm, 3.6 MB forskwasm.wasm).On a cache miss the body is now
tee()d: one branch goes toWebAssembly.compileStreaming(), the other is piped into COS in the background.pipeTo()closes the writable stream once the body is exhausted, which is when the user agent verifies the bytes against the declared hash, so there is no explicitwrite()/close()to make. If the write cannot start (denied, quota exceeded, …) the store branch is canceled, otherwisetee()keeps buffering the whole module for a reader that will never read it.On a cache hit the stored file is read as
file.stream()rather than as a wholeFile. Both paths give the response a fixedContent-Type: application/wasm; COS verifies the bytes against the hash on write, so the stored MIME type is never consulted.This is the shape the explainer recommends: https://github.com/WICG/cross-origin-storage#example-streaming-a-file-into-cos-while-using-it
Testing
Tested against the Cross-Origin Storage extension v0.1.25 in Chrome 156, with
flutter createoutput built both ways and served from two ports so the second origin exercises a cross-origin COS hit. The bundledflutter.jswas rebuilt from these sources with the sameesbuild --bundle --minifystep the engine'sBUILD.gnruns.flutter build web(CanvasKit)chromium/canvaskit.wasmstored on first load, served from COS on the second origin with no network request for itflutter build web --wasm(Skwasm)skwasm.wasmstored on first load, served from COS on the second origin with no network request for itAll four combinations render the counter app and increment on tap.
Instrumenting
WritableStreamin the page shows the difference directly. Before, notee()and onewrite()carrying a single 5,447,033-byteBlob. After,tee()called once and the same 5,447,033 bytes arriving as chunked writes: 3 when the module comes from the HTTP cache, 7 when it comes over the network.Failure path: with a deliberately corrupted hash in
wasmHashes, the write rejects withDataErroron close, one warning is logged, nothing is left in COS, and the app still starts and is interactive, because the compile branch is independent of the store branch.