diff --git a/packages/flutter/lib/src/animation/listener_helpers.dart b/packages/flutter/lib/src/animation/listener_helpers.dart index 1f03100efb..ad72ef7667 100644 --- a/packages/flutter/lib/src/animation/listener_helpers.dart +++ b/packages/flutter/lib/src/animation/listener_helpers.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:collection'; import 'dart:ui' show VoidCallback; import 'package:flutter/foundation.dart'; @@ -35,11 +34,9 @@ abstract class AnimationLazyListenerMixin implements _ListenerMixin { } /// Called when the number of listeners changes from zero to one. - @protected void didStartListening(); /// Called when the number of listeners changes from one to zero. - @protected void didStopListening(); /// Whether there are any listeners. @@ -64,7 +61,7 @@ abstract class AnimationEagerListenerMixin implements _ListenerMixin { /// A mixin that implements the addListener/removeListener protocol and notifies /// all the registered listeners when notifyListeners is called. abstract class AnimationLocalListenersMixin extends _ListenerMixin { - final Set _listeners = new LinkedHashSet(); + final List _listeners = []; /// Calls the listener every time the value of the animation changes. /// @@ -87,24 +84,9 @@ abstract class AnimationLocalListenersMixin extends _ListenerMixin { /// If listeners are added or removed during this function, the modifications /// will not change which listeners are called during this iteration. void notifyListeners() { - final List localListeners = new List.from(_listeners); - for (VoidCallback listener in localListeners) { - try { - if (_listeners.contains(listener)) - listener(); - } catch (exception, stack) { - FlutterError.reportError(new FlutterErrorDetails( - exception: exception, - stack: stack, - library: 'animation library', - context: 'while notifying listeners for $runtimeType', - informationCollector: (StringBuffer information) { - information.writeln('The $runtimeType notifying listeners was:'); - information.write(' $this'); - } - )); - } - } + List localListeners = new List.from(_listeners); + for (VoidCallback listener in localListeners) + listener(); } } @@ -112,7 +94,7 @@ abstract class AnimationLocalListenersMixin extends _ListenerMixin { /// and notifies all the registered listeners when notifyStatusListeners is /// called. abstract class AnimationLocalStatusListenersMixin extends _ListenerMixin { - final Set _statusListeners = new LinkedHashSet(); + final List _statusListeners = []; /// Calls listener every time the status of the animation changes. /// @@ -135,23 +117,8 @@ abstract class AnimationLocalStatusListenersMixin extends _ListenerMixin { /// If listeners are added or removed during this function, the modifications /// will not change which listeners are called during this iteration. void notifyStatusListeners(AnimationStatus status) { - final List localListeners = new List.from(_statusListeners); - for (AnimationStatusListener listener in localListeners) { - try { - if (_statusListeners.contains(listener)) - listener(status); - } catch (exception, stack) { - FlutterError.reportError(new FlutterErrorDetails( - exception: exception, - stack: stack, - library: 'animation library', - context: 'while notifying status listeners for $runtimeType', - informationCollector: (StringBuffer information) { - information.writeln('The $runtimeType notifying status listeners was:'); - information.write(' $this'); - } - )); - } - } + List localListeners = new List.from(_statusListeners); + for (AnimationStatusListener listener in localListeners) + listener(status); } } diff --git a/packages/flutter/lib/src/foundation/change_notifier.dart b/packages/flutter/lib/src/foundation/change_notifier.dart index 08761f3c17..f09dd2d058 100644 --- a/packages/flutter/lib/src/foundation/change_notifier.dart +++ b/packages/flutter/lib/src/foundation/change_notifier.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:collection'; - import 'package:meta/meta.dart'; import 'assertions.dart'; @@ -39,11 +37,11 @@ abstract class Listenable { /// It is O(N) for adding and removing listeners and O(N²) for dispatching /// notifications (where N is the number of listeners). class ChangeNotifier extends Listenable { - Set _listeners = new LinkedHashSet(); + List _listeners; bool _debugAssertNotDisposed() { assert(() { - if (_listeners == null) { + if (_listeners == const []) { throw new FlutterError( 'A $runtimeType was used after being disposed.\n' 'Once you have called dispose() on a $runtimeType, it can no longer be used.' @@ -60,6 +58,7 @@ class ChangeNotifier extends Listenable { @override void addListener(VoidCallback listener) { assert(_debugAssertNotDisposed); + _listeners ??= []; _listeners.add(listener); } @@ -72,7 +71,7 @@ class ChangeNotifier extends Listenable { @override void removeListener(VoidCallback listener) { assert(_debugAssertNotDisposed); - _listeners.remove(listener); + _listeners?.remove(listener); } /// Discards any resources used by the object. After this is called, the @@ -84,7 +83,7 @@ class ChangeNotifier extends Listenable { @mustCallSuper void dispose() { assert(_debugAssertNotDisposed); - _listeners = null; + _listeners = const []; } /// Call all the registered listeners. @@ -102,7 +101,7 @@ class ChangeNotifier extends Listenable { void notifyListeners() { assert(_debugAssertNotDisposed); if (_listeners != null) { - final List localListeners = new List.from(_listeners); + List localListeners = new List.from(_listeners); for (VoidCallback listener in localListeners) { try { if (_listeners.contains(listener)) diff --git a/packages/flutter/lib/src/material/page.dart b/packages/flutter/lib/src/material/page.dart index 70f789430f..55c6506790 100644 --- a/packages/flutter/lib/src/material/page.dart +++ b/packages/flutter/lib/src/material/page.dart @@ -167,6 +167,14 @@ class _CupertinoBackGestureController extends NavigationGestureController { } void handleStatusChanged(AnimationStatus status) { + // This can happen if an earlier status listener ends up calling dispose() + // on this object. + // TODO(abarth): Consider changing AnimationController not to call listeners + // that were removed while calling other listeners. + // See . + if (controller == null) + return; + if (status == AnimationStatus.dismissed) { navigator.pop(); assert(controller == null); diff --git a/packages/flutter/test/animation/iteration_patterns_test.dart b/packages/flutter/test/animation/iteration_patterns_test.dart deleted file mode 100644 index 50001994cb..0000000000 --- a/packages/flutter/test/animation/iteration_patterns_test.dart +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:flutter_test/flutter_test.dart'; -import 'package:flutter/animation.dart'; -import 'package:flutter/widgets.dart'; - -void main() { - setUp(() { - WidgetsFlutterBinding.ensureInitialized(); - WidgetsBinding.instance.resetEpoch(); - }); - - test('AnimationController with mutating listener', () { - final AnimationController controller = new AnimationController( - duration: const Duration(milliseconds: 100), - vsync: const TestVSync(), - ); - final List log = []; - - final VoidCallback listener1 = () { log.add('listener1'); }; - final VoidCallback listener3 = () { log.add('listener3'); }; - final VoidCallback listener4 = () { log.add('listener4'); }; - final VoidCallback listener2 = () { - log.add('listener2'); - controller.removeListener(listener1); - controller.removeListener(listener3); - controller.addListener(listener4); - }; - - controller.addListener(listener1); - controller.addListener(listener2); - controller.addListener(listener3); - controller.value = 0.2; - expect(log, ['listener1', 'listener2']); - log.clear(); - - controller.value = 0.3; - expect(log, ['listener2', 'listener4']); - log.clear(); - - controller.value = 0.4; - expect(log, ['listener2', 'listener4']); - log.clear(); - }); - - test('AnimationController with mutating status listener', () { - final AnimationController controller = new AnimationController( - duration: const Duration(milliseconds: 100), - vsync: const TestVSync(), - ); - final List log = []; - - final AnimationStatusListener listener1 = (AnimationStatus status) { log.add('listener1'); }; - final AnimationStatusListener listener3 = (AnimationStatus status) { log.add('listener3'); }; - final AnimationStatusListener listener4 = (AnimationStatus status) { log.add('listener4'); }; - final AnimationStatusListener listener2 = (AnimationStatus status) { - log.add('listener2'); - controller.removeStatusListener(listener1); - controller.removeStatusListener(listener3); - controller.addStatusListener(listener4); - }; - - controller.addStatusListener(listener1); - controller.addStatusListener(listener2); - controller.addStatusListener(listener3); - controller.forward(); - expect(log, ['listener1', 'listener2']); - log.clear(); - - controller.reverse(); - expect(log, ['listener2', 'listener4']); - log.clear(); - - controller.forward(); - expect(log, ['listener2', 'listener4']); - log.clear(); - - controller.dispose(); - }); - - testWidgets('AnimationController with throwing listener', (WidgetTester tester) async { - final AnimationController controller = new AnimationController( - duration: const Duration(milliseconds: 100), - vsync: const TestVSync(), - ); - final List log = []; - - final VoidCallback listener1 = () { log.add('listener1'); }; - final VoidCallback badListener = () { log.add('badListener'); throw null; }; - final VoidCallback listener2 = () { log.add('listener2'); }; - - controller.addListener(listener1); - controller.addListener(badListener); - controller.addListener(listener2); - controller.value = 0.2; - expect(log, ['listener1', 'badListener', 'listener2']); - expect(tester.takeException(), isNullThrownError); - log.clear(); - }); - - testWidgets('AnimationController with throwing status listener', (WidgetTester tester) async { - final AnimationController controller = new AnimationController( - duration: const Duration(milliseconds: 100), - vsync: const TestVSync(), - ); - final List log = []; - - final AnimationStatusListener listener1 = (AnimationStatus status) { log.add('listener1'); }; - final AnimationStatusListener badListener = (AnimationStatus status) { log.add('badListener'); throw null; }; - final AnimationStatusListener listener2 = (AnimationStatus status) { log.add('listener2'); }; - - controller.addStatusListener(listener1); - controller.addStatusListener(badListener); - controller.addStatusListener(listener2); - controller.forward(); - expect(log, ['listener1', 'badListener', 'listener2']); - expect(tester.takeException(), isNullThrownError); - log.clear(); - controller.dispose(); - }); -} diff --git a/packages/flutter/test/foundation/change_notifier_test.dart b/packages/flutter/test/foundation/change_notifier_test.dart index d5ec519624..24cb71bf47 100644 --- a/packages/flutter/test/foundation/change_notifier_test.dart +++ b/packages/flutter/test/foundation/change_notifier_test.dart @@ -24,12 +24,12 @@ void main() { test.addListener(listener); test.addListener(listener); test.notify(); - expect(log, ['listener']); + expect(log, ['listener', 'listener']); log.clear(); test.removeListener(listener); test.notify(); - expect(log, []); + expect(log, ['listener']); log.clear(); test.removeListener(listener); @@ -79,7 +79,7 @@ void main() { test.removeListener(listener2); test.addListener(listener2); test.notify(); - expect(log, ['badListener', 'listener2']); + expect(log, ['badListener', 'listener1', 'listener2']); expect(tester.takeException(), isNullThrownError); log.clear(); }); @@ -110,7 +110,7 @@ void main() { log.clear(); test.notify(); - expect(log, ['listener2', 'listener4']); + expect(log, ['listener2', 'listener4', 'listener4']); log.clear(); });