diff --git a/packages/flutter/lib/src/foundation/diagnostics.dart b/packages/flutter/lib/src/foundation/diagnostics.dart index 79389543c6..294192a8ea 100644 --- a/packages/flutter/lib/src/foundation/diagnostics.dart +++ b/packages/flutter/lib/src/foundation/diagnostics.dart @@ -2486,7 +2486,7 @@ class DiagnosticsProperty extends DiagnosticsNode { json['exception'] = exception.toString(); json['propertyType'] = propertyType.toString(); json['defaultLevel'] = describeEnum(_defaultLevel); - if (T is Diagnosticable || T is DiagnosticsNode) + if (value is Diagnosticable || value is DiagnosticsNode) json['isDiagnosticableValue'] = true; return json; } diff --git a/packages/flutter/lib/src/widgets/widget_inspector.dart b/packages/flutter/lib/src/widgets/widget_inspector.dart index 77549159e3..0c03a449b9 100644 --- a/packages/flutter/lib/src/widgets/widget_inspector.dart +++ b/packages/flutter/lib/src/widgets/widget_inspector.dart @@ -1487,17 +1487,32 @@ mixin WidgetInspectorService { } if (config.includeProperties) { - List properties = node.getProperties(); - if (properties.isEmpty && value is Diagnosticable) { - properties = value.toDiagnosticsNode().getProperties(); + final List properties = node.getProperties(); + if (properties.isEmpty && node is DiagnosticsProperty + && config.expandPropertyValues && value is Diagnosticable) { + // Special case to expand property values. + json['properties'] = _nodesToJson( + value.toDiagnosticsNode().getProperties().where( + (DiagnosticsNode node) => !node.isFiltered(DiagnosticLevel.info), + ), + _SerializeConfig(groupName: config.groupName, + subtreeDepth: 0, + expandPropertyValues: false, + ), + parent: node, + ); + } else { + json['properties'] = _nodesToJson( + properties.where( + (DiagnosticsNode node) => !node.isFiltered( + createdByLocalProject ? DiagnosticLevel.fine : DiagnosticLevel + .info), + ), + _SerializeConfig.merge( + config, subtreeDepth: math.max(0, config.subtreeDepth - 1)), + parent: node, + ); } - json['properties'] = _nodesToJson( - properties.where( - (DiagnosticsNode node) => !node.isFiltered(createdByLocalProject ? DiagnosticLevel.fine : DiagnosticLevel.info), - ), - _SerializeConfig.merge(config, subtreeDepth: math.max(0, config.subtreeDepth - 1)), - parent: node, - ); } if (node is DiagnosticsProperty) { @@ -1515,18 +1530,6 @@ mixin WidgetInspectorService { 'codePoint': value.codePoint, }; } - if (config.expandPropertyValues && value is Diagnosticable) { - json['properties'] = _nodesToJson( - value.toDiagnosticsNode().getProperties().where( - (DiagnosticsNode node) => !node.isFiltered(DiagnosticLevel.info), - ), - _SerializeConfig(groupName: config.groupName, - subtreeDepth: 0, - expandPropertyValues: false, - ), - parent: node, - ); - } } return json; } diff --git a/packages/flutter/test/widgets/widget_inspector_test.dart b/packages/flutter/test/widgets/widget_inspector_test.dart index e1a3b0064b..4c09d4b657 100644 --- a/packages/flutter/test/widgets/widget_inspector_test.dart +++ b/packages/flutter/test/widgets/widget_inspector_test.dart @@ -8,6 +8,7 @@ import 'dart:io' show Platform; import 'dart:math'; import 'dart:ui' as ui; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; @@ -97,6 +98,39 @@ class _ClockTextState extends State { // End of block of code where widget creation location line numbers and // columns will impact whether tests pass. +// Class to enable building trees of nodes with cycles between properties of +// nodes and the properties of those properties. +// This exposed a bug in code serializing DiagnosticsNode objects that did not +// handle these sorts of cycles robustly. +class CyclicDiagnostic extends DiagnosticableTree { + CyclicDiagnostic(this.name); + + // Field used to create cyclic relationships. + CyclicDiagnostic related; + final List children = []; + + final String name; + + @override + String toStringShort() => '$runtimeType-$name'; + + // We have to override toString to avoid the toString call itself triggering a + // stack overflow. + @override + String toString({ DiagnosticLevel minLevel = DiagnosticLevel.debug }) { + return toStringShort(); + } + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(DiagnosticsProperty('related', related)); + } + + @override + List debugDescribeChildren() => children; +} + class _CreationLocation { const _CreationLocation({ @required this.file, @@ -1145,6 +1179,40 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { } }); + testWidgets('cyclic diagnostics regression test', (WidgetTester tester) async { + const String group = 'test-group'; + final CyclicDiagnostic a = CyclicDiagnostic('a'); + final CyclicDiagnostic b = CyclicDiagnostic('b'); + a.related = b; + a.children.add(b.toDiagnosticsNode()); + b.related = a; + + final DiagnosticsNode diagnostic = a.toDiagnosticsNode(); + final String id = service.toId(diagnostic, group); + final Map subtreeJson = await service.testExtension('getDetailsSubtree', {'arg': id, 'objectGroup': group}); + expect(subtreeJson['objectId'], equals(id)); + expect(subtreeJson.containsKey('children'), isTrue); + final List propertiesJson = subtreeJson['properties']; + expect(propertiesJson.length, equals(1)); + final Map relatedProperty = propertiesJson.first; + expect(relatedProperty['name'], equals('related')); + expect(relatedProperty['description'], equals('CyclicDiagnostic-b')); + expect(relatedProperty.containsKey('isDiagnosticableValue'), isTrue); + expect(relatedProperty.containsKey('children'), isFalse); + expect(relatedProperty.containsKey('properties'), isTrue); + final List relatedWidgetProperties = relatedProperty['properties']; + expect(relatedWidgetProperties.length, equals(1)); + final Map nestedRelatedProperty = relatedWidgetProperties.first; + expect(nestedRelatedProperty['name'], equals('related')); + // Make sure we do not include properties or children for diagnostic a + // which we already included as the root node as that would indicate a + // cycle. + expect(nestedRelatedProperty['description'], equals('CyclicDiagnostic-a')); + expect(nestedRelatedProperty.containsKey('isDiagnosticableValue'), isTrue); + expect(nestedRelatedProperty.containsKey('properties'), isFalse); + expect(nestedRelatedProperty.containsKey('children'), isFalse); + }); + testWidgets('ext.flutter.inspector.getRootWidgetSummaryTree', (WidgetTester tester) async { const String group = 'test-group'; @@ -1515,7 +1583,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { _CreationLocation location = knownLocations[id]; expect(location.file, equals(file)); // ClockText widget. - expect(location.line, equals(50)); + expect(location.line, equals(51)); expect(location.column, equals(9)); expect(count, equals(1)); @@ -1524,7 +1592,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { location = knownLocations[id]; expect(location.file, equals(file)); // Text widget in _ClockTextState build method. - expect(location.line, equals(88)); + expect(location.line, equals(89)); expect(location.column, equals(12)); expect(count, equals(1)); @@ -1549,7 +1617,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { location = knownLocations[id]; expect(location.file, equals(file)); // ClockText widget. - expect(location.line, equals(50)); + expect(location.line, equals(51)); expect(location.column, equals(9)); expect(count, equals(3)); // 3 clock widget instances rebuilt. @@ -1558,7 +1626,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { location = knownLocations[id]; expect(location.file, equals(file)); // Text widget in _ClockTextState build method. - expect(location.line, equals(88)); + expect(location.line, equals(89)); expect(location.column, equals(12)); expect(count, equals(3)); // 3 clock widget instances rebuilt.