Unverified Commit 5de6684b authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Add more info to `OverlayState.insert` error messages (#129363)

I was debugging an Overlay issue and felt I could have identified the problem faster if the existing assertions provided more information about the current state of the OverlayEntry and Overlay.
parent 0bc5a2bc
......@@ -222,7 +222,7 @@ class OverlayEntry implements Listenable {
}
@override
String toString() => '${describeIdentity(this)}(opaque: $opaque; maintainState: $maintainState)';
String toString() => '${describeIdentity(this)}(opaque: $opaque; maintainState: $maintainState)${_disposedByOwner ? "(DISPOSED)" : ""}';
}
class _OverlayEntryWidget extends StatefulWidget {
......@@ -296,7 +296,7 @@ class _OverlayEntryWidgetState extends State<_OverlayEntryWidget> {
late final Iterable<RenderBox> _hitTestOrderIterable = _createChildIterable(reversed: true);
// The following uses sync* because hit-testing is lazy, and LinkedList as a
// Iterable doesn't support current modification.
// Iterable doesn't support concurrent modification.
Iterable<RenderBox> _createChildIterable({ required bool reversed }) sync* {
final LinkedList<_OverlayEntryLocation>? children = _sortedTheaterSiblings;
if (children == null || children.isEmpty) {
......@@ -543,6 +543,55 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
return _entries.length;
}
bool _debugCanInsertEntry(OverlayEntry entry) {
final List<DiagnosticsNode> operandsInformation = <DiagnosticsNode>[
DiagnosticsProperty<OverlayEntry>('The OverlayEntry was', entry, style: DiagnosticsTreeStyle.errorProperty),
DiagnosticsProperty<OverlayState>(
'The Overlay the OverlayEntry was trying to insert to was', this, style: DiagnosticsTreeStyle.errorProperty,
),
];
if (!mounted) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Attempted to insert an OverlayEntry to an already disposed Overlay.'),
...operandsInformation,
]);
}
final OverlayState? currentOverlay = entry._overlay;
final bool alreadyContainsEntry = _entries.contains(entry);
if (alreadyContainsEntry) {
final bool inconsistentOverlayState = !identical(currentOverlay, this);
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('The specified entry is already present in the target Overlay.'),
...operandsInformation,
if (inconsistentOverlayState) ErrorHint('This could be an error in the Flutter framework.')
else ErrorHint(
'Consider calling remove on the OverlayEntry before inserting it to a different Overlay, '
'or switching to the OverlayPortal API to avoid manual OverlayEntry management.'
),
if (inconsistentOverlayState) DiagnosticsProperty<OverlayState>(
"The OverlayEntry's current Overlay was", currentOverlay, style: DiagnosticsTreeStyle.errorProperty,
),
]);
}
if (currentOverlay == null) {
return true;
}
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('The specified entry is already present in a different Overlay.'),
...operandsInformation,
DiagnosticsProperty<OverlayState>("The OverlayEntry's current Overlay was", currentOverlay, style: DiagnosticsTreeStyle.errorProperty,),
ErrorHint(
'Consider calling remove on the OverlayEntry before inserting it to a different Overlay, '
'or switching to the OverlayPortal API to avoid manual OverlayEntry management.'
)
]);
}
/// Insert the given entry into the overlay.
///
/// If `below` is non-null, the entry is inserted just below `below`.
......@@ -552,8 +601,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
/// It is an error to specify both `above` and `below`.
void insert(OverlayEntry entry, { OverlayEntry? below, OverlayEntry? above }) {
assert(_debugVerifyInsertPosition(above, below));
assert(!_entries.contains(entry), 'The specified entry is already present in the Overlay.');
assert(entry._overlay == null, 'The specified entry is already present in another Overlay.');
assert(_debugCanInsertEntry(entry));
entry._overlay = this;
setState(() {
_entries.insert(_insertionIndex(below, above), entry);
......@@ -569,14 +617,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
/// It is an error to specify both `above` and `below`.
void insertAll(Iterable<OverlayEntry> entries, { OverlayEntry? below, OverlayEntry? above }) {
assert(_debugVerifyInsertPosition(above, below));
assert(
entries.every((OverlayEntry entry) => !_entries.contains(entry)),
'One or more of the specified entries are already present in the Overlay.',
);
assert(
entries.every((OverlayEntry entry) => entry._overlay == null),
'One or more of the specified entries are already present in another Overlay.',
);
assert(entries.every(_debugCanInsertEntry));
if (entries.isEmpty) {
return;
}
......
......@@ -1139,6 +1139,72 @@ void main() {
);
});
testWidgets('OverlayEntry throws if inserted to an invalid Overlay', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: Overlay(),
),
);
final OverlayState overlay = tester.state(find.byType(Overlay));
final OverlayEntry entry = OverlayEntry(builder: (BuildContext context) => const SizedBox());
expect(
() => overlay.insert(entry),
returnsNormally,
);
// Throws when inserted to the same Overlay.
expect(
() => overlay.insert(entry),
throwsA(isA<FlutterError>().having(
(FlutterError error) => error.toString(),
'toString()',
allOf(
contains('The specified entry is already present in the target Overlay.'),
contains('The OverlayEntry was'),
contains('The Overlay the OverlayEntry was trying to insert to was'),
),
)),
);
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: SizedBox(child: Overlay()),
),
);
// Throws if inserted to an already disposed Overlay.
expect(
() => overlay.insert(entry),
throwsA(isA<FlutterError>().having(
(FlutterError error) => error.toString(),
'toString()',
allOf(
contains('Attempted to insert an OverlayEntry to an already disposed Overlay.'),
contains('The OverlayEntry was'),
contains('The Overlay the OverlayEntry was trying to insert to was'),
),
)),
);
final OverlayState newOverlay = tester.state(find.byType(Overlay));
// Throws when inserted to a different Overlay without calling remove.
expect(
() => newOverlay.insert(entry),
throwsA(isA<FlutterError>().having(
(FlutterError error) => error.toString(),
'toString()',
allOf(
contains('The specified entry is already present in a different Overlay.'),
contains('The OverlayEntry was'),
contains('The Overlay the OverlayEntry was trying to insert to was'),
contains("The OverlayEntry's current Overlay was"),
),
)),
);
});
group('OverlayEntry listenable', () {
final GlobalKey overlayKey = GlobalKey();
final Widget emptyOverlay = Directionality(
......
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