From 97167ed5a62db0f2bd4c9ddfc7a128ddd2393aa9 Mon Sep 17 00:00:00 2001 From: Kishan Rathore <34465683+rkishan516@users.noreply.github.com> Date: Wed, 26 Mar 2025 07:46:26 +0530 Subject: [PATCH] Fix: Remove attach target on deactivation of widget from overlay portal controller (#164439) Fix: Remove attach target on deactivation of widget from overlay portal controller fixes: #164376 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --- packages/flutter/lib/src/widgets/overlay.dart | 10 +- .../test/widgets/overlay_portal_test.dart | 173 ++++++++++++++++++ 2 files changed, 181 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart index 6f8ae86385..a7733d4983 100644 --- a/packages/flutter/lib/src/widgets/overlay.dart +++ b/packages/flutter/lib/src/widgets/overlay.dart @@ -1920,7 +1920,8 @@ class _OverlayPortalState extends State { void _setupController(OverlayPortalController controller) { assert( - controller._attachTarget == null || controller._attachTarget == this, + controller._attachTarget == this || + !((controller._attachTarget?.context as StatefulElement?)?.debugIsActive ?? false), 'Failed to attach $controller to $this. It is already attached to ${controller._attachTarget}.', ); final int? controllerZOrderIndex = controller._zOrderIndex; @@ -1951,8 +1952,13 @@ class _OverlayPortalState extends State { } @override - void dispose() { + void activate() { assert(widget.controller._attachTarget == this); + super.activate(); + } + + @override + void dispose() { widget.controller._attachTarget = null; _locationCache?._debugMarkLocationInvalid(); _locationCache = null; diff --git a/packages/flutter/test/widgets/overlay_portal_test.dart b/packages/flutter/test/widgets/overlay_portal_test.dart index 57f0226c9c..2fde5200e7 100644 --- a/packages/flutter/test/widgets/overlay_portal_test.dart +++ b/packages/flutter/test/widgets/overlay_portal_test.dart @@ -1365,6 +1365,179 @@ void main() { verifyTreeIsClean(); }); + testWidgets('PortalController can be assigned to another after deactivate', ( + WidgetTester tester, + ) async { + final OverlayPortalController controller1 = OverlayPortalController(); + final GlobalKey overlayKey = GlobalKey(); + + final OverlayEntry overlayEntry1 = OverlayEntry( + builder: (BuildContext context) { + return OverlayPortal( + controller: controller1, + overlayChildBuilder: (BuildContext context) => const Placeholder(), + ); + }, + ); + + final OverlayEntry overlayEntry2 = OverlayEntry( + builder: (BuildContext context) { + return OverlayPortal( + controller: controller1, + overlayChildBuilder: (BuildContext context) => const Placeholder(), + ); + }, + ); + + addTearDown(() { + overlayEntry1 + ..remove() + ..dispose(); + overlayEntry2.dispose(); + }); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Overlay(key: overlayKey, initialEntries: [overlayEntry1]), + ), + ); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Overlay(key: overlayKey, initialEntries: [overlayEntry2]), + ), + ); + + verifyTreeIsClean(); + expect(tester.takeException(), isNull); + }); + + testWidgets('Reactivation maintains portal state', (WidgetTester tester) async { + final OverlayPortalController controller1 = OverlayPortalController(); + final GlobalKey> portalKey = GlobalKey>(); + + late OverlayEntry overlayEntry1, overlayEntry2; + addTearDown(() { + overlayEntry1 + ..remove() + ..dispose(); + overlayEntry2 + ..remove() + ..dispose(); + }); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Overlay( + initialEntries: [ + overlayEntry1 = OverlayEntry( + builder: + (BuildContext context) => OverlayPortal( + key: portalKey, + controller: controller1, + overlayChildBuilder: (BuildContext context) => const Placeholder(), + ), + ), + ], + ), + ), + ); + + controller1.show(); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: SizedBox( + child: Overlay( + initialEntries: [ + overlayEntry2 = OverlayEntry( + builder: + (BuildContext context) => OverlayPortal( + key: portalKey, + controller: controller1, + overlayChildBuilder: (BuildContext context) => const Placeholder(), + ), + ), + ], + ), + ), + ), + ); + + expect(find.byType(Placeholder), findsOneWidget); + expect(controller1.isShowing, equals(true)); + }); + + testWidgets('attachTarget is restored after reparenting', (WidgetTester tester) async { + final GlobalKey> portalKey = GlobalKey>(); + final RenderBox childBox = RenderConstrainedBox(additionalConstraints: const BoxConstraints()); + final RenderBox overlayChildBox = RenderConstrainedBox( + additionalConstraints: const BoxConstraints(), + ); + + bool moveToSecondOverlay = false; + + final Widget child = WidgetToRenderBoxAdapter(renderBox: childBox); + final Widget overlayChild = WidgetToRenderBoxAdapter(renderBox: overlayChildBox); + + final OverlayEntry overlayEntry1 = OverlayEntry( + builder: (BuildContext context) { + return !moveToSecondOverlay + ? OverlayPortal( + key: portalKey, + controller: controller1, + overlayChildBuilder: (BuildContext context) => overlayChild, + child: child, + ) + : const SizedBox(); + }, + ); + final OverlayEntry overlayEntry2 = OverlayEntry( + builder: (BuildContext context) { + return moveToSecondOverlay + ? OverlayPortal( + key: portalKey, + controller: controller1, + overlayChildBuilder: (BuildContext context) => overlayChild, + child: child, + ) + : const SizedBox(); + }, + ); + addTearDown(() { + overlayEntry1 + ..remove() + ..dispose(); + overlayEntry2 + ..remove() + ..dispose(); + }); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Stack( + children: [ + Overlay(initialEntries: [overlayEntry1]), + Overlay(initialEntries: [overlayEntry2]), + ], + ), + ), + ); + + // Move to second overlay + moveToSecondOverlay = true; + overlayEntry1.markNeedsBuild(); + overlayEntry2.markNeedsBuild(); + await tester.pump(); + + verifyTreeIsClean(); + }); + group('GlobalKey Reparenting', () { testWidgets('child is laid out before overlay child after OverlayEntry shuffle', ( WidgetTester tester,