Relax `OverlayPortal` asserts (#129053)

Fixes https://github.com/flutter/flutter/issues/129025

Also 
- simplifies OverlayPortal code a bit and adds an assert.
-  `Tooltip` shouldn't rebuild when hiding/showing the tooltip

packages/flutter/lib/src/material/tooltip.dart

@@ -425,30 +425,27 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
   // _handleMouseExit. The set is cleared in _handleTapToDismiss, typically when
   // a PointerDown event interacts with some other UI component.
   final Set<int> _activeHoveringPointerDevices = <int>{};
+
+  static bool _isTooltipVisible(AnimationStatus status) {
+    return switch (status) {
+      AnimationStatus.completed || AnimationStatus.forward || AnimationStatus.reverse => true,
+      AnimationStatus.dismissed                                                       => false,
+    };
+  }
+
   AnimationStatus _animationStatus = AnimationStatus.dismissed;
   void _handleStatusChanged(AnimationStatus status) {
     assert(mounted);
-    final bool entryNeedsUpdating;
-    switch (status) {
-      case AnimationStatus.dismissed:
-        entryNeedsUpdating = _animationStatus != AnimationStatus.dismissed;
-        if (entryNeedsUpdating) {
-          Tooltip._openedTooltips.remove(this);
-          _overlayController.hide();
-        }
-      case AnimationStatus.completed:
-      case AnimationStatus.forward:
-      case AnimationStatus.reverse:
-        entryNeedsUpdating = _animationStatus == AnimationStatus.dismissed;
-        if (entryNeedsUpdating) {
-          _overlayController.show();
-          Tooltip._openedTooltips.add(this);
-          SemanticsService.tooltip(_tooltipMessage);
-        }
-    }
-
-    if (entryNeedsUpdating) {
-      setState(() { /* Rebuild to update the OverlayEntry */ });
+    switch ((_isTooltipVisible(_animationStatus), _isTooltipVisible(status))) {
+      case (true, false):
+        Tooltip._openedTooltips.remove(this);
+        _overlayController.hide();
+      case (false, true):
+        _overlayController.show();
+        Tooltip._openedTooltips.add(this);
+        SemanticsService.tooltip(_tooltipMessage);
+      case (true, true) || (false, false):
+        break;
     }
     _animationStatus = status;
   }
@@ -753,10 +750,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
       decoration: widget.decoration ?? tooltipTheme.decoration ?? defaultDecoration,
       textStyle: widget.textStyle ?? tooltipTheme.textStyle ?? defaultTextStyle,
       textAlign: widget.textAlign ?? tooltipTheme.textAlign ?? _defaultTextAlign,
-      animation: CurvedAnimation(
-        parent: _controller,
-        curve: Curves.fastOutSlowIn,
-      ),
+      animation: CurvedAnimation(parent: _controller, curve: Curves.fastOutSlowIn),
       target: target,
       verticalOffset: widget.verticalOffset ?? tooltipTheme.verticalOffset ?? _defaultVerticalOffset,
       preferBelow: widget.preferBelow ?? tooltipTheme.preferBelow ?? _defaultPreferBelow,

packages/flutter/lib/src/painting/geometry.dart

@@ -48,7 +48,7 @@ Offset positionDependentBox({
   // VERTICAL DIRECTION
   final bool fitsBelow = target.dy + verticalOffset + childSize.height <= size.height - margin;
   final bool fitsAbove = target.dy - verticalOffset - childSize.height >= margin;
-  final bool tooltipBelow = preferBelow ? fitsBelow || !fitsAbove : !(fitsAbove || !fitsBelow);
+  final bool tooltipBelow = fitsAbove == fitsBelow ? preferBelow : fitsBelow;
   final double y;
   if (tooltipBelow) {
     y = math.min(target.dy + verticalOffset, size.height - margin);
@@ -56,19 +56,11 @@ Offset positionDependentBox({
     y = math.max(target.dy - verticalOffset - childSize.height, margin);
   }
   // HORIZONTAL DIRECTION
-  final double x;
-  if (size.width - margin * 2.0 < childSize.width) {
-    x = (size.width - childSize.width) / 2.0;
-  } else {
-    final double normalizedTargetX = clampDouble(target.dx, margin, size.width - margin);
-    final double edge = margin + childSize.width / 2.0;
-    if (normalizedTargetX < edge) {
-      x = margin;
-    } else if (normalizedTargetX > size.width - edge) {
-      x = size.width - margin - childSize.width;
-    } else {
-      x = normalizedTargetX - childSize.width / 2.0;
-    }
-  }
+  final double flexibleSpace = size.width - childSize.width;
+  final double x = flexibleSpace <= 2 * margin
+    // If there's not enough horizontal space for margin + child, center the
+    // child.
+    ? flexibleSpace / 2.0
+    : clampDouble(target.dx - childSize.width / 2, margin, flexibleSpace - margin);
   return Offset(x, y);
 }

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

@@ -1500,47 +1500,35 @@ class _OverlayPortalState extends State<OverlayPortal> {
   // used as the slot of the overlay child widget.
   //
   // The developer must call `show` to reveal the overlay so we can get a unique
-  // timestamp of the user interaction for sorting.
+  // timestamp of the user interaction for determining the z-index of the
+  // overlay child in the overlay.
   //
   // Avoid invalidating the cache if possible, since the framework uses `==` to
   // compare slots, and _OverlayEntryLocation can't override that operator since
-  // it's mutable.
+  // it's mutable. Changing slots can be relatively slow.
   bool _childModelMayHaveChanged = true;
   _OverlayEntryLocation? _locationCache;
+  static bool _isTheSameLocation(_OverlayEntryLocation locationCache, _RenderTheaterMarker marker) {
+    return locationCache._childModel == marker.overlayEntryWidgetState
+        && locationCache._theater == marker.theater;
+  }
+
   _OverlayEntryLocation _getLocation(int zOrderIndex, bool targetRootOverlay) {
     final _OverlayEntryLocation? cachedLocation = _locationCache;
-    if (cachedLocation != null && !_childModelMayHaveChanged) {
+    late final _RenderTheaterMarker marker = _RenderTheaterMarker.of(context, targetRootOverlay: targetRootOverlay);
+    final bool isCacheValid = cachedLocation != null
+                           && (!_childModelMayHaveChanged || _isTheSameLocation(cachedLocation, marker));
+    _childModelMayHaveChanged = false;
+    if (isCacheValid) {
       assert(cachedLocation._zOrderIndex == zOrderIndex);
+      assert(cachedLocation._debugIsLocationValid());
       return cachedLocation;
     }
-    _childModelMayHaveChanged = false;
-    final _RenderTheaterMarker? marker = _RenderTheaterMarker.maybeOf(context, targetRootOverlay: targetRootOverlay);
-    if (marker == null) {
-      throw FlutterError.fromParts(<DiagnosticsNode>[
-        ErrorSummary('No Overlay widget found.'),
-        ErrorDescription(
-          '${widget.runtimeType} widgets require an Overlay widget ancestor.\n'
-          'An overlay lets widgets float on top of other widget children.',
-        ),
-        ErrorHint(
-          'To introduce an Overlay widget, you can either directly '
-          'include one, or use a widget that contains an Overlay itself, '
-          'such as a Navigator, WidgetApp, MaterialApp, or CupertinoApp.',
-        ),
-        ...context.describeMissingAncestor(expectedAncestorType: Overlay),
-      ]);
-    }
-    final _OverlayEntryLocation returnValue;
-    if (cachedLocation == null) {
-      returnValue = _OverlayEntryLocation(zOrderIndex, marker.overlayEntryWidgetState, marker.theater);
-    } else if (cachedLocation._childModel != marker.overlayEntryWidgetState || cachedLocation._theater != marker.theater) {
-      cachedLocation._dispose();
-      returnValue = _OverlayEntryLocation(zOrderIndex, marker.overlayEntryWidgetState, marker.theater);
-    } else {
-      returnValue = cachedLocation;
-    }
-    assert(returnValue._zOrderIndex == zOrderIndex);
-    return _locationCache = returnValue;
+    // Otherwise invalidate the cache and create a new location.
+    cachedLocation?._debugMarkLocationInvalid();
+    final _OverlayEntryLocation newLocation = _OverlayEntryLocation(zOrderIndex, marker.overlayEntryWidgetState, marker.theater);
+    assert(newLocation._zOrderIndex == zOrderIndex);
+    return _locationCache = newLocation;
   }
 
   @override
@@ -1582,7 +1570,7 @@ class _OverlayPortalState extends State<OverlayPortal> {
   @override
   void dispose() {
     widget.controller._attachTarget = null;
-    _locationCache?._dispose();
+    _locationCache?._debugMarkLocationInvalid();
     _locationCache = null;
     super.dispose();
   }
@@ -1593,14 +1581,14 @@ class _OverlayPortalState extends State<OverlayPortal> {
       '${widget.controller.runtimeType}.show() should not be called during build.'
     );
     setState(() { _zOrderIndex = zOrderIndex; });
-    _locationCache?._dispose();
+    _locationCache?._debugMarkLocationInvalid();
     _locationCache = null;
   }
 
   void hide() {
     assert(SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks);
     setState(() { _zOrderIndex = null; });
-    _locationCache?._dispose();
+    _locationCache?._debugMarkLocationInvalid();
     _locationCache = null;
   }
 
@@ -1681,7 +1669,7 @@ final class _OverlayEntryLocation extends LinkedListEntry<_OverlayEntryLocation>
   }
 
   void _addChild(_RenderDeferredLayoutBox child) {
-    assert(_debugNotDisposed());
+    assert(_debugIsLocationValid());
     _addToChildModel(child);
     _theater._addDeferredChild(child);
     assert(child.parent == _theater);
@@ -1696,7 +1684,7 @@ final class _OverlayEntryLocation extends LinkedListEntry<_OverlayEntryLocation>
 
   void _moveChild(_RenderDeferredLayoutBox child, _OverlayEntryLocation fromLocation) {
     assert(fromLocation != this);
-    assert(_debugNotDisposed());
+    assert(_debugIsLocationValid());
     final _RenderTheater fromTheater = fromLocation._theater;
     final _OverlayEntryWidgetState fromModel = fromLocation._childModel;
 
@@ -1712,34 +1700,54 @@ final class _OverlayEntryLocation extends LinkedListEntry<_OverlayEntryLocation>
   }
 
   void _activate(_RenderDeferredLayoutBox child) {
-    assert(_debugNotDisposed());
+    // This call is allowed even when this location is invalidated.
+    // See _OverlayPortalElement.activate.
     assert(_overlayChildRenderBox == null, '$_overlayChildRenderBox');
     _theater._adoptDeferredLayoutBoxChild(child);
     _overlayChildRenderBox = child;
   }
 
   void _deactivate(_RenderDeferredLayoutBox child) {
-    assert(_debugNotDisposed());
+    // This call is allowed even when this location is invalidated.
     _theater._dropDeferredLayoutBoxChild(child);
     _overlayChildRenderBox = null;
   }
 
-  bool _debugNotDisposed() {
-    if (_debugDisposedStackTrace == null) {
+  // Throws a StateError if this location is already invalidated and shouldn't
+  // be used as an OverlayPortal slot. Must be used in asserts.
+  //
+  // Generally, `assert(_debugIsLocationValid())` should be used to prevent
+  // invalid accesses to an invalid `_OverlayEntryLocation` object. Exceptions
+  // to this rule are _removeChild, _deactive, which will be called when the
+  // OverlayPortal is being removed from the widget tree and may use the
+  // location information to perform cleanup tasks.
+  //
+  // Another exception is the _activate method which is called by
+  // _OverlayPortalElement.activate. See the comment in _OverlayPortalElement.activate.
+  bool _debugIsLocationValid() {
+    if (_debugMarkLocationInvalidStackTrace == null) {
       return true;
     }
-    throw StateError('$this is already disposed. Stack trace: $_debugDisposedStackTrace');
+    throw StateError('$this is already disposed. Stack trace: $_debugMarkLocationInvalidStackTrace');
   }
 
-  StackTrace? _debugDisposedStackTrace;
+  // The StackTrace of the first _debugMarkLocationInvalid call. It's only for
+  // debugging purposes and the StackTrace will only be captured in debug builds.
+  //
+  // The effect of this method is not reversible. Once marked invalid, this
+  // object can't be marked as valid again.
+  StackTrace? _debugMarkLocationInvalidStackTrace;
   @mustCallSuper
-  void _dispose() {
-    assert(_debugNotDisposed());
+  void _debugMarkLocationInvalid() {
+    assert(_debugIsLocationValid());
     assert(() {
-      _debugDisposedStackTrace = StackTrace.current;
+      _debugMarkLocationInvalidStackTrace = StackTrace.current;
       return true;
     }());
   }
+
+  @override
+  String toString() => '${objectRuntimeType(this, '_OverlayEntryLocation')}[${shortHash(this)}] ${_debugMarkLocationInvalidStackTrace != null ? "(INVALID)":""}';
 }
 
 class _RenderTheaterMarker extends InheritedWidget {
@@ -1758,13 +1766,31 @@ class _RenderTheaterMarker extends InheritedWidget {
         || oldWidget.overlayEntryWidgetState != overlayEntryWidgetState;
   }
 
-  static _RenderTheaterMarker? maybeOf(BuildContext context, { bool targetRootOverlay = false }) {
+  static _RenderTheaterMarker of(BuildContext context, { bool targetRootOverlay = false }) {
+    final _RenderTheaterMarker? marker;
     if (targetRootOverlay) {
       final InheritedElement? ancestor = _rootRenderTheaterMarkerOf(context.getElementForInheritedWidgetOfExactType<_RenderTheaterMarker>());
       assert(ancestor == null || ancestor.widget is _RenderTheaterMarker);
-      return ancestor != null ? context.dependOnInheritedElement(ancestor) as _RenderTheaterMarker? : null;
+      marker = ancestor != null ? context.dependOnInheritedElement(ancestor) as _RenderTheaterMarker? : null;
+    } else {
+      marker = context.dependOnInheritedWidgetOfExactType<_RenderTheaterMarker>();
+    }
+    if (marker != null) {
+      return marker;
     }
-    return context.dependOnInheritedWidgetOfExactType<_RenderTheaterMarker>();
+    throw FlutterError.fromParts(<DiagnosticsNode>[
+      ErrorSummary('No Overlay widget found.'),
+      ErrorDescription(
+        '${context.widget.runtimeType} widgets require an Overlay widget ancestor.\n'
+        'An overlay lets widgets float on top of other widget children.',
+      ),
+      ErrorHint(
+        'To introduce an Overlay widget, you can either directly '
+        'include one, or use a widget that contains an Overlay itself, '
+        'such as a Navigator, WidgetApp, MaterialApp, or CupertinoApp.',
+      ),
+      ...context.describeMissingAncestor(expectedAncestorType: Overlay),
+    ]);
   }
 
   static InheritedElement? _rootRenderTheaterMarkerOf(InheritedElement? theaterMarkerElement) {
@@ -1792,7 +1818,7 @@ class _OverlayPortal extends RenderObjectWidget {
     required this.overlayChild,
     required this.child,
   }) : assert(overlayChild == null || overlayLocation != null),
-       assert(overlayLocation == null || overlayLocation._debugNotDisposed());
+       assert(overlayLocation == null || overlayLocation._debugIsLocationValid());
 
   final Widget? overlayChild;
 
@@ -1863,6 +1889,9 @@ class _OverlayPortalElement extends RenderObjectElement {
       if (box != null) {
         assert(!box.attached);
         assert(renderObject._deferredLayoutChild == box);
+        // updateChild has not been called at this point so the RenderTheater in
+        // the overlay location could be detached. Adding children to a detached
+        // RenderObject is still allowed however this isn't the most efficient.
         (overlayChild.slot! as _OverlayEntryLocation)._activate(box);
       }
     }
@@ -1872,12 +1901,8 @@ class _OverlayPortalElement extends RenderObjectElement {
   void deactivate() {
     final Element? overlayChild = _overlayChild;
     // Instead of just detaching the render objects, removing them from the
-    // render subtree entirely such that if the widget gets reparented to a
-    // different overlay entry, the overlay child is inserted in the right
-    // position in the overlay's child list.
-    //
-    // This is also a workaround for the !renderObject.attached assert in the
-    // `RenderObjectElement.deactive()` method.
+    // render subtree entirely. This is a workaround for the
+    // !renderObject.attached assert in the `super.deactive()` method.
     if (overlayChild != null) {
       final _RenderDeferredLayoutBox? box = overlayChild.renderObject as _RenderDeferredLayoutBox?;
       if (box != null) {
@@ -1902,7 +1927,7 @@ class _OverlayPortalElement extends RenderObjectElement {
   // reparenting between _overlayChild and _child, thus the non-null-typed slots.
   @override
   void moveRenderObjectChild(_RenderDeferredLayoutBox child, _OverlayEntryLocation oldSlot, _OverlayEntryLocation newSlot) {
-    assert(newSlot._debugNotDisposed());
+    assert(newSlot._debugIsLocationValid());
     newSlot._moveChild(child, oldSlot);
   }
 

packages/flutter/test/material/tooltip_test.dart

@@ -2245,6 +2245,39 @@ void main() {
       reason: 'Tooltip should NOT be visible when hovered and tapped, when trigger mode is tap',
     );
   });
+
+  testWidgetsWithLeakTracking('Tooltip does not rebuild for fade in / fade out animation', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      const MaterialApp(
+        home: Center(
+          child: SizedBox.square(
+            dimension: 10.0,
+            child: Tooltip(
+              message: tooltipText,
+              waitDuration: Duration(seconds: 1),
+              triggerMode: TooltipTriggerMode.longPress,
+              child: SizedBox.expand(),
+            ),
+          ),
+        ),
+      ),
+    );
+    final TooltipState tooltipState = tester.state(find.byType(Tooltip));
+    final Element element = tooltipState.context as Element;
+    // The Tooltip widget itself is almost stateless thus doesn't need
+    // rebuilding.
+    expect(element.dirty, isFalse);
+
+    expect(tooltipState.ensureTooltipVisible(), isTrue);
+    expect(element.dirty, isFalse);
+    await tester.pump(const Duration(seconds: 1));
+    expect(element.dirty, isFalse);
+
+    expect(Tooltip.dismissAllToolTips(), isTrue);
+    expect(element.dirty, isFalse);
+    await tester.pump(const Duration(seconds: 1));
+    expect(element.dirty, isFalse);
+  });
 }
 
 Future<void> setWidgetForTooltipMode(

packages/flutter/test/widgets/overlay_portal_test.dart

@@ -164,6 +164,97 @@ void main() {
     await tester.pumpWidget(SizedBox(child: widget));
   });
 
+  testWidgets('Safe to hide overlay child and remove OverlayPortal in the same frame', (WidgetTester tester) async {
+    // Regression test for https://github.com/flutter/flutter/issues/129025.
+    final Widget widget = Directionality(
+      key: GlobalKey(debugLabel: 'key'),
+      textDirection: TextDirection.ltr,
+      child: Overlay(
+        initialEntries: <OverlayEntry>[
+          OverlayEntry(
+            builder: (BuildContext context) {
+              return OverlayPortal(
+                controller: controller1,
+                overlayChildBuilder: (BuildContext context) => const SizedBox(),
+                child: const SizedBox(),
+              );
+            },
+          ),
+        ],
+      ),
+    );
+
+    controller1.show();
+    await tester.pumpWidget(widget);
+
+    controller1.hide();
+    await tester.pumpWidget(const SizedBox());
+    expect(tester.takeException(), isNull);
+  });
+
+  testWidgets('Safe to hide overlay child and reparent OverlayPortal in the same frame', (WidgetTester tester) async {
+    final OverlayPortal overlayPortal = OverlayPortal(
+      key: GlobalKey(debugLabel: 'key'),
+      controller: controller1,
+      overlayChildBuilder: (BuildContext context) => const SizedBox(),
+      child: const SizedBox(),
+    );
+
+    List<Widget> children = <Widget>[ const SizedBox(), overlayPortal ];
+
+    late StateSetter setState;
+    final Widget widget = Directionality(
+      textDirection: TextDirection.ltr,
+      child: Overlay(
+        initialEntries: <OverlayEntry>[
+          OverlayStatefulEntry(
+            builder: (BuildContext context, StateSetter setter) {
+              setState = setter;
+              return Column(children: children);
+            },
+          ),
+        ],
+      ),
+    );
+
+    controller1.show();
+    await tester.pumpWidget(widget);
+
+    controller1.hide();
+    setState(() {
+      children = <Widget>[ overlayPortal, const SizedBox() ];
+    });
+    await tester.pumpWidget(widget);
+    expect(tester.takeException(), isNull);
+  });
+
+  testWidgets('Safe to hide overlay child and reparent OverlayPortal in the same frame 2', (WidgetTester tester) async {
+    final Widget widget = Directionality(
+      key: GlobalKey(debugLabel: 'key'),
+      textDirection: TextDirection.ltr,
+      child: Overlay(
+        initialEntries: <OverlayEntry>[
+          OverlayEntry(
+            builder: (BuildContext context) {
+              return OverlayPortal(
+                controller: controller1,
+                overlayChildBuilder: (BuildContext context) => const SizedBox(),
+                child: const SizedBox(),
+              );
+            },
+          ),
+        ],
+      ),
+    );
+
+    controller1.show();
+    await tester.pumpWidget(widget);
+
+    controller1.hide();
+    await tester.pumpWidget(SizedBox(child: widget));
+    expect(tester.takeException(), isNull);
+  });
+
   testWidgets('Throws when the same controller is attached to multiple OverlayPortal', (WidgetTester tester) async {
     final OverlayPortalController controller = OverlayPortalController(debugLabel: 'local controller');
     final Widget widget = Directionality(
@@ -1407,6 +1498,64 @@ void main() {
         expect(counter2.layoutCount, 3);
       });
     });
+
+    testWidgets('Safe to move the overlay child to a different Overlay and remove the old Overlay', (WidgetTester tester) async {
+      controller1.show();
+      final GlobalKey key = GlobalKey(debugLabel: 'key');
+      final GlobalKey oldOverlayKey = GlobalKey(debugLabel: 'old overlay');
+      final GlobalKey newOverlayKey = GlobalKey(debugLabel: 'new overlay');
+      final GlobalKey overlayChildKey = GlobalKey(debugLabel: 'overlay child key');
+      await tester.pumpWidget(
+        Directionality(
+          textDirection: TextDirection.ltr,
+          child: Overlay(
+            key: oldOverlayKey,
+            initialEntries: <OverlayEntry>[
+              OverlayEntry(
+                builder: (BuildContext context) {
+                  return OverlayPortal(
+                    key: key,
+                    controller: controller1,
+                    overlayChildBuilder: (BuildContext context) => SizedBox(key: overlayChildKey),
+                    child: const SizedBox(),
+                  );
+                },
+              ),
+            ],
+          ),
+        ),
+      );
+
+      expect(find.byKey(overlayChildKey), findsOneWidget);
+      expect(find.byKey(newOverlayKey), findsNothing);
+      expect(find.byKey(oldOverlayKey), findsOneWidget);
+
+      await tester.pumpWidget(
+        Directionality(
+          textDirection: TextDirection.ltr,
+          child: Overlay(
+            key: newOverlayKey,
+            initialEntries: <OverlayEntry>[
+              OverlayEntry(
+                builder: (BuildContext context) {
+                  return OverlayPortal(
+                    key: key,
+                    controller: controller1,
+                    overlayChildBuilder: (BuildContext context) => SizedBox(key: overlayChildKey),
+                    child: const SizedBox(),
+                  );
+                },
+              ),
+            ],
+          ),
+        ),
+      );
+
+      expect(tester.takeException(), isNull);
+      expect(find.byKey(overlayChildKey), findsOneWidget);
+      expect(find.byKey(newOverlayKey), findsOneWidget);
+      expect(find.byKey(oldOverlayKey), findsNothing);
+    });
   });
 
   group('Paint order', () {