-
Notifications
You must be signed in to change notification settings - Fork 6k
Fixes UI freezes when multiple Flutter VC shared one engine #18816
Fixes UI freezes when multiple Flutter VC shared one engine #18816
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
gaaclarke
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.
Looks good, please add a test here: https://github.com/flutter/engine/blob/master/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm
|
Thanks for your contribution. Please see https://github.com/flutter/flutter/wiki/Testing-the-engine#objective-c---ios-embedding for how to add a test. |
|
@gaaclarke @xster Done. |
gaaclarke
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.
I updated your tests. If you add a negative test you should have a positive test too.
xster
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.
LGTM
| OCMReject([viewControllerMock surfaceUpdated:[OCMArg any]]); | ||
| } | ||
|
|
||
| - (void)testViewDidDisappearDoesPauseEngine { |
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.
Can we tweak the names a bit? Sounds like we're testing for 2 different outcomes with the same conditions :)
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.
i'll follow up with a PR so we can land this now.
Description
Assume we have two Tabs, A、B, both share the same engine, when we switch Tab from A to B, A's
viewDidDisappearis called after B'sviewWillAppear, so it break the surface creation.Related Issues
flutter/flutter#39036 (comment)
Tests
Not yet.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.