-
Notifications
You must be signed in to change notification settings - Fork 29.7k
CupertinoDialogRoute leak fix [prod-leak-fix]
#148774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CupertinoDialogRoute leak fix [prod-leak-fix]
#148774
Conversation
|
@polina-c Looks like previous changes for hopefully this closes |
| curve: Curves.easeInOut, | ||
| ); | ||
| } | ||
|
|
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 you assert here that _fadeAnimation != null.
It will improve code readability and verify our assumptions are correct even after future changes.
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.
Actually it may be better to create local:
final fadeAnimation = _fadeAnimation!
And then use it without !
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.
Would like me to only add local variable change
final fadeAnimation = _fadeAnimation!And then use it without
!
Or both assert and local variable change
It's a bit confusing because of the below line:
Actually it may be better to create a local:
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.
With local variable assert is not needed, because the expression contains '!', that will fail in case of null.
So, just local.
|
Approved to kick off google testing. The comments still need to be addressed. |
|
@polina-c Added change as mentioned and it seems google test have passed as well 🥂 |
flutter/flutter@d02292d...73bf206 2024-05-22 [email protected] `CupertinoDialogRoute` leak fix (flutter/flutter#148774) 2024-05-22 [email protected] Marks Windows plugin_test to be flaky (flutter/flutter#148835) 2024-05-22 [email protected] Add tests for actions.0.dart API example. (flutter/flutter#148678) 2024-05-22 [email protected] Introduce `WidgetStateBorderSide.lerp` (flutter/flutter#148122) 2024-05-22 [email protected] add `default-flavor` field to flutter pubspec, which will be used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter/flutter#147968) 2024-05-22 [email protected] [wiki migration] Pages under docs/postmortems/ (flutter/flutter#148798) 2024-05-22 [email protected] Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter/flutter#148812) 2024-05-22 [email protected] Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter/flutter#148808) 2024-05-21 [email protected] Fix two dimensional viewport unexpected null exception when no child is laid out (flutter/flutter#148256) 2024-05-21 [email protected] Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter/flutter#148807) 2024-05-21 [email protected] Add test for undo_history_controller.0.dart (flutter/flutter#148205) 2024-05-21 [email protected] Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter/flutter#148802) 2024-05-21 [email protected] Fix test that leaks images. (flutter/flutter#148494) 2024-05-21 [email protected] Fix warnings in `dependency_version_checker.gradle.kts` (flutter/flutter#148699) 2024-05-21 [email protected] [wiki migration] Android team pages (flutter/flutter#148585) 2024-05-21 [email protected] Fix leaky test. (flutter/flutter#148788) 2024-05-21 [email protected] Add DropdownButton.menuWidth (flutter/flutter#148125) 2024-05-21 [email protected] Add test for focus example 2 (flutter/flutter#147624) 2024-05-21 [email protected] Add a migrator to remove `FlutterMultiDexApplication.java` (flutter/flutter#148515) 2024-05-21 [email protected] [wiki migration] Infra team pages (flutter/flutter#148718) 2024-05-21 [email protected] Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter/flutter#148776) 2024-05-21 [email protected] Fix the output of the CDN test. (flutter/flutter#148730) 2024-05-21 [email protected] [wiki migration] Release team pages (flutter/flutter#148723) 2024-05-21 [email protected] Remove hidden dependencies on LocalProcessManager (flutter/flutter#148096) 2024-05-21 [email protected] Adds Missing `onHover` & `onFocusChange` for `OutlinedButton.icon` (flutter/flutter#144374) 2024-05-21 [email protected] Adds tests to NestedScrollView examples (flutter/flutter#148170) 2024-05-21 [email protected] Roll Flutter Engine from c2ef01f6f1ab to 8a352f01e503 (18 revisions) (flutter/flutter#148766) 2024-05-21 [email protected] `switch` expressions: finale (flutter/flutter#148711) 2024-05-21 [email protected] [iOS] specify minimum os version for native asset frameworks (flutter/flutter#148504) 2024-05-21 [email protected] Removed brand references from MenuAnchor.dart (flutter/flutter#148760) 2024-05-21 [email protected] Skip flaky test in expression_evaluation_test.dart (flutter/flutter#148737) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages 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 Packages: 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
…ps://github.com/hello-coder-xu/flutter into fix/_floatingActionButtonVisibilityValue-update * 'fix/_floatingActionButtonVisibilityValue-update' of https://github.com/hello-coder-xu/flutter: fix: update _floatingActionButtonVisibility only if floatingActionButton is not null [wiki migration] Remaining pages under docs/about/ (flutter#148782) Roll Flutter Engine from b6971cdf14f8 to 8b094fbb94d8 (3 revisions) (flutter#148883) Fix the second TextFormField to trigger onTapOutside (flutter#148206) Try removing robolectric from `integration_test` tests (flutter#148803) Prevent test folder deletion on running `flutter create --empty` on an existing app project (flutter#147160) [wiki migration] Tool team pages (flutter#148779) Roll Flutter Engine from c89defa55801 to b6971cdf14f8 (6 revisions) (flutter#148819) [native_assets] Add support for link hooks (flutter#148474) Roll Packages from ba19b24 to 6525441 (12 revisions) (flutter#148864) Update tokens to 4.0.0 (flutter#148789) Move Linux web_long_running_tests_2_5 to bringup (flutter#148854) `CupertinoDialogRoute` leak fix (flutter#148774) Marks Windows plugin_test to be flaky (flutter#148835)
CupertinoDialogRoute leak fixCupertinoDialogRoute leak fix [prod-leak-fix]
part of #141198
Reattempts to fix
CupertinoDialogRoutethe only thing left close CurvedAnimation leaks.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.