Unverified Commit 0b3c5d12 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Defer the OverlayEntry listenable disposal until its widget is unmounted (#102794)

parent cdb4831d
...@@ -45,9 +45,12 @@ import 'ticker_provider.dart'; ...@@ -45,9 +45,12 @@ import 'ticker_provider.dart';
/// if widgets in an overlay entry with [maintainState] set to true repeatedly /// if widgets in an overlay entry with [maintainState] set to true repeatedly
/// call [State.setState], the user's battery will be drained unnecessarily. /// call [State.setState], the user's battery will be drained unnecessarily.
/// ///
/// [OverlayEntry] is a [ChangeNotifier] that notifies when the widget built by /// [OverlayEntry] is a [Listenable] that notifies when the widget built by
/// [builder] is mounted or unmounted, whose exact state can be queried by /// [builder] is mounted or unmounted, whose exact state can be queried by
/// [mounted]. /// [mounted]. After the owner of the [OverlayEntry] calls [remove] and then
/// [dispose], the widget may not be immediately removed from the widget tree.
/// As a result listeners of the [OverlayEntry] can get notified for one last
/// time after the [dispose] call, when the widget is eventually unmounted.
/// ///
/// See also: /// See also:
/// ///
...@@ -55,7 +58,7 @@ import 'ticker_provider.dart'; ...@@ -55,7 +58,7 @@ import 'ticker_provider.dart';
/// * [OverlayState] /// * [OverlayState]
/// * [WidgetsApp] /// * [WidgetsApp]
/// * [MaterialApp] /// * [MaterialApp]
class OverlayEntry extends ChangeNotifier { class OverlayEntry implements Listenable {
/// Creates an overlay entry. /// Creates an overlay entry.
/// ///
/// To insert the entry into an [Overlay], first find the overlay using /// To insert the entry into an [Overlay], first find the overlay using
...@@ -86,6 +89,7 @@ class OverlayEntry extends ChangeNotifier { ...@@ -86,6 +89,7 @@ class OverlayEntry extends ChangeNotifier {
bool get opaque => _opaque; bool get opaque => _opaque;
bool _opaque; bool _opaque;
set opaque(bool value) { set opaque(bool value) {
assert(!_disposedByOwner);
if (_opaque == value) if (_opaque == value)
return; return;
_opaque = value; _opaque = value;
...@@ -109,6 +113,7 @@ class OverlayEntry extends ChangeNotifier { ...@@ -109,6 +113,7 @@ class OverlayEntry extends ChangeNotifier {
bool get maintainState => _maintainState; bool get maintainState => _maintainState;
bool _maintainState; bool _maintainState;
set maintainState(bool value) { set maintainState(bool value) {
assert(!_disposedByOwner);
assert(_maintainState != null); assert(_maintainState != null);
if (_maintainState == value) if (_maintainState == value)
return; return;
...@@ -120,14 +125,21 @@ class OverlayEntry extends ChangeNotifier { ...@@ -120,14 +125,21 @@ class OverlayEntry extends ChangeNotifier {
/// Whether the [OverlayEntry] is currently mounted in the widget tree. /// Whether the [OverlayEntry] is currently mounted in the widget tree.
/// ///
/// The [OverlayEntry] notifies its listeners when this value changes. /// The [OverlayEntry] notifies its listeners when this value changes.
bool get mounted => _mounted; bool get mounted => _overlayStateMounted.value;
bool _mounted = false;
void _updateMounted(bool value) { /// Whether the `_OverlayState`s built using this [OverlayEntry] is currently
if (value == _mounted) { /// mounted.
return; final ValueNotifier<bool> _overlayStateMounted = ValueNotifier<bool>(false);
}
_mounted = value; @override
notifyListeners(); void addListener(VoidCallback listener) {
assert(!_disposedByOwner);
_overlayStateMounted.addListener(listener);
}
@override
void removeListener(VoidCallback listener) {
_overlayStateMounted.removeListener(listener);
} }
OverlayState? _overlay; OverlayState? _overlay;
...@@ -145,6 +157,7 @@ class OverlayEntry extends ChangeNotifier { ...@@ -145,6 +157,7 @@ class OverlayEntry extends ChangeNotifier {
/// until the next frame (i.e. many milliseconds later). /// until the next frame (i.e. many milliseconds later).
void remove() { void remove() {
assert(_overlay != null); assert(_overlay != null);
assert(!_disposedByOwner);
final OverlayState overlay = _overlay!; final OverlayState overlay = _overlay!;
_overlay = null; _overlay = null;
if (!overlay.mounted) if (!overlay.mounted)
...@@ -164,9 +177,40 @@ class OverlayEntry extends ChangeNotifier { ...@@ -164,9 +177,40 @@ class OverlayEntry extends ChangeNotifier {
/// ///
/// You need to call this function if the output of [builder] has changed. /// You need to call this function if the output of [builder] has changed.
void markNeedsBuild() { void markNeedsBuild() {
assert(!_disposedByOwner);
_key.currentState?._markNeedsBuild(); _key.currentState?._markNeedsBuild();
} }
void _didUnmount() {
assert(!mounted);
if (_disposedByOwner) {
_overlayStateMounted.dispose();
}
}
bool _disposedByOwner = false;
/// Discards any resources used by this [OverlayEntry].
///
/// This method must be called after [remove] if the [OverlayEntry] is
/// inserted into an [Overlay].
///
/// After this is called, the object is not in a usable state and should be
/// discarded (calls to [addListener] will throw after the object is disposed).
/// However, the listeners registered may not be immediately released until
/// the widget built using this [OverlayEntry] is unmounted from the widget
/// tree.
///
/// This method should only be called by the object's owner.
void dispose() {
assert(!_disposedByOwner);
assert(_overlay == null, 'An OverlayEntry must first be removed from the Overlay before dispose is called.');
_disposedByOwner = true;
if (!mounted) {
_overlayStateMounted.dispose();
}
}
@override @override
String toString() => '${describeIdentity(this)}(opaque: $opaque; maintainState: $maintainState)'; String toString() => '${describeIdentity(this)}(opaque: $opaque; maintainState: $maintainState)';
} }
...@@ -192,12 +236,13 @@ class _OverlayEntryWidgetState extends State<_OverlayEntryWidget> { ...@@ -192,12 +236,13 @@ class _OverlayEntryWidgetState extends State<_OverlayEntryWidget> {
@override @override
void initState() { void initState() {
super.initState(); super.initState();
widget.entry._updateMounted(true); widget.entry._overlayStateMounted.value = true;
} }
@override @override
void dispose() { void dispose() {
widget.entry._updateMounted(false); widget.entry._overlayStateMounted.value = false;
widget.entry._didUnmount();
super.dispose(); super.dispose();
} }
......
...@@ -1057,6 +1057,115 @@ void main() { ...@@ -1057,6 +1057,115 @@ void main() {
expect(renderObject.clipBehavior, clip); expect(renderObject.clipBehavior, clip);
} }
}); });
group('OverlayEntry listenable', () {
final GlobalKey overlayKey = GlobalKey();
final Widget emptyOverlay = Directionality(
textDirection: TextDirection.ltr,
child: Overlay(key: overlayKey),
);
testWidgets('mounted state can be listened', (WidgetTester tester) async {
await tester.pumpWidget(emptyOverlay);
final OverlayState overlay = overlayKey.currentState! as OverlayState;
final List<bool> mountedLog = <bool>[];
final OverlayEntry entry = OverlayEntry(
builder: (BuildContext context) => Container(),
);
entry.addListener(() {
mountedLog.add(entry.mounted);
});
overlay.insert(entry);
expect(mountedLog, isEmpty);
// Pump a frame. The Overlay entry will be mounted.
await tester.pump();
expect(mountedLog, <bool>[true]);
entry.remove();
expect(mountedLog, <bool>[true]);
await tester.pump();
expect(mountedLog, <bool>[true, false]);
// Insert & remove again.
overlay.insert(entry);
await tester.pump();
entry.remove();
await tester.pump();
expect(mountedLog, <bool>[true, false, true, false]);
});
testWidgets('throw if disposed before removal', (WidgetTester tester) async {
await tester.pumpWidget(emptyOverlay);
final OverlayState overlay = overlayKey.currentState! as OverlayState;
final OverlayEntry entry = OverlayEntry(
builder: (BuildContext context) => Container(),
);
overlay.insert(entry);
Object? error;
try {
entry.dispose();
} catch (e) {
error = e;
}
expect(error, isAssertionError);
});
test('dispose works', () {
final OverlayEntry entry = OverlayEntry(
builder: (BuildContext context) => Container(),
);
entry.dispose();
Object? error;
try {
entry.addListener(() { });
} catch (e) {
error = e;
}
expect(error, isAssertionError);
});
testWidgets('delayed dispose', (WidgetTester tester) async {
await tester.pumpWidget(emptyOverlay);
final OverlayState overlay = overlayKey.currentState! as OverlayState;
final List<bool> mountedLog = <bool>[];
final OverlayEntry entry = OverlayEntry(
builder: (BuildContext context) => Container(),
);
entry.addListener(() {
mountedLog.add(entry.mounted);
});
overlay.insert(entry);
await tester.pump();
expect(mountedLog, <bool>[true]);
entry.remove();
// Call dispose on the entry. The listeners should be notified for one
// last time after this.
entry.dispose();
expect(mountedLog, <bool>[true]);
await tester.pump();
expect(mountedLog, <bool>[true, false]);
expect(tester.takeException(), isNull);
// The entry is no longer usable.
Object? error;
try {
entry.addListener(() { });
} catch (e) {
error = e;
}
expect(error, isAssertionError);
});
});
} }
class StatefulTestWidget extends StatefulWidget { class StatefulTestWidget extends StatefulWidget {
......
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