Add wasm opt-in setting #8270
Add wasm opt-in setting #8270
Conversation
packages/devtools_app/lib/src/shared/preferences/preferences.dart
Outdated
Show resolved
Hide resolved
|
EDIT: resolved this issue. See .gif in PR description above for proof of switching the renderer properly. |
| @@ -23,6 +23,9 @@ function unregisterDevToolsServiceWorker() { | |||
|
|
|||
| // Bootstrap app for 3P environments: | |||
| function bootstrapAppFor3P() { | |||
| const searchParams = new URLSearchParams(window.location.search); | |||
| const renderer = searchParams.get('renderer'); | |||
| const userConfig = renderer ? {'renderer': renderer} : {}; | |||
There was a problem hiding this comment.
Where do you store user settings? Can you read the setting directly from there? This way you don't need to patch the URL. People tend to save and share URLs, so one person's setting could leak to another person.
There was a problem hiding this comment.
Issue here is that you're not actually using the userConfig variable. You probably need to splat it into the config below, i.e.:
...
config: {
canvasKitBaseUrl: 'canvaskit/',
...userConfig,
},
...
Also, if you want JS + CanvasKit by default, you need to set the renderer to canvaskit when there is no url query parameter. The bootstrapper will use the first build it can find that works in the browser environment and abides by the configuration set by the user. The dual-compile build has the dart2wasm+skwasm build as its first option, and dart2js+canvaskit as its second option. If you want to skip the first option, you need to specify that you want canvaskit.
I would actually change the way this url query parameter is structured and just simply make it like allowWasm=true or something like that. Then, I would specify 'renderer': canvaskit if that query parameter is not found (or a non-truthy value), and then don't specify the renderer at all if allowWasm=true. That way, if someone opens it in a browser that doesn't actually support WasmGC or some of the other things skwasm needs, it will still fall back to dart2js+CanvasKit.
There was a problem hiding this comment.
We store settings on disk in the ~/.flutter-devtools/.devtools file. DevTools runs on local host, so URLs won't typically be shared unless someone is running on a remote machine and passes that URL around, which is unlikely.
| @@ -23,6 +23,9 @@ function unregisterDevToolsServiceWorker() { | |||
|
|
|||
| // Bootstrap app for 3P environments: | |||
| function bootstrapAppFor3P() { | |||
| const searchParams = new URLSearchParams(window.location.search); | |||
| const renderer = searchParams.get('renderer'); | |||
| const userConfig = renderer ? {'renderer': renderer} : {}; | |||
There was a problem hiding this comment.
Issue here is that you're not actually using the userConfig variable. You probably need to splat it into the config below, i.e.:
...
config: {
canvasKitBaseUrl: 'canvaskit/',
...userConfig,
},
...
Also, if you want JS + CanvasKit by default, you need to set the renderer to canvaskit when there is no url query parameter. The bootstrapper will use the first build it can find that works in the browser environment and abides by the configuration set by the user. The dual-compile build has the dart2wasm+skwasm build as its first option, and dart2js+canvaskit as its second option. If you want to skip the first option, you need to specify that you want canvaskit.
I would actually change the way this url query parameter is structured and just simply make it like allowWasm=true or something like that. Then, I would specify 'renderer': canvaskit if that query parameter is not found (or a non-truthy value), and then don't specify the renderer at all if allowWasm=true. That way, if someone opens it in a browser that doesn't actually support WasmGC or some of the other things skwasm needs, it will still fall back to dart2js+CanvasKit.
| // TODO(kenz): this may cause an infinite loop of reloading the page if | ||
| // the setting from storage or the query parameter indicate we should be | ||
| // loading with WASM, but each time we reload the page, something goes wrong | ||
| // and we fall back to JS. | ||
| toggleWasmEnabled(enabledFromStorage || enabledFromQueryParams); |
There was a problem hiding this comment.
@eyebrowsoffire @yjbanov is there a way to know that we tried to use WASM but had to fall back? Otherwise this could get in an infinite loop when the attempt to load with WASM fails and we fell back to JS.
This would make kIsWasm false, and we would continually try to change the value of the wasmEnabled notifier to true based on the value of (enabledFromStorage || enabledFromQueryParams). This would then trigger the listener and reload the page again, and repeat.
There was a problem hiding this comment.
I think you can just check the actual query params via JS interop (I'm not sure if DevToolsQueryParams is actually doing that or if it is some other system for composing the URL to pass back to the user) and then also look at kIsWasm. If the query parameters in the actual url have the useWasm parameter, but you're not in wasm mode, then we've fallen back to JS.
I have contemplated exposing some more rich information about what happened in the bootstrap process and why, but I don't know exactly what that would look like and we haven't implemented anything currently.
kenzieschmoll
force-pushed
the
wasm-opt-in
branch
from
f91fb31
to
be44066
Compare
|
@eyebrowsoffire @yjbanov this PR is ready for review. |
This PR adds a persistent setting to DevTools to opt in to the wasm build. Enabling this setting adds a query parameter to the DevTools URI, which the flutter bootstrap looks for to determine which web renderer to use (following the official guidance). The changes to the flutter bootstrap file are not added behind a flag, so once this PR lands, it will be possible to force wasm by adding the query parameter
wasm=true(provided that DevTools was compiled with wasm to begin with).Work towards #7856.
The setting is behind a feature flag right now (
FeatureFlags.wasmOptInSetting), but this flag has been set to true as part of this PR. We will leave the flag for easy disabling of this feature if we need to turn off the wasm-opt-in and we can remove in the future when we are confident that the wasm build of DevTools is stable.