diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index f18d0a58e3..c3db9cc8c0 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -3085,8 +3085,21 @@ abstract class Element extends DiagnosticableTree implements BuildContext { /// The configuration for this element. @override - Widget get widget => _widget; - Widget _widget; + Widget get widget => _widget!; + Widget? _widget; + + /// Returns true if the Element is defunct. + /// + /// This getter always returns false in profile and release builds. + /// See the lifecycle documentation for [Element] for additional information. + bool get debugIsDefunct { + bool isDefunct = false; + assert(() { + isDefunct = _lifecycleState == _ElementLifecycle.defunct; + return true; + }()); + return isDefunct; + } /// The object that manages the lifecycle of this element. @override @@ -3801,10 +3814,14 @@ abstract class Element extends DiagnosticableTree implements BuildContext { assert(depth != null); assert(owner != null); // Use the private property to avoid a CastError during hot reload. - final Key? key = _widget.key; + final Key? key = _widget!.key; if (key is GlobalKey) { owner!._unregisterGlobalKey(key, this); } + // Release resources to reduce the severity of memory leaks caused by + // defunct, but accidentally retained Elements. + _widget = null; + _dependencies = null; _lifecycleState = _ElementLifecycle.defunct; } @@ -4116,7 +4133,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { /// A short, textual description of this element. @override - String toStringShort() => widget.toStringShort(); + String toStringShort() => _widget?.toStringShort() ?? '${describeIdentity(this)}(DEFUNCT)'; @override DiagnosticsNode toDiagnosticsNode({ String? name, DiagnosticsTreeStyle? style }) { @@ -4134,9 +4151,9 @@ abstract class Element extends DiagnosticableTree implements BuildContext { if (_lifecycleState != _ElementLifecycle.initial) { properties.add(ObjectFlagProperty('depth', depth, ifNull: 'no depth')); } - properties.add(ObjectFlagProperty('widget', widget, ifNull: 'no widget')); - properties.add(DiagnosticsProperty('key', widget.key, showName: false, defaultValue: null, level: DiagnosticLevel.hidden)); - widget.debugFillProperties(properties); + properties.add(ObjectFlagProperty('widget', _widget, ifNull: 'no widget')); + properties.add(DiagnosticsProperty('key', _widget?.key, showName: false, defaultValue: null, level: DiagnosticLevel.hidden)); + _widget?.debugFillProperties(properties); properties.add(FlagProperty('dirty', value: dirty, ifTrue: 'dirty')); if (_dependencies != null && _dependencies!.isNotEmpty) { final List diagnosticsDependencies = _dependencies! @@ -4296,7 +4313,9 @@ class _ElementDiagnosticableTreeNode extends DiagnosticableTreeNode { Map toJsonMap(DiagnosticsSerializationDelegate delegate) { final Map json = super.toJsonMap(delegate); final Element element = value as Element; - json['widgetRuntimeType'] = element.widget.runtimeType.toString(); + if (!element.debugIsDefunct) { + json['widgetRuntimeType'] = element.widget.runtimeType.toString(); + } json['stateful'] = stateful; return json; } @@ -4660,7 +4679,7 @@ class StatelessElement extends ComponentElement { class StatefulElement extends ComponentElement { /// Creates an element that uses the given widget as its configuration. StatefulElement(StatefulWidget widget) - : state = widget.createState(), + : _state = widget.createState(), super(widget) { assert(() { if (!state._debugTypesAreRight(widget)) { @@ -4695,7 +4714,8 @@ class StatefulElement extends ComponentElement { /// There is a one-to-one relationship between [State] objects and the /// [StatefulElement] objects that hold them. The [State] objects are created /// by [StatefulElement] in [mount]. - final State state; + State get state => _state!; + State? _state; @override void reassemble() { @@ -4810,6 +4830,9 @@ class StatefulElement extends ComponentElement { ]); }()); state._element = null; + // Release resources to reduce the severity of memory leaks caused by + // defunct, but accidentally retained Elements. + _state = null; } @override @@ -4895,7 +4918,7 @@ class StatefulElement extends ComponentElement { @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); - properties.add(DiagnosticsProperty>('state', state, defaultValue: null)); + properties.add(DiagnosticsProperty>('state', _state, defaultValue: null)); } } @@ -5705,13 +5728,14 @@ abstract class RenderObjectElement extends Element { @override void unmount() { + final RenderObjectWidget oldWidget = widget; super.unmount(); assert( !renderObject.attached, 'A RenderObject was still attached when attempting to unmount its ' 'RenderObjectElement: $renderObject', ); - widget.didUnmountRenderObject(renderObject); + oldWidget.didUnmountRenderObject(renderObject); } void _updateParentData(ParentDataWidget parentDataWidget) { diff --git a/packages/flutter/lib/src/widgets/widget_inspector.dart b/packages/flutter/lib/src/widgets/widget_inspector.dart index f17411a6e1..cad2636c66 100644 --- a/packages/flutter/lib/src/widgets/widget_inspector.dart +++ b/packages/flutter/lib/src/widgets/widget_inspector.dart @@ -1205,6 +1205,7 @@ mixin WidgetInspectorService { /// /// Use this method only for testing to ensure that object references from one /// test case do not impact other test cases. + @visibleForTesting @protected void disposeAllGroups() { _groups.clear(); @@ -1213,6 +1214,19 @@ mixin WidgetInspectorService { _nextId = 0; } + /// Reset all InspectorService state. + /// + /// Use this method only for testing to write hermetic tests for + /// WidgetInspectorService. + @visibleForTesting + @protected + @mustCallSuper + void resetAllState() { + disposeAllGroups(); + selection.clear(); + setPubRootDirectories([]); + } + /// Free all references to objects in a group. /// /// Objects and their associated ids in the group may be kept alive by @@ -2413,7 +2427,8 @@ class InspectorSelection { /// Setting [candidates] or calling [clear] resets the selection. /// /// Returns null if the selection is invalid. - RenderObject? get current => _current; + RenderObject? get current => active ? _current : null; + RenderObject? _current; set current(RenderObject? value) { if (_current != value) { @@ -2427,7 +2442,10 @@ class InspectorSelection { /// Setting [candidates] or calling [clear] resets the selection. /// /// Returns null if the selection is invalid. - Element? get currentElement => _currentElement; + Element? get currentElement { + return _currentElement?.debugIsDefunct ?? true ? null : _currentElement; + } + Element? _currentElement; set currentElement(Element? element) { if (currentElement != element) { @@ -3039,7 +3057,7 @@ String? _describeCreationLocation(Object object) { /// /// Currently creation locations are only available for [Widget] and [Element]. _Location? _getCreationLocation(Object? object) { - final Object? candidate = object is Element ? object.widget : object; + final Object? candidate = object is Element && !object.debugIsDefunct ? object.widget : object; return candidate is _HasCreationLocation ? candidate._location : null; } diff --git a/packages/flutter/test/widgets/framework_test.dart b/packages/flutter/test/widgets/framework_test.dart index 5b871fc990..91350bd4ef 100644 --- a/packages/flutter/test/widgets/framework_test.dart +++ b/packages/flutter/test/widgets/framework_test.dart @@ -1572,6 +1572,22 @@ void main() { await tester.pumpWidget(Container()); expect(tester.binding.buildOwner!.globalKeyCount, initialCount + 0); }); + + testWidgets('Widget and State properties are nulled out when unmounted', (WidgetTester tester) async { + await tester.pumpWidget(const _StatefulLeaf()); + final StatefulElement element = tester.element(find.byType(_StatefulLeaf)); + expect(element.state, isA>()); + expect(element.widget, isA<_StatefulLeaf>()); + // Replace the widget tree to unmount the element. + await tester.pumpWidget(Container()); + // Accessing state/widget now throws a CastError because they have been + // nulled out to reduce severity of memory leaks when an Element (e.g. in + // the form of a BuildContext) is retained past its useful life. See also + // https://github.com/flutter/flutter/issues/79605 for examples why this may + // occur. + expect(() => element.state, throwsA(isA())); + expect(() => element.widget, throwsA(isA())); + }, skip: kIsWeb); } class _WidgetWithNoVisitChildren extends StatelessWidget { diff --git a/packages/flutter/test/widgets/widget_inspector_test.dart b/packages/flutter/test/widgets/widget_inspector_test.dart index 13d912d07e..aa2d11d2b7 100644 --- a/packages/flutter/test/widgets/widget_inspector_test.dart +++ b/packages/flutter/test/widgets/widget_inspector_test.dart @@ -233,6 +233,10 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { final TestWidgetInspectorService service = TestWidgetInspectorService(); WidgetInspectorService.instance = service; + tearDown(() { + service.resetAllState(); + }); + testWidgets('WidgetInspector smoke test', (WidgetTester tester) async { // This is a smoke test to verify that adding the inspector doesn't crash. await tester.pumpWidget( @@ -759,6 +763,61 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { expect(service.selection.currentElement, equals(elementA)); }); + testWidgets('WidgetInspectorService defunct selection regression test', (WidgetTester tester) async { + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Stack( + children: const [ + Text('a', textDirection: TextDirection.ltr), + ], + ), + ), + ); + final Element elementA = find.text('a').evaluate().first; + + service.setSelection(elementA); + expect(service.selection.currentElement, equals(elementA)); + expect(service.selection.current, equals(elementA.renderObject)); + + await tester.pumpWidget( + const SizedBox( + child: Text('b', textDirection: TextDirection.ltr), + ), + ); + // Selection is now empty as the element is defunct. + expect(service.selection.currentElement, equals(null)); + expect(service.selection.current, equals(null)); + + // Verify that getting the debug creation location of the defunct element + // does not crash. + expect(debugIsLocalCreationLocation(elementA), isFalse); + + // Verify that generating json for a defunct element does not crash. + expect( + elementA.toDiagnosticsNode().toJsonMap( + InspectorSerializationDelegate( + service: service, + summaryTree: false, + includeProperties: true, + addAdditionalPropertiesCallback: null, + ), + ), + isNotNull, + ); + + final Element elementB = find.text('b').evaluate().first; + service.setSelection(elementB); + expect(service.selection.currentElement, equals(elementB)); + expect(service.selection.current, equals(elementB.renderObject)); + + // Set selection back to a defunct element. + service.setSelection(elementA); + + expect(service.selection.currentElement, equals(null)); + expect(service.selection.current, equals(null)); + }); + testWidgets('WidgetInspectorService getParentChain', (WidgetTester tester) async { const String group = 'test-group'; @@ -934,6 +993,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { ), ); final Element elementA = find.text('a').evaluate().first; + service.setSelection(elementA, 'my-group'); late String pubRootTest; if (widgetTracked) { final Map jsonObject = json.decode( @@ -1047,6 +1107,8 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { activeDevToolsServerAddress = 'http://127.0.0.1:9100'; connectedVmServiceUri = 'http://127.0.0.1:55269/798ay5al_FM=/'; + setupDefaultPubRootDirectory(service); + await tester.pumpWidget( Directionality( textDirection: TextDirection.ltr, @@ -1079,6 +1141,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { testWidgets('test transformDebugCreator will not add DevToolsDeepLinkProperty for non-overflow errors', (WidgetTester tester) async { activeDevToolsServerAddress = 'http://127.0.0.1:9100'; connectedVmServiceUri = 'http://127.0.0.1:55269/798ay5al_FM=/'; + setupDefaultPubRootDirectory(service); await tester.pumpWidget( Directionality( @@ -1110,6 +1173,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { testWidgets('test transformDebugCreator will not add DevToolsDeepLinkProperty if devtoolsServerAddress is unavailable', (WidgetTester tester) async { activeDevToolsServerAddress = null; connectedVmServiceUri = 'http://127.0.0.1:55269/798ay5al_FM=/'; + setupDefaultPubRootDirectory(service); await tester.pumpWidget( Directionality( @@ -2782,6 +2846,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { ), ), ); + service.setSelection(find.text('Hello, World!').evaluate().first, 'my-group'); // Figure out the pubRootDirectory final Map jsonObject = (await service.testExtension( @@ -2855,10 +2920,13 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { final Map additionalJson = {}; final Object? value = node.value; if (value is Element) { - additionalJson['renderObject'] = - value.renderObject!.toDiagnosticsNode().toJsonMap( - delegate.copyWith(subtreeDepth: 0), - ); + final RenderObject? renderObject = value.renderObject; + if (renderObject != null) { + additionalJson['renderObject'] = + renderObject.toDiagnosticsNode().toJsonMap( + delegate.copyWith(subtreeDepth: 0), + ); + } } additionalJson['callbackExecuted'] = true; return additionalJson; @@ -2892,6 +2960,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { }); testWidgets('debugIsLocalCreationLocation test', (WidgetTester tester) async { + setupDefaultPubRootDirectory(service); final GlobalKey key = GlobalKey(); @@ -2953,6 +3022,23 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { ); }); } + + static void setupDefaultPubRootDirectory(TestWidgetInspectorService service) { + final Map jsonObject = const SizedBox().toDiagnosticsNode().toJsonMap(InspectorSerializationDelegate(service: service)); + final Map creationLocation = jsonObject['creationLocation']! as Map; + expect(creationLocation, isNotNull); + final String file = creationLocation['file']! as String; + expect(file, endsWith('widget_inspector_test.dart')); + final List segments = Uri + .parse(file) + .pathSegments; + final String pubRootTest = '/' + + segments.take(segments.length - 2).join('/'); + + // Strip a couple subdirectories away to generate a plausible pub root + // directory. + service.setPubRootDirectories([pubRootTest]); + } } void addToKnownLocationsMap({ diff --git a/packages/flutter/test/widgets/widget_inspector_test_utils.dart b/packages/flutter/test/widgets/widget_inspector_test_utils.dart index 948aefa612..94d1d796b4 100644 --- a/packages/flutter/test/widgets/widget_inspector_test_utils.dart +++ b/packages/flutter/test/widgets/widget_inspector_test_utils.dart @@ -62,4 +62,11 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { binding.buildOwner!.reassemble(binding.renderViewElement!); } } + + @override + void resetAllState() { + super.resetAllState(); + eventsDispatched.clear(); + rebuildCount = 0; + } }