Add CurvedAnimation disposals in some widgets [prod-leak-fix] by ksokolovskyi · Pull Request #143790 · flutter/flutter

ksokolovskyi · 2024-02-20T22:11:07Z

Contributes to #141198

Description

Adds CurvedAnimation disposals to material/chip.dart, material/input_decorator.dart, material/toggleable.dart, widgets/animated_switcher.dart, widgets/overscroll_indicator.dart.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I updated tests to check the change I am making, or this PR is [test-exempt].
All existing and new tests are passing.

ksokolovskyi · 2024-02-20T22:17:00Z

@polina-c, could you please review this PR?

polina-c · 2024-02-21T16:51:07Z

packages/flutter/lib/src/widgets/overscroll_indicator.dart

  @override
  void dispose() {
    _glowController.dispose();
+    _decelerator.removeListener(notifyListeners);


Is it necessary to call _decelerator.removeListener(notifyListeners);?
Does not 'dispose' remove them?

If it is necessary, can you comment why?

Thanks for the review!
You are right, _glowController.dispose() removes this listener, so _decelerator.removeListener(notifyListeners) is redundant, so I've removed it in the latest commit.

polina-c

Thank you! Looks good, but one line is confusing.

Piinks · 2024-02-21T18:34:45Z

packages/flutter/lib/src/material/toggleable.dart

@@ -80,7 +80,7 @@ mixin ToggleableStateMixin<S extends StatefulWidget> on TickerProviderStateMixin
  /// To paint the actual radial reaction, [ToggleablePainter.paintRadialReaction]
  /// may be used.
  Animation<double> get reactionHoverFade => _reactionHoverFade;


Should the getters also be updated with this type change?

@Piinks, thanks for review. I've replaced Animation<double> with CurvedAnimation in all the places you mentioned.

Piinks · 2024-02-21T18:37:09Z

packages/flutter/lib/src/material/toggleable.dart

@@ -91,7 +91,7 @@ mixin ToggleableStateMixin<S extends StatefulWidget> on TickerProviderStateMixin
  /// To paint the actual radial reaction, [ToggleablePainter.paintRadialReaction]
  /// may be used.
  Animation<double> get reactionFocusFade => _reactionFocusFade;


Piinks · 2024-02-21T18:37:19Z

packages/flutter/lib/src/material/toggleable.dart

@@ -69,7 +69,7 @@ mixin ToggleableStateMixin<S extends StatefulWidget> on TickerProviderStateMixin
  /// To paint the actual radial reaction, [ToggleablePainter.paintRadialReaction]
  /// may be used.
  Animation<double> get reaction => _reaction;


Piinks

LGTM

auto-submit · 2024-02-22T17:16:33Z

auto label is removed for flutter/flutter/143790, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

ksokolovskyi · 2024-02-22T19:23:01Z

@polina-c @Piinks thanks for the review!

…r#143790)

flutter/flutter@41581c9...39585e6 2024-02-23 [email protected] Re-enable Impeller goldens blocking. (flutter/flutter#144023) 2024-02-23 [email protected] Roll Flutter Engine from 0f3ad23b84ed to 5f99a6c3289e (16 revisions) (flutter/flutter#144028) 2024-02-23 [email protected] Roll Packages from 078c2a3 to 7df2085 (3 revisions) (flutter/flutter#144030) 2024-02-23 [email protected] Run tests on iOS 17 exclusively (flutter/flutter#144022) 2024-02-23 [email protected] Retry button tap in [FlutterUITests testFlutterViewWarm] (flutter/flutter#143967) 2024-02-23 [email protected] Remove deprecated `InteractiveViewer.alignPanAxis` (flutter/flutter#142500) 2024-02-23 [email protected] Remove deprecated `KeepAliveHandle.release` (flutter/flutter#143961) 2024-02-23 [email protected] Remove deprecated AnimatedListItemBuilder, AnimatedListRemovedItemBuilder (flutter/flutter#143974) 2024-02-23 [email protected] Manual roll Flutter Engine from 06448ee8165f to 0f3ad23b84ed (16 revisions) (flutter/flutter#143989) 2024-02-23 [email protected] Remove deprecated TimelineSummary.writeSummaryToFile (flutter/flutter#143983) 2024-02-23 [email protected] Remove unused drone_dimension field (flutter/flutter#143984) 2024-02-23 [email protected] Remove deprecated MediaQuery.boldTextOverride (flutter/flutter#143960) 2024-02-22 [email protected] Revert engine back to 06448ee8165f4ff6579ef9510ac69322eda896ac (flutter/flutter#143986) 2024-02-22 [email protected] Remove deprecated FlutterDriver.enableAccessibility (flutter/flutter#143979) 2024-02-22 [email protected] Roll Flutter Engine from cb6115d3a6ab to c5d2e011652b (3 revisions) (flutter/flutter#143980) 2024-02-22 [email protected] Revert "Introduce tone-based surfaces and accent color add-ons - Part 2" (flutter/flutter#143973) 2024-02-22 [email protected] Add CloseButtonTooltip to the 'X' button on a SnackBar (flutter/flutter#143934) 2024-02-22 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.24.3 to 3.24.4 (flutter/flutter#143971) 2024-02-22 [email protected] Roll Flutter Engine from 06448ee8165f to cb6115d3a6ab (10 revisions) (flutter/flutter#143972) 2024-02-22 [email protected] Use inlining annotations on important methods for all targets (flutter/flutter#143923) 2024-02-22 [email protected] Add missing `TextPainter.strutStyle` to paragraph style (flutter/flutter#143771) 2024-02-22 [email protected] Shift Moto G4 staging tests to A02s (flutter/flutter#143957) 2024-02-22 [email protected] Add CurvedAnimation disposals in some widgets (flutter/flutter#143790) 2024-02-22 [email protected] Fix documentation typo in basic.dart (flutter/flutter#143887) 2024-02-22 [email protected] Fixed Small Typo in Emulators Test Name (flutter/flutter#143578) 2024-02-22 [email protected] Support using lightweight Flutter Engines to run tests (flutter/flutter#141726) 2024-02-22 [email protected] Roll Flutter Engine from 9250bfdb38c5 to 06448ee8165f (1 revision) (flutter/flutter#143943) 2024-02-22 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland Add UI Benchmarks (#143542) (#143799)" (flutter/flutter#143946) 2024-02-22 [email protected] Roll Packages from 48048f6 to 078c2a3 (11 revisions) (flutter/flutter#143935) 2024-02-22 [email protected] Roll Flutter Engine from 7777480eb94d to 9250bfdb38c5 (8 revisions) (flutter/flutter#143933) 2024-02-22 [email protected] Reland Add UI Benchmarks (#143542) (flutter/flutter#143799) 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

…r#143790)

Add CurvedAnimation disposals in some widgets

8a038ae

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. labels Feb 20, 2024

Merge branch 'master' into fix-curved-animation-leaks-1

35dea73

Merge branch 'master' into fix-curved-animation-leaks-1

9e1cbad

polina-c added the a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker label Feb 21, 2024

polina-c reviewed Feb 21, 2024

View reviewed changes

polina-c requested changes Feb 21, 2024

View reviewed changes

ksokolovskyi added 2 commits February 21, 2024 18:07

Merge branch 'master' into fix-curved-animation-leaks-1

288a0c7

Removed not needed removeListener call

5994778

ksokolovskyi requested a review from polina-c February 21, 2024 17:13

polina-c approved these changes Feb 21, 2024

View reviewed changes

polina-c requested a review from Piinks February 21, 2024 17:59

Piinks reviewed Feb 21, 2024

View reviewed changes

Replaced Animation<double> by CurvedAnimation in ToggleableStateMixin

834b8dc

ksokolovskyi requested a review from Piinks February 21, 2024 21:31

Piinks approved these changes Feb 22, 2024

View reviewed changes

Merge branch 'master' into fix-curved-animation-leaks-1

0a85b61

polina-c added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 22, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 22, 2024

Merge branch 'master' into fix-curved-animation-leaks-1

0453afd

polina-c added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 22, 2024

auto-submit bot merged commit f020535 into flutter:master Feb 22, 2024

engine-flutter-autoroll mentioned this pull request Feb 23, 2024

Roll Flutter from 41581c9a1b90 to fcd154bd247f (26 revisions) flutter/packages#6188

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2024

f020535 Add CurvedAnimation disposals in some widgets (flutter/flutte…

8c06844

…r#143790)

engine-flutter-autoroll mentioned this pull request Feb 23, 2024

Roll Flutter from 41581c9a1b90 to 00229827ab7c (27 revisions) flutter/packages#6189

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2024

f020535 Add CurvedAnimation disposals in some widgets (flutter/flutte…

76c3042

…r#143790)

engine-flutter-autoroll mentioned this pull request Feb 23, 2024

Roll Flutter from 41581c9a1b90 to bb856079c401 (28 revisions) flutter/packages#6190

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2024

f020535 Add CurvedAnimation disposals in some widgets (flutter/flutte…

eb5fc8a

…r#143790)

engine-flutter-autoroll mentioned this pull request Feb 23, 2024

Roll Flutter from 41581c9a1b90 to bab5f85df560 (29 revisions) flutter/packages#6193

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2024

f020535 Add CurvedAnimation disposals in some widgets (flutter/flutte…

707c439

…r#143790)

engine-flutter-autoroll mentioned this pull request Feb 23, 2024

Roll Flutter from 41581c9a1b90 to 39585e66c11f (31 revisions) flutter/packages#6194

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2024

f020535 Add CurvedAnimation disposals in some widgets (flutter/flutte…

08d7254

…r#143790)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024

f020535 Add CurvedAnimation disposals in some widgets (flutter/flutte…

6556747

…r#143790)

polina-c changed the title ~~Add CurvedAnimation disposals in some widgets~~ Add CurvedAnimation disposals in some widgets [prod-leak-fix] Aug 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CurvedAnimation disposals in some widgets [prod-leak-fix]#143790

Add CurvedAnimation disposals in some widgets [prod-leak-fix]#143790
auto-submit[bot] merged 8 commits intoflutter:masterfrom
ksokolovskyi:fix-curved-animation-leaks-1

ksokolovskyi commented Feb 20, 2024 •

edited

Loading

Uh oh!

ksokolovskyi commented Feb 20, 2024

Uh oh!

polina-c Feb 21, 2024

Uh oh!

ksokolovskyi Feb 21, 2024

Uh oh!

polina-c left a comment

Uh oh!

Piinks Feb 21, 2024

Uh oh!

ksokolovskyi Feb 21, 2024

Uh oh!

Piinks Feb 21, 2024

Uh oh!

Piinks Feb 21, 2024

Uh oh!

Piinks left a comment

Uh oh!

auto-submit bot commented Feb 22, 2024

Uh oh!

ksokolovskyi commented Feb 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ksokolovskyi commented Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pre-launch Checklist

Uh oh!

ksokolovskyi commented Feb 20, 2024

Uh oh!

polina-c Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

ksokolovskyi Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

polina-c left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

ksokolovskyi Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

Piinks Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

Piinks Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Feb 22, 2024

Uh oh!

ksokolovskyi commented Feb 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ksokolovskyi commented Feb 20, 2024 •

edited

Loading