Address post-submit comments for sliders (flutter#15836)

* Address post-submit comments * Added dispose protection for the animation controllers.
mikolaj-staworzynski-red · Mar 25, 2018 · 2171fcb · 2171fcb
1 parent a6d8ca2
commit 2171fcb
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 12 deletions.
diff --git a/packages/flutter/lib/src/material/slider.dart b/packages/flutter/lib/src/material/slider.dart
@@ -248,21 +248,17 @@ class _SliderState extends State<Slider> with TickerProviderStateMixin {
       duration: Duration.zero,
       vsync: this,
     );
-    // Create timer in a cancelled state, so that we don't have to
-    // check for null below.
-    interactionTimer = new Timer(Duration.zero, () {});
-    interactionTimer.cancel();
     enableController.value = widget.onChanged != null ? 1.0 : 0.0;
     positionController.value = _unlerp(widget.value);
   }
 
   @override
   void dispose() {
+    interactionTimer?.cancel();
     overlayController.dispose();
     valueIndicatorController.dispose();
     enableController.dispose();
     positionController.dispose();
-    interactionTimer?.cancel();
     super.dispose();
   }
 
@@ -643,25 +639,24 @@ class _RenderSlider extends RenderBox {
       _state.overlayController.forward();
       if (showValueIndicator) {
         _state.valueIndicatorController.forward();
-        if (_state.interactionTimer.isActive) {
-          _state.interactionTimer.cancel();
-        }
+        _state.interactionTimer?.cancel();
         _state.interactionTimer = new Timer(_minimumInteractionTime * timeDilation, () {
-          if (!_active && _state.valueIndicatorController.status == AnimationStatus.completed) {
+          _state.interactionTimer = null;
+          if (!_active &&
+              _state.valueIndicatorController.status == AnimationStatus.completed) {
             _state.valueIndicatorController.reverse();
           }
-          _state.interactionTimer.cancel();
         });
       }
     }
   }
 
   void _endInteraction() {
-    if (_active) {
+    if (_active && _state.mounted) {
       _active = false;
       _currentDragValue = 0.0;
       _state.overlayController.reverse();
-      if (showValueIndicator && !_state.interactionTimer.isActive) {
+      if (showValueIndicator && _state.interactionTimer == null) {
         _state.valueIndicatorController.reverse();
       }
     }

diff --git a/packages/flutter/test/material/slider_test.dart b/packages/flutter/test/material/slider_test.dart
@@ -1208,4 +1208,45 @@ void main() {
     await expectValueIndicator(isVisible: false, theme: theme, enabled: true);
     await expectValueIndicator(isVisible: false, theme: theme, enabled: false);
   });
+
+  testWidgets("Slider doesn't start any animations after dispose", (WidgetTester tester) async {
+    final Key sliderKey = new UniqueKey();
+    double value = 0.0;
+    await tester.pumpWidget(
+      new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new StatefulBuilder(
+          builder: (BuildContext context, StateSetter setState) {
+            return new MediaQuery(
+              data: new MediaQueryData.fromWindow(window),
+              child: new Material(
+                child: new Center(
+                  child: new Slider(
+                    key: sliderKey,
+                    value: value,
+                    divisions: 4,
+                    onChanged: (double newValue) {
+                      setState(() {
+                        value = newValue;
+                      });
+                    },
+                  ),
+                ),
+              ),
+            );
+          },
+        ),
+      ),
+    );
+
+    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byKey(sliderKey)));
+    await tester.pumpAndSettle(const Duration(milliseconds: 100));
+    expect(value, equals(0.5));
+    await gesture.moveBy(const Offset(-500.0, 0.0));
+    await tester.pumpAndSettle(const Duration(milliseconds: 100));
+    // Change the tree to dispose the original widget.
+    await tester.pumpWidget(new Container());
+    expect(await tester.pumpAndSettle(const Duration(milliseconds: 100)), equals(1));
+    await gesture.up();
+  });
 }