-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] Final nail in FlutterViewEmbedder's coffin #49769
Conversation
ditman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup @mdebbar!
I think my only "somewhat blocking" comment would be to not make the resources field a first-class citizen of the view if it's only needed by the HTML Renderer, and keep those resources as an implementation detail of the HTML renderer.
Everything else is mostly LGTM and "woah, nice!" type of comments :P
lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart
Show resolved
Hide resolved
lib/web_ui/lib/src/engine/view_embedder/global_html_attributes.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
(Odd test failures, though? Did we break some pointer binding initialization?)
00:10 +237 ~2 -1: pointer_binding_test.dart: ClickDebouncer Dedupes click if pointer down/up flushed recently [E]
Expected: [
PointerChange:PointerChange.add,
PointerChange:PointerChange.down,
PointerChange:PointerChange.up
]
Actual: []
Which: at location [0] is [] which shorter than expected
Queued up events should be flushed to the framework because the time expired before the click event arrived.
|
@ditman I couldn't repro that failure locally. Rebased the PR and everything seems to be passing.. |
…142628) flutter/engine@9ccd81d...20e5361 2024-01-31 [email protected] Roll Dart SDK from fbf1d8ebceb4 to 1f136c7b962d (1 revision) (flutter/engine#50203) 2024-01-31 [email protected] Log FlutterJSONMessageCodec decode errors before asserting (flutter/engine#50163) 2024-01-31 [email protected] [web] Final nail in FlutterViewEmbedder's coffin (flutter/engine#49769) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
ResourceManagerout ofFlutterViewEmbedder.GlobalHtmlAttributesout ofFlutterViewEmbedder.FlutterViewEmbedder.reset().flutterViewEmbedder.ensureFlutterViewEmbedderInitialized.FlutterViewEmbedder.Fixes flutter/flutter#134443