Unverified Commit 16b73481 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Switch debugAssertNotDisposed to be a static (#104772)

This reverts part of the change made in #103456 to expose a debug check for subclasses of ChangeNotifier to avoid code duplication. Instead of making debugAssertNotDisposed a public instance function, it is now a public static function. It makes it harder to call, slightly, but it means that everyone who implemented ChangeNotifier instead of extending it doesn't get broken.
parent b625353e
...@@ -129,19 +129,22 @@ class ChangeNotifier implements Listenable { ...@@ -129,19 +129,22 @@ class ChangeNotifier implements Listenable {
/// ```dart /// ```dart
/// class MyNotifier with ChangeNotifier { /// class MyNotifier with ChangeNotifier {
/// void doUpdate() { /// void doUpdate() {
/// assert(debugAssertNotDisposed()); /// assert(ChangeNotifier.debugAssertNotDisposed(this));
/// // ... /// // ...
/// } /// }
/// } /// }
/// ``` /// ```
/// {@end-tool} /// {@end-tool}
@protected // This is static and not an instance method because too many people try to
bool debugAssertNotDisposed() { // implement ChangeNotifier instead of extending it (and so it is too breaking
// to add a method, especially for debug).
static bool debugAssertNotDisposed(ChangeNotifier notifier) {
assert(() { assert(() {
if (_debugDisposed) { if (notifier._debugDisposed) {
throw FlutterError( throw FlutterError(
'A $runtimeType was used after being disposed.\n' 'A ${notifier.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 ${notifier.runtimeType}, it '
'can no longer be used.',
); );
} }
return true; return true;
...@@ -166,7 +169,7 @@ class ChangeNotifier implements Listenable { ...@@ -166,7 +169,7 @@ class ChangeNotifier implements Listenable {
/// so, stopping that same work. /// so, stopping that same work.
@protected @protected
bool get hasListeners { bool get hasListeners {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
return _count > 0; return _count > 0;
} }
...@@ -198,7 +201,7 @@ class ChangeNotifier implements Listenable { ...@@ -198,7 +201,7 @@ class ChangeNotifier implements Listenable {
/// the list of closures that are notified when the object changes. /// the list of closures that are notified when the object changes.
@override @override
void addListener(VoidCallback listener) { void addListener(VoidCallback listener) {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
if (_count == _listeners.length) { if (_count == _listeners.length) {
if (_count == 0) { if (_count == 0) {
_listeners = List<VoidCallback?>.filled(1, null); _listeners = List<VoidCallback?>.filled(1, null);
...@@ -293,7 +296,7 @@ class ChangeNotifier implements Listenable { ...@@ -293,7 +296,7 @@ class ChangeNotifier implements Listenable {
/// This method should only be called by the object's owner. /// This method should only be called by the object's owner.
@mustCallSuper @mustCallSuper
void dispose() { void dispose() {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(() { assert(() {
_debugDisposed = true; _debugDisposed = true;
return true; return true;
...@@ -321,7 +324,7 @@ class ChangeNotifier implements Listenable { ...@@ -321,7 +324,7 @@ class ChangeNotifier implements Listenable {
@visibleForTesting @visibleForTesting
@pragma('vm:notify-debugger-on-exception') @pragma('vm:notify-debugger-on-exception')
void notifyListeners() { void notifyListeners() {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
if (_count == 0) { if (_count == 0) {
return; return;
} }
......
...@@ -473,7 +473,7 @@ abstract class RestorableProperty<T> extends ChangeNotifier { ...@@ -473,7 +473,7 @@ abstract class RestorableProperty<T> extends ChangeNotifier {
@override @override
void dispose() { void dispose() {
assert(debugAssertNotDisposed()); // FYI, This uses ChangeNotifier's _debugDisposed, not _disposed. assert(ChangeNotifier.debugAssertNotDisposed(this)); // FYI, This uses ChangeNotifier's _debugDisposed, not _disposed.
_owner?._unregister(this); _owner?._unregister(this);
super.dispose(); super.dispose();
_disposed = true; _disposed = true;
...@@ -483,14 +483,14 @@ abstract class RestorableProperty<T> extends ChangeNotifier { ...@@ -483,14 +483,14 @@ abstract class RestorableProperty<T> extends ChangeNotifier {
String? _restorationId; String? _restorationId;
RestorationMixin? _owner; RestorationMixin? _owner;
void _register(String restorationId, RestorationMixin owner) { void _register(String restorationId, RestorationMixin owner) {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(restorationId != null); assert(restorationId != null);
assert(owner != null); assert(owner != null);
_restorationId = restorationId; _restorationId = restorationId;
_owner = owner; _owner = owner;
} }
void _unregister() { void _unregister() {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(_restorationId != null); assert(_restorationId != null);
assert(_owner != null); assert(_owner != null);
_restorationId = null; _restorationId = null;
...@@ -503,14 +503,14 @@ abstract class RestorableProperty<T> extends ChangeNotifier { ...@@ -503,14 +503,14 @@ abstract class RestorableProperty<T> extends ChangeNotifier {
@protected @protected
State get state { State get state {
assert(isRegistered); assert(isRegistered);
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
return _owner!; return _owner!;
} }
/// Whether this property is currently registered with a [RestorationMixin]. /// Whether this property is currently registered with a [RestorationMixin].
@protected @protected
bool get isRegistered { bool get isRegistered {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
return _restorationId != null; return _restorationId != null;
} }
} }
......
...@@ -1092,7 +1092,7 @@ class ShortcutRegistry with ChangeNotifier { ...@@ -1092,7 +1092,7 @@ class ShortcutRegistry with ChangeNotifier {
/// ///
/// Returns a copy: modifying the returned map will have no effect. /// Returns a copy: modifying the returned map will have no effect.
Map<ShortcutActivator, Intent> get shortcuts { Map<ShortcutActivator, Intent> get shortcuts {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
return <ShortcutActivator, Intent>{ return <ShortcutActivator, Intent>{
for (final MapEntry<ShortcutRegistryEntry, Map<ShortcutActivator, Intent>> entry in _tokenShortcuts.entries) for (final MapEntry<ShortcutRegistryEntry, Map<ShortcutActivator, Intent>> entry in _tokenShortcuts.entries)
...entry.value, ...entry.value,
...@@ -1123,7 +1123,7 @@ class ShortcutRegistry with ChangeNotifier { ...@@ -1123,7 +1123,7 @@ class ShortcutRegistry with ChangeNotifier {
/// * [ShortcutRegistryEntry.dispose], a function used to remove the set of /// * [ShortcutRegistryEntry.dispose], a function used to remove the set of
/// shortcuts associated with a particular entry. /// shortcuts associated with a particular entry.
ShortcutRegistryEntry addAll(Map<ShortcutActivator, Intent> value) { ShortcutRegistryEntry addAll(Map<ShortcutActivator, Intent> value) {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
final ShortcutRegistryEntry entry = ShortcutRegistryEntry._(this); final ShortcutRegistryEntry entry = ShortcutRegistryEntry._(this);
_tokenShortcuts[entry] = value; _tokenShortcuts[entry] = value;
assert(_debugCheckForDuplicates()); assert(_debugCheckForDuplicates());
...@@ -1191,7 +1191,7 @@ class ShortcutRegistry with ChangeNotifier { ...@@ -1191,7 +1191,7 @@ class ShortcutRegistry with ChangeNotifier {
// Replaces all the shortcuts associated with the given entry from this // Replaces all the shortcuts associated with the given entry from this
// registry. // registry.
void _replaceAll(ShortcutRegistryEntry entry, Map<ShortcutActivator, Intent> value) { void _replaceAll(ShortcutRegistryEntry entry, Map<ShortcutActivator, Intent> value) {
assert(debugAssertNotDisposed()); assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(_debugCheckTokenIsValid(entry)); assert(_debugCheckTokenIsValid(entry));
_tokenShortcuts[entry] = value; _tokenShortcuts[entry] = value;
assert(_debugCheckForDuplicates()); assert(_debugCheckForDuplicates());
......
...@@ -475,6 +475,29 @@ void main() { ...@@ -475,6 +475,29 @@ void main() {
); );
}); });
test('Calling debugAssertNotDisposed works as intended', () {
final TestNotifier testNotifier = TestNotifier();
expect(ChangeNotifier.debugAssertNotDisposed(testNotifier), isTrue);
testNotifier.dispose();
FlutterError? error;
try {
ChangeNotifier.debugAssertNotDisposed(testNotifier);
} on FlutterError catch (e) {
error = e;
}
expect(error, isNotNull);
expect(error, isFlutterError);
expect(
error!.toStringDeep(),
equalsIgnoringHashCodes(
'FlutterError\n'
' A TestNotifier was used after being disposed.\n'
' Once you have called dispose() on a TestNotifier, it can no\n'
' longer be used.\n',
),
);
});
test('notifyListener can be called recursively', () { test('notifyListener can be called recursively', () {
final Counter counter = Counter(); final Counter counter = Counter();
final List<String> log = <String>[]; final List<String> log = <String>[];
......
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