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.

Author: LongCatIsLooong

packages/flutter/lib/src/widgets/overlay.dart

@@ -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;
     }

packages/flutter/test/widgets/overlay_test.dart

@@ -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(