Skip to content
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

Fix leak of CurvedAnimations in long-lived ImplicitlyAnimatedWidgets. #84785

Merged
merged 18 commits into from Jun 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a5e767f
Fix leak of CurvedAnimations in long-lived ImplicitlyAnimatedWidgets.
apwilson Jun 17, 2021
02c9f3b
Fix leak of CurvedAnimations in long-lived ImplicitlyAnimatedWidgets.
apwilson Jun 17, 2021
ef6376c
Merge branch 'master' of github.com:flutter/flutter into leak
apwilson Jun 17, 2021
852c52c
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 17, 2021
728aa2b
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 17, 2021
38cded3
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 17, 2021
96ee7a8
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 17, 2021
9375d72
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
bdf2fd7
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
3077b19
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
a843ef4
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
ff71303
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
c3f3fe0
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
35a0997
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
341f24c
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
eeb232e
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
b6c3451
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
f5715da
Merge branch 'leak' of https://github.com/apwilson/flutter into leak
apwilson Jun 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/flutter/lib/src/animation/animations.dart
Expand Up @@ -408,6 +408,9 @@ class CurvedAnimation extends Animation<double> with AnimationWithParentMixin<do
/// animation is used to animate.
AnimationStatus? _curveDirection;

/// True if this CurvedAnimation has been disposed.
bool isDisposed = false;

void _updateCurveDirection(AnimationStatus status) {
switch (status) {
case AnimationStatus.dismissed:
Expand All @@ -427,6 +430,12 @@ class CurvedAnimation extends Animation<double> with AnimationWithParentMixin<do
return reverseCurve == null || (_curveDirection ?? parent.status) != AnimationStatus.reverse;
}

/// Cleans up any listeners added by this CurvedAnimation.
void dispose() {
isDisposed = true;
parent.removeStatusListener(_updateCurveDirection);
}

@override
double get value {
final Curve? activeCurve = _useForwardCurve ? curve : reverseCurve;
Expand Down
5 changes: 4 additions & 1 deletion packages/flutter/lib/src/widgets/implicit_animations.dart
Expand Up @@ -380,8 +380,10 @@ abstract class ImplicitlyAnimatedWidgetState<T extends ImplicitlyAnimatedWidget>
@override
void didUpdateWidget(T oldWidget) {
super.didUpdateWidget(oldWidget);
if (widget.curve != oldWidget.curve)
if (widget.curve != oldWidget.curve) {
(_animation as CurvedAnimation).dispose();
apwilson marked this conversation as resolved.
Show resolved Hide resolved
_animation = _createCurve();
}
_controller.duration = widget.duration;
if (_constructTweens()) {
forEachTween((Tween<dynamic>? tween, dynamic targetValue, TweenConstructor<dynamic> constructor) {
Expand All @@ -401,6 +403,7 @@ abstract class ImplicitlyAnimatedWidgetState<T extends ImplicitlyAnimatedWidget>

@override
void dispose() {
(_animation as CurvedAnimation).dispose();
_controller.dispose();
super.dispose();
}
Expand Down
38 changes: 38 additions & 0 deletions packages/flutter/test/animation/animations_test.dart
Expand Up @@ -302,6 +302,44 @@ FlutterError
expect(curved.value, moreOrLessEquals(0.0));
});

test('CurvedAnimation stops listening to parent when disposed.', () async {
const Interval forwardCurve = Interval(0.0, 0.5);
const Interval reverseCurve = Interval(0.5, 1.0);

final AnimationController controller = AnimationController(
duration: const Duration(milliseconds: 100),
reverseDuration: const Duration(milliseconds: 100),
vsync: const TestVSync(),
);
final CurvedAnimation curved = CurvedAnimation(
parent: controller, curve: forwardCurve, reverseCurve: reverseCurve);

expect(forwardCurve.transform(0.5), 1.0);
expect(reverseCurve.transform(0.5), 0.0);

controller.forward(from: 0.5);
expect(controller.status, equals(AnimationStatus.forward));
expect(curved.value, equals(1.0));

controller.value = 1.0;
expect(controller.status, equals(AnimationStatus.completed));

controller.reverse(from: 0.5);
expect(controller.status, equals(AnimationStatus.reverse));
expect(curved.value, equals(0.0));

expect(curved.isDisposed, isFalse);
curved.dispose();
expect(curved.isDisposed, isTrue);

controller.value = 0.0;
expect(controller.status, equals(AnimationStatus.dismissed));

controller.forward(from: 0.5);
expect(controller.status, equals(AnimationStatus.forward));
expect(curved.value, equals(0.0));
});

test('ReverseAnimation running with different forward and reverse durations.', () {
final AnimationController controller = AnimationController(
duration: const Duration(milliseconds: 100),
Expand Down
53 changes: 53 additions & 0 deletions packages/flutter/test/widgets/implicit_animations_test.dart
Expand Up @@ -350,6 +350,59 @@ void main() {
await tester.pump(additionalDelay);
expect(mockOnEndFunction.called, 1);
});

testWidgets('Ensure CurvedAnimations are disposed on widget change',
(WidgetTester tester) async {
final GlobalKey<ImplicitlyAnimatedWidgetState<AnimatedOpacity>> key =
GlobalKey<ImplicitlyAnimatedWidgetState<AnimatedOpacity>>();
final ValueNotifier<Curve> curve = ValueNotifier<Curve>(const Interval(0.0, 0.5));
await tester.pumpWidget(wrap(
child: ValueListenableBuilder<Curve>(
valueListenable: curve,
builder: (_, Curve c, __) => AnimatedOpacity(
key: key,
opacity: 1.0,
duration: const Duration(seconds: 1),
curve: c,
child: Container(color: Colors.green)),
),
));

final ImplicitlyAnimatedWidgetState<AnimatedOpacity>? firstState = key.currentState;
final Animation<double>? firstAnimation = firstState?.animation;
if (firstAnimation == null)
fail('animation was null!');

final CurvedAnimation firstCurvedAnimation =
firstAnimation as CurvedAnimation;

expect(firstCurvedAnimation.isDisposed, isFalse);

curve.value = const Interval(0.0, 0.6);
await tester.pumpAndSettle();

final ImplicitlyAnimatedWidgetState<AnimatedOpacity>? secondState = key.currentState;
final Animation<double>? secondAnimation = secondState?.animation;
if (secondAnimation == null)
fail('animation was null!');

final CurvedAnimation secondCurvedAnimation = secondAnimation as CurvedAnimation;

expect(firstState, equals(secondState));
expect(firstAnimation, isNot(equals(secondAnimation)));

expect(firstCurvedAnimation.isDisposed, isTrue);
expect(secondCurvedAnimation.isDisposed, isFalse);

await tester.pumpWidget(
wrap(
child: const Offstage(),
),
);
await tester.pumpAndSettle();

expect(secondCurvedAnimation.isDisposed, isTrue);
});
}

Widget wrap({required Widget child}) {
Expand Down