Unverified Commit 5f3bee55 authored by chunhtai's avatar chunhtai Committed by GitHub

Allow remove listener on disposed change notifier (#97988)

parent 74c4e635
...@@ -103,7 +103,12 @@ abstract class ValueListenable<T> extends Listenable { ...@@ -103,7 +103,12 @@ abstract class ValueListenable<T> extends Listenable {
/// * [ValueNotifier], which is a [ChangeNotifier] that wraps a single value. /// * [ValueNotifier], which is a [ChangeNotifier] that wraps a single value.
class ChangeNotifier implements Listenable { class ChangeNotifier implements Listenable {
int _count = 0; int _count = 0;
List<VoidCallback?> _listeners = List<VoidCallback?>.filled(0, null); // The _listeners is intentionally set to a fixed-length _GrowableList instead
// of const [] for performance reasons.
// See https://github.com/flutter/flutter/pull/71947/files#r545722476 for
// more details.
static final List<VoidCallback?> _emptyListeners = List<VoidCallback?>.filled(0, null);
List<VoidCallback?> _listeners = _emptyListeners;
int _notificationCallStackDepth = 0; int _notificationCallStackDepth = 0;
int _reentrantlyRemovedListeners = 0; int _reentrantlyRemovedListeners = 0;
bool _debugDisposed = false; bool _debugDisposed = false;
...@@ -220,7 +225,7 @@ class ChangeNotifier implements Listenable { ...@@ -220,7 +225,7 @@ class ChangeNotifier implements Listenable {
/// ///
/// If the given listener is not registered, the call is ignored. /// If the given listener is not registered, the call is ignored.
/// ///
/// This method must not be called after [dispose] has been called. /// This method returns immediately if [dispose] has been called.
/// ///
/// {@macro flutter.foundation.ChangeNotifier.addListener} /// {@macro flutter.foundation.ChangeNotifier.addListener}
/// ///
...@@ -230,7 +235,11 @@ class ChangeNotifier implements Listenable { ...@@ -230,7 +235,11 @@ class ChangeNotifier implements Listenable {
/// changes. /// changes.
@override @override
void removeListener(VoidCallback listener) { void removeListener(VoidCallback listener) {
assert(_debugAssertNotDisposed()); // This method is allowed to be called on disposed instances for usability
// reasons. Due to how our frame scheduling logic between render objects and
// overlays, it is common that the owner of this instance would be disposed a
// frame earlier than the listeners. Allowing calls to this method after it
// is disposed makes it easier for listeners to properly clean up.
for (int i = 0; i < _count; i++) { for (int i = 0; i < _count; i++) {
final VoidCallback? listenerAtIndex = _listeners[i]; final VoidCallback? listenerAtIndex = _listeners[i];
if (listenerAtIndex == listener) { if (listenerAtIndex == listener) {
...@@ -253,8 +262,7 @@ class ChangeNotifier implements Listenable { ...@@ -253,8 +262,7 @@ class ChangeNotifier implements Listenable {
/// Discards any resources used by the object. After this is called, the /// Discards any resources used by the object. After this is called, the
/// object is not in a usable state and should be discarded (calls to /// object is not in a usable state and should be discarded (calls to
/// [addListener] and [removeListener] will throw after the object is /// [addListener] will throw after the object is disposed).
/// disposed).
/// ///
/// This method should only be called by the object's owner. /// This method should only be called by the object's owner.
@mustCallSuper @mustCallSuper
...@@ -264,6 +272,8 @@ class ChangeNotifier implements Listenable { ...@@ -264,6 +272,8 @@ class ChangeNotifier implements Listenable {
_debugDisposed = true; _debugDisposed = true;
return true; return true;
}()); }());
_listeners = _emptyListeners;
_count = 0;
} }
/// Call all the registered listeners. /// Call all the registered listeners.
......
...@@ -323,15 +323,12 @@ void main() { ...@@ -323,15 +323,12 @@ void main() {
expect(log, isEmpty); expect(log, isEmpty);
}); });
test('Cannot use a disposed ChangeNotifier', () { test('Cannot use a disposed ChangeNotifier except for remove listener', () {
final TestNotifier source = TestNotifier(); final TestNotifier source = TestNotifier();
source.dispose(); source.dispose();
expect(() { expect(() {
source.addListener(() {}); source.addListener(() {});
}, throwsFlutterError); }, throwsFlutterError);
expect(() {
source.removeListener(() {});
}, throwsFlutterError);
expect(() { expect(() {
source.dispose(); source.dispose();
}, throwsFlutterError); }, throwsFlutterError);
...@@ -340,6 +337,18 @@ void main() { ...@@ -340,6 +337,18 @@ void main() {
}, throwsFlutterError); }, throwsFlutterError);
}); });
test('Can remove listener on a disposed ChangeNotifier', () {
final TestNotifier source = TestNotifier();
FlutterError? error;
try {
source.dispose();
source.removeListener(() {});
} on FlutterError catch (e) {
error = e;
}
expect(error, isNull);
});
test('Value notifier', () { test('Value notifier', () {
final ValueNotifier<double> notifier = ValueNotifier<double>(2.0); final ValueNotifier<double> notifier = ValueNotifier<double>(2.0);
......
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