[web] make addView/removeView functions sync #51103
Conversation
|
@ditman While this PR is a subject for discussion, do you think the engine will benefit if I extract the test into a separate PR, adapting it for the async case? I didn't find other tests in the repo for addView JS function when I was working on these changes |
|
@p-mazhnik it's likely that I'll land this, either in this PR or in a separate one cherry-picking this change + some stuff, I've been convinced to:
|
|
@p-mazhnik the test is indeed very useful, thanks for that, and that will land FOR SURE regardless of what happens with the synchronicity of the views. I take back what I said about renaming stuff above, let's do one change at a time, and figure out if this being sync hurts or helps :) |
|
@ditman in case you need a version of this test for the current async scenario, e.g. for internal testing, I also have the one: https://github.com/flutter/engine/compare/main...p-mazhnik:addview-test?expand=1 |
|
@p-mazhnik that's fantastic, thanks for both the implementations of the test! |
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
Thanks for the change, let's make those methods sync!
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/engine/51103, due to 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.
|
|
Please @mdebbar, can you PTAL? We need 2 approvals! |
|
Thansk @p-mazhnik!! |
…145042) flutter/engine@e25e977...ea848c3 2024-03-12 [email protected] Fix some races in the platform isolate tests (flutter/engine#51358) 2024-03-12 [email protected] Roll Skia from 187488b64570 to bbe453e3525d (1 revision) (flutter/engine#51366) 2024-03-12 [email protected] [web] make addView/removeView functions sync (flutter/engine#51103) 2024-03-12 [email protected] Roll Dart SDK from 7f26f32f374f to b19e0995f317 (1 revision) (flutter/engine#51363) 2024-03-12 [email protected] [dart:ui] Add view ID to `PointerData.toString` (flutter/engine#51352) 2024-03-12 [email protected] Fix null filter NOP case in DlLocalMatrixImageFilter (flutter/engine#51340) 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
3 participants
This PR changes the JS-accessible methods to add/remove views in a multi-view web app from async to sync.
It also adds a new add/remove view test that tests calling the actual JS methods, and verifies in the engine that the view has been registered/unregistered.
Related: flutter/flutter#138930