From c2c183387e4ab9007d37d70cd28277e0dcd555ea Mon Sep 17 00:00:00 2001 From: davidhicks980 <59215665+davidhicks980@users.noreply.github.com> Date: Thu, 20 Jun 2024 14:10:05 -0400 Subject: [PATCH] [material/menu_anchor.dart] Remove _MenuAnchorState from parent when disposed. (#149586) `_MenuAnchorState` now removes itself from its parent regardless of whether `_MenuAnchorState._isOpen` is true. Before, _MenuAnchorState would only detach itself when `_MenuAnchorState._isOpen == true`. This led to _MenuAnchorState being retained until an anchor's parent was disposed. Before: https://github.com/flutter/flutter/assets/59215665/48f1617a-a3d0-49ab-a767-f41e3a30ea41 After: https://github.com/flutter/flutter/assets/59215665/31312bb6-49ec-4083-b8de-e9ae2a42a8b3 Fixes https://github.com/flutter/flutter/issues/149584 I was a bit unsure what the best way of testing this change would be, and wanted feedback from the Flutter team. Is there a way for us to view at the instance count of `_MenuAnchorState`, or expose `_MenuAnchorState._anchorChildren.length` for testing? - [] I added new tests to check the change I am making, or this PR is [test-exempt]. --- .../flutter/lib/src/material/menu_anchor.dart | 4 +- .../test/material/menu_anchor_test.dart | 46 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/material/menu_anchor.dart b/packages/flutter/lib/src/material/menu_anchor.dart index 04b7865106d..45f4f240b47 100644 --- a/packages/flutter/lib/src/material/menu_anchor.dart +++ b/packages/flutter/lib/src/material/menu_anchor.dart @@ -328,8 +328,10 @@ class _MenuAnchorState extends State { assert(_debugMenuInfo('Disposing of $this')); if (_isOpen) { _close(inDispose: true); - _parent?._removeChild(this); } + + _parent?._removeChild(this); + _parent = null; _anchorChildren.clear(); _menuController._detach(this); _internalMenuController = null; diff --git a/packages/flutter/test/material/menu_anchor_test.dart b/packages/flutter/test/material/menu_anchor_test.dart index 85676da1834..5063d286160 100644 --- a/packages/flutter/test/material/menu_anchor_test.dart +++ b/packages/flutter/test/material/menu_anchor_test.dart @@ -8,6 +8,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:leak_tracker/leak_tracker.dart'; import '../widgets/semantics_tester.dart'; @@ -3803,6 +3804,51 @@ void main() { ..rect(color: overlayColor.withOpacity(0.1)), ); }); + + testWidgets('Garbage collector destroys child _MenuAnchorState after parent is closed', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/149584 + await tester.pumpWidget( + MaterialApp( + home: MenuAnchor( + controller: controller, + menuChildren: const [ + SubmenuButton( + menuChildren: [], + child: Text(''), + ) + ], + ), + ), + ); + + controller.open(); + await tester.pump(); + + final WeakReference state = + WeakReference( + tester.firstState>( + find.byType(SubmenuButton), + ), + ); + expect(state.target, isNotNull); + + controller.close(); + await tester.pump(); + + controller.open(); + await tester.pump(); + + controller.close(); + await tester.pump(); + + // Garbage collect. 1 should be enough, but 3 prevents flaky tests. + await tester.runAsync(() async { + await forceGC(fullGcCycles: 3); + }); + + expect(state.target, isNull); + }, skip: kIsWeb // [intended] ForceGC does not work in web and in release mode. See https://api.flutter.dev/flutter/package-leak_tracker_leak_tracker/forceGC.html + ); } List createTestMenus({