Commit aa82b741 authored by Adam Barth's avatar Adam Barth Committed by GitHub

Revert "Strengthen animation listener iteration patterns" (#7552)

parent 4e8628c1
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:collection';
import 'dart:ui' show VoidCallback; import 'dart:ui' show VoidCallback;
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
...@@ -35,11 +34,9 @@ abstract class AnimationLazyListenerMixin implements _ListenerMixin { ...@@ -35,11 +34,9 @@ abstract class AnimationLazyListenerMixin implements _ListenerMixin {
} }
/// Called when the number of listeners changes from zero to one. /// Called when the number of listeners changes from zero to one.
@protected
void didStartListening(); void didStartListening();
/// Called when the number of listeners changes from one to zero. /// Called when the number of listeners changes from one to zero.
@protected
void didStopListening(); void didStopListening();
/// Whether there are any listeners. /// Whether there are any listeners.
...@@ -64,7 +61,7 @@ abstract class AnimationEagerListenerMixin implements _ListenerMixin { ...@@ -64,7 +61,7 @@ abstract class AnimationEagerListenerMixin implements _ListenerMixin {
/// A mixin that implements the addListener/removeListener protocol and notifies /// A mixin that implements the addListener/removeListener protocol and notifies
/// all the registered listeners when notifyListeners is called. /// all the registered listeners when notifyListeners is called.
abstract class AnimationLocalListenersMixin extends _ListenerMixin { abstract class AnimationLocalListenersMixin extends _ListenerMixin {
final Set<VoidCallback> _listeners = new LinkedHashSet<VoidCallback>(); final List<VoidCallback> _listeners = <VoidCallback>[];
/// Calls the listener every time the value of the animation changes. /// Calls the listener every time the value of the animation changes.
/// ///
...@@ -87,24 +84,9 @@ abstract class AnimationLocalListenersMixin extends _ListenerMixin { ...@@ -87,24 +84,9 @@ abstract class AnimationLocalListenersMixin extends _ListenerMixin {
/// If listeners are added or removed during this function, the modifications /// If listeners are added or removed during this function, the modifications
/// will not change which listeners are called during this iteration. /// will not change which listeners are called during this iteration.
void notifyListeners() { void notifyListeners() {
final List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners); List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners);
for (VoidCallback listener in localListeners) { for (VoidCallback listener in localListeners)
try { listener();
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');
}
));
}
}
} }
} }
...@@ -112,7 +94,7 @@ abstract class AnimationLocalListenersMixin extends _ListenerMixin { ...@@ -112,7 +94,7 @@ abstract class AnimationLocalListenersMixin extends _ListenerMixin {
/// and notifies all the registered listeners when notifyStatusListeners is /// and notifies all the registered listeners when notifyStatusListeners is
/// called. /// called.
abstract class AnimationLocalStatusListenersMixin extends _ListenerMixin { abstract class AnimationLocalStatusListenersMixin extends _ListenerMixin {
final Set<AnimationStatusListener> _statusListeners = new LinkedHashSet<AnimationStatusListener>(); final List<AnimationStatusListener> _statusListeners = <AnimationStatusListener>[];
/// Calls listener every time the status of the animation changes. /// Calls listener every time the status of the animation changes.
/// ///
...@@ -135,23 +117,8 @@ abstract class AnimationLocalStatusListenersMixin extends _ListenerMixin { ...@@ -135,23 +117,8 @@ abstract class AnimationLocalStatusListenersMixin extends _ListenerMixin {
/// If listeners are added or removed during this function, the modifications /// If listeners are added or removed during this function, the modifications
/// will not change which listeners are called during this iteration. /// will not change which listeners are called during this iteration.
void notifyStatusListeners(AnimationStatus status) { void notifyStatusListeners(AnimationStatus status) {
final List<AnimationStatusListener> localListeners = new List<AnimationStatusListener>.from(_statusListeners); List<AnimationStatusListener> localListeners = new List<AnimationStatusListener>.from(_statusListeners);
for (AnimationStatusListener listener in localListeners) { for (AnimationStatusListener listener in localListeners)
try { listener(status);
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');
}
));
}
}
} }
} }
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:collection';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'assertions.dart'; import 'assertions.dart';
...@@ -39,11 +37,11 @@ abstract class Listenable { ...@@ -39,11 +37,11 @@ abstract class Listenable {
/// It is O(N) for adding and removing listeners and O(N²) for dispatching /// It is O(N) for adding and removing listeners and O(N²) for dispatching
/// notifications (where N is the number of listeners). /// notifications (where N is the number of listeners).
class ChangeNotifier extends Listenable { class ChangeNotifier extends Listenable {
Set<VoidCallback> _listeners = new LinkedHashSet<VoidCallback>(); List<VoidCallback> _listeners;
bool _debugAssertNotDisposed() { bool _debugAssertNotDisposed() {
assert(() { assert(() {
if (_listeners == null) { if (_listeners == const <VoidCallback>[]) {
throw new FlutterError( throw new FlutterError(
'A $runtimeType was used after being disposed.\n' 'A $runtimeType was used after being disposed.\n'
'Once you have called dispose() on a $runtimeType, it can no longer be used.' 'Once you have called dispose() on a $runtimeType, it can no longer be used.'
...@@ -60,6 +58,7 @@ class ChangeNotifier extends Listenable { ...@@ -60,6 +58,7 @@ class ChangeNotifier extends Listenable {
@override @override
void addListener(VoidCallback listener) { void addListener(VoidCallback listener) {
assert(_debugAssertNotDisposed); assert(_debugAssertNotDisposed);
_listeners ??= <VoidCallback>[];
_listeners.add(listener); _listeners.add(listener);
} }
...@@ -72,7 +71,7 @@ class ChangeNotifier extends Listenable { ...@@ -72,7 +71,7 @@ class ChangeNotifier extends Listenable {
@override @override
void removeListener(VoidCallback listener) { void removeListener(VoidCallback listener) {
assert(_debugAssertNotDisposed); assert(_debugAssertNotDisposed);
_listeners.remove(listener); _listeners?.remove(listener);
} }
/// Discards any resources used by the object. After this is called, the /// Discards any resources used by the object. After this is called, the
...@@ -84,7 +83,7 @@ class ChangeNotifier extends Listenable { ...@@ -84,7 +83,7 @@ class ChangeNotifier extends Listenable {
@mustCallSuper @mustCallSuper
void dispose() { void dispose() {
assert(_debugAssertNotDisposed); assert(_debugAssertNotDisposed);
_listeners = null; _listeners = const <VoidCallback>[];
} }
/// Call all the registered listeners. /// Call all the registered listeners.
...@@ -102,7 +101,7 @@ class ChangeNotifier extends Listenable { ...@@ -102,7 +101,7 @@ class ChangeNotifier extends Listenable {
void notifyListeners() { void notifyListeners() {
assert(_debugAssertNotDisposed); assert(_debugAssertNotDisposed);
if (_listeners != null) { if (_listeners != null) {
final List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners); List<VoidCallback> localListeners = new List<VoidCallback>.from(_listeners);
for (VoidCallback listener in localListeners) { for (VoidCallback listener in localListeners) {
try { try {
if (_listeners.contains(listener)) if (_listeners.contains(listener))
......
...@@ -167,6 +167,14 @@ class _CupertinoBackGestureController extends NavigationGestureController { ...@@ -167,6 +167,14 @@ class _CupertinoBackGestureController extends NavigationGestureController {
} }
void handleStatusChanged(AnimationStatus status) { 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 <https://github.com/flutter/flutter/issues/7533>.
if (controller == null)
return;
if (status == AnimationStatus.dismissed) { if (status == AnimationStatus.dismissed) {
navigator.pop(); navigator.pop();
assert(controller == null); assert(controller == null);
......
// 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<String> log = <String>[];
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, <String>['listener1', 'listener2']);
log.clear();
controller.value = 0.3;
expect(log, <String>['listener2', 'listener4']);
log.clear();
controller.value = 0.4;
expect(log, <String>['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<String> log = <String>[];
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, <String>['listener1', 'listener2']);
log.clear();
controller.reverse();
expect(log, <String>['listener2', 'listener4']);
log.clear();
controller.forward();
expect(log, <String>['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<String> log = <String>[];
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, <String>['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<String> log = <String>[];
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, <String>['listener1', 'badListener', 'listener2']);
expect(tester.takeException(), isNullThrownError);
log.clear();
controller.dispose();
});
}
...@@ -24,12 +24,12 @@ void main() { ...@@ -24,12 +24,12 @@ void main() {
test.addListener(listener); test.addListener(listener);
test.addListener(listener); test.addListener(listener);
test.notify(); test.notify();
expect(log, <String>['listener']); expect(log, <String>['listener', 'listener']);
log.clear(); log.clear();
test.removeListener(listener); test.removeListener(listener);
test.notify(); test.notify();
expect(log, <String>[]); expect(log, <String>['listener']);
log.clear(); log.clear();
test.removeListener(listener); test.removeListener(listener);
...@@ -79,7 +79,7 @@ void main() { ...@@ -79,7 +79,7 @@ void main() {
test.removeListener(listener2); test.removeListener(listener2);
test.addListener(listener2); test.addListener(listener2);
test.notify(); test.notify();
expect(log, <String>['badListener', 'listener2']); expect(log, <String>['badListener', 'listener1', 'listener2']);
expect(tester.takeException(), isNullThrownError); expect(tester.takeException(), isNullThrownError);
log.clear(); log.clear();
}); });
...@@ -110,7 +110,7 @@ void main() { ...@@ -110,7 +110,7 @@ void main() {
log.clear(); log.clear();
test.notify(); test.notify();
expect(log, <String>['listener2', 'listener4']); expect(log, <String>['listener2', 'listener4', 'listener4']);
log.clear(); log.clear();
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment