Skip to content

Conversation

@polina-c
Copy link
Contributor

@polina-c polina-c commented Apr 30, 2024

Fixes #145600

Prerequisite:

  • clean up ~400 leaking tests of CurvedAnimation.

@ValentinVignal , @Dimilkalathiya and @ksokolovskyi

I highly appreciate your help resolving leaks.
I noticed couple overlaps during last week. I appreciate the effort and do not want it to be less efficient than it could be. How about splitting work?

Suggested parts:

  1. Failures of Windows framework_tests_libraries_leak_tracking, from top of stdout
  2. Failures of Windows framework_tests_libraries_leak_tracking, from bottom of stdout
  3. Failures of Windows framework_tests_widgets_leak_tracking, from top of stdout
  4. Failures of Windows framework_tests_widgets_leak_tracking, from bottom of stdout

Overlaps still can happen, but with less probability. Feel free to pick a part.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Apr 30, 2024
@polina-c polina-c changed the title - Remaining leaks. Apr 30, 2024
@polina-c polina-c changed the title Remaining leaks. Remaining leaks of CurvedAnimation. May 1, 2024
@ValentinVignal
Copy link
Contributor

ValentinVignal commented May 4, 2024

Great idea, maybe we can split the work once #146999 is merged. For now, most of the leaks are from CupertinoPageTransition and CupertinoFullscreenDialogTransition and hopefully #146999 will fix them

@polina-c
Copy link
Contributor Author

polina-c commented May 5, 2024

Great idea, maybe we can split the work once #146999 is merged. For now, most of the leaks are from CupertinoPageTransition and CupertinoFullscreenDialogTransition and hopefully #146999 will fix them

Cool!
I did not realize this PR still fixes some leaks. If it does, should it be reflected in name in addition to janks?
I see this PR is waiting for review. In general, feel free to ping me on GitHub or twitter (PolinaCC) if a PR is waiting for something more than one business day.

@polina-c
Copy link
Contributor Author

polina-c commented May 8, 2024

Number of leaking test libraries:

  • Windows framework_tests_libraries_leak_tracking: 24
  • Windows framework_tests_widgets_leak_tracking: 3

It is way less than original ~400
Thank you for fixes!

@Dimilkalathiya
Copy link
Contributor

Dimilkalathiya commented May 20, 2024

It seems we can we only have one issue to fix to close CurvedAnimation chapter (after #147823 is merged) 👀

as of now only 3 test files are leaking:-

  • material/dialog_test.dart
  • cupertino/dialog_test.dart
  • cupertino/route_test.dart
Screenshot 2024-05-20 at 10 40 40 PM

Origin of leak for all of them is CupertinoDialogRoute in which _buildCupertinoDialogTransitions is using CurvedAnimation without any means of disposal

@polina-c
Copy link
Contributor Author

It seems we can we only have one issue to fix to close CurvedAnimation chapter (after #147823 is merged) 👀

as of now only 3 test files are leaking:-

  • material/dialog_test.dart
  • cupertino/dialog_test.dart
  • cupertino/route_test.dart

Cool!

@polina-c
Copy link
Contributor Author

polina-c commented May 26, 2024

Time to celebrate - no remaining leaks of CurvedAnimation!!!

Thank you @Dimilkalathiya , for landing last PR 🩵

I will cleanup CurvedAnimation TODOs

@polina-c polina-c closed this May 26, 2024
@polina-c polina-c reopened this May 26, 2024
@polina-c polina-c added the a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker label May 26, 2024
@polina-c polina-c changed the title Remaining leaks of CurvedAnimation. Remove opt out for CurvedAnimation. May 26, 2024
@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. f: cupertino flutter/packages/flutter/cupertino repository f: routes Navigator, Router, and related APIs. f: focus Focus traversal, gaining or losing focus labels May 26, 2024
@polina-c polina-c marked this pull request as ready for review May 27, 2024 02:18
@polina-c polina-c requested a review from goderbauer May 27, 2024 02:18
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@polina-c polina-c merged commit 65e3007 into flutter:master May 28, 2024
@polina-c polina-c deleted the count-leaks branch May 28, 2024 23:35
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 29, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 29, 2024
flutter/flutter@a1a33e6...c85fa6a

2024-05-29 [email protected] Clean leak in editable_text_test.dart. (flutter/flutter#149223)
2024-05-29 [email protected] Add tests for animated_switcher.0.dart API example. (flutter/flutter#149180)
2024-05-29 [email protected] Manual roll Flutter Engine from d0323905fc2f to b26e1b023cdb (16 revisions) (flutter/flutter#149220)
2024-05-29 [email protected] Change snack bar default hitTestBehavior to deferToChild when SnackBarThemeData.insetPadding is not null (flutter/flutter#148568)
2024-05-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "sliverGridDelegate mainAxisExtent add assert (#148470)" (flutter/flutter#149224)
2024-05-29 [email protected] sliverGridDelegate mainAxisExtent add assert (flutter/flutter#148470)
2024-05-29 [email protected] Fix `SearchAnchor` suggestions not refreshing after long API call (flutter/flutter#148767)
2024-05-28 [email protected] Add link to golden file test docs in the framework gardener guide (flutter/flutter#149207)
2024-05-28 [email protected] Remove opt out for CurvedAnimation. (flutter/flutter#147594)
2024-05-28 [email protected] Fix the RenderFlex.computeDryBaseline implementation to match computeDistanceToActualBaseline (flutter/flutter#149062)
2024-05-28 [email protected] Clean leaky test. (flutter/flutter#149199)
2024-05-28 [email protected] Change `android_plugin_new_output_dir_test.dart` test description (flutter/flutter#149198)
2024-05-28 [email protected] fix M2 InputDecorator suffix icon doesn't turn red on error (flutter/flutter#149161)
2024-05-28 [email protected] Add selectionOverlayBuilder in CupertinoDatePicker and CupertinoTimerâ�¦ (flutter/flutter#143079)
2024-05-28 [email protected] Mouse onEnter and onExit now support hovering stylus (flutter/flutter#149006)
2024-05-28 [email protected] Remove `TextEditingController` private member access (flutter/flutter#149042)
2024-05-28 [email protected] Roll Packages from b7bcb4b to a933c30 (1 revision) (flutter/flutter#149184)
2024-05-28 [email protected] Roll Flutter Engine from b1751088c7e9 to d0323905fc2f (2 revisions) (flutter/flutter#149169)
2024-05-28 [email protected] [tool] Use kebabCase directly (flutter/flutter#149150)
2024-05-28 [email protected] [wiki migration] Leftover wiki pages and links (flutter/flutter#148989)

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
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: focus Focus traversal, gaining or losing focus f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CurvedAnimation is leaking.

4 participants