[web] Introduce js interop to enable experimental flags on web #17099
Conversation
|
@mdebbar @hterkelsen : Can we defer load canvaskit code to eliminate -2.1% regression. Our target for Q1 is to reduce 10% and expecting CanvasKit to grow over the next 3-4 months. |
|
@ferhatb I can revert the canvaskit flag back to being a const, which will tree-shake our canvaskit-related code for users who don't enable canvas kit. This increase in code size won't affect people who enable canvas kit because they are already including canvaskit code in their bundles. But it will affect today's default mode which doesn't enable canvaskit. |
|
@mdebbar : Looking towards future, we want initial bundle to not contain canvaskit (treeshaken) , but then defer load canvaskit dart code + wasm. Yes i would revert canvaskit flag for now unless its easy to defer dart code and lazily load even for default. |
| WebExperiments._(); | ||
|
|
||
| /// Experiment flag for using the Skia-based rendering backend. | ||
| bool get useSkia => _useSkia ?? false; |
There was a problem hiding this comment.
While you're here, could you please rename this to useCanvasKit? We're consolidating on the name "CanvasKit". Keep the old FLUTTER_WEB_USE_SKIA flag for now. Changing that would be a breaking change for the framework tests.
| for (final String name in message.keys) { | ||
| final bool enabled = message[name]; | ||
| switch (name) { | ||
| case 'useSkia': |
There was a problem hiding this comment.
This name can also be useCanvasKit since nothing uses it yet.
|
When in the lifecycle do we expect for the |
I totally missed that. Good catch! Can we use JS-interop instead of platform messages? JS-interop is synchronous so we can guarantee the sequence of events w.r.t. to the start-up lifecycle. |
mdebbar
force-pushed
the
web_experiments
branch
from
659fe3a to
014a672
Compare
mdebbar
changed the title
lib/web_ui/lib/src/engine.dart
Outdated
| domRenderer; | ||
|
|
||
| // Calling this getter to force the [WebExperiments] instance to be created. | ||
| webExperiments; |
There was a problem hiding this comment.
domRenderer is actually a bad pattern to mimic. We had several bugs due to unpredictable initialization order. Consider using the ensureInitialized pattern from the various "Binding" classes we have.
| /// certain experiments at runtime without the need to access engine internals. | ||
| class WebExperiments { | ||
| WebExperiments._() { | ||
| js.context['_flutter_internal_update_experiment'] = updateExperiment; |
There was a problem hiding this comment.
I'm surprised this works without jsify or allowInterop or some such 🤔
| } | ||
|
|
||
| /// Experiment flag for using canvas-based text measurement. | ||
| bool get useCanvasText => _useCanvasText ?? false; |
There was a problem hiding this comment.
Does allowing null help with anything?
There was a problem hiding this comment.
Yes. In tests, we set the flag to a specific value (true or false), then we set it back to null when the test is done. The value null means "use the default value".
mdebbar
force-pushed
the
web_experiments
branch
from
9a76e8f to
fdedecd
Compare
mdebbar
force-pushed
the
web_experiments
branch
from
fdedecd to
c2114c4
Compare
5 participants
Initially, this will be used in benchmarks to enable/disable canvas-based text layout.
Eventually, this can be used by app developers to control what experiments they want to use in their apps. Currently, the only way to control experiments would be through environment variables which are verbose and only work in release mode.
cc @hterkelsen I added the canvas kit flag in here too. Please let me know if you want me to remove it. One side effect of this PR is that the CanvasKit-related code isn't tree-shaken out. That results in an increase of 14kb (~2.1%) in flutter_gallery's release build. I personally think this is fine because we are moving to a model where we use both html and canvaskit backends interchangeably.