[Widget Inspector] Fix stack overflow error for Flutter web when requesting a large widget tree (#159454)

Fixes https://github.com/flutter/devtools/issues/8553

Context: 

A Flutter web customer with a large widget tree was getting a stack
overflow error when they toggled on "show implementation widgets" in the
Flutter DevTools Inspector. This is because building the JSON tree
recursively was hitting Chrome's stack limit.

This PR creates the JSON tree **iteratively** if the `getRootWidgetTree`
service extension is called with `fullDetails = false` (which is what
DevTools uses to fetch the widget tree).

For all other instances of creating a widget JSON map (for example, when
fetching widget properties) the recursive implementation is used. This
allows properties provided by subclasses implementing `toJsonMap` to be
included in the response.

Note: Because with this change `toJsonMap` is only called when
`fullDetails = true` and `toJsonMapIterative` is only called when
`fullDetails = false`, this PR partially reverts the changes in
https://github.com/flutter/flutter/pull/157309.
This commit is contained in:
Elliott Brooks 2024-11-26 13:22:23 -08:00 committed by GitHub
parent 4ff6698838
commit 21bea32f66
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 170 additions and 154 deletions

View File

@ -8,6 +8,7 @@
/// @docImport 'package:flutter/widgets.dart'; /// @docImport 'package:flutter/widgets.dart';
library; library;
import 'dart:collection';
import 'dart:math' as math; import 'dart:math' as math;
import 'dart:ui' show clampDouble; import 'dart:ui' show clampDouble;
@ -1421,6 +1422,17 @@ class TextTreeRenderer {
} }
} }
/// The JSON representation of a [DiagnosticsNode].
typedef _JsonDiagnosticsNode = Map<String, Object?>;
/// Stack containing [DiagnosticNode]s to convert to JSON and the callback to
/// call with the JSON.
///
/// Using a stack is required to process the widget tree iteratively instead of
/// recursively.
typedef _NodesToJsonifyStack
= ListQueue<(DiagnosticsNode, void Function(_JsonDiagnosticsNode))>;
/// Defines diagnostics data for a [value]. /// Defines diagnostics data for a [value].
/// ///
/// For debug and profile modes, [DiagnosticsNode] provides a high quality /// For debug and profile modes, [DiagnosticsNode] provides a high quality
@ -1605,29 +1617,12 @@ abstract class DiagnosticsNode {
/// by this method and interactive tree views in the Flutter IntelliJ /// by this method and interactive tree views in the Flutter IntelliJ
/// plugin. /// plugin.
@mustCallSuper @mustCallSuper
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, {
bool fullDetails = true,
}) {
Map<String, Object?> result = <String, Object?>{}; Map<String, Object?> result = <String, Object?>{};
assert(() { assert(() {
final bool hasChildren = getChildren().isNotEmpty; final bool hasChildren = getChildren().isNotEmpty;
final Map<String, Object?> essentialDetails = <String, Object?>{ result = <String, Object?>{
'description': toDescription(), 'description': toDescription(),
'shouldIndent': style != DiagnosticsTreeStyle.flat &&
style != DiagnosticsTreeStyle.error,
...delegate.additionalNodeProperties(this, fullDetails: fullDetails),
if (delegate.subtreeDepth > 0)
'children': toJsonList(
delegate.filterChildren(getChildren(), this),
this,
delegate,
fullDetails: fullDetails,
),
};
result = !fullDetails ? essentialDetails : <String, Object?>{
...essentialDetails,
'type': runtimeType.toString(), 'type': runtimeType.toString(),
if (name != null) if (name != null)
'name': name, 'name': name,
@ -1651,12 +1646,18 @@ abstract class DiagnosticsNode {
'allowWrap': allowWrap, 'allowWrap': allowWrap,
if (allowNameWrap) if (allowNameWrap)
'allowNameWrap': allowNameWrap, 'allowNameWrap': allowNameWrap,
...delegate.additionalNodeProperties(this),
if (delegate.includeProperties) if (delegate.includeProperties)
'properties': toJsonList( 'properties': toJsonList(
delegate.filterProperties(getProperties(), this), delegate.filterProperties(getProperties(), this),
this, this,
delegate, delegate,
fullDetails: fullDetails, ),
if (delegate.subtreeDepth > 0)
'children': toJsonList(
delegate.filterChildren(getChildren(), this),
this,
delegate,
), ),
}; };
return true; return true;
@ -1664,6 +1665,35 @@ abstract class DiagnosticsNode {
return result; return result;
} }
/// Iteratively serialize the node to a JSON map according to the
/// configuration provided in the [DiagnosticsSerializationDelegate].
///
/// This is only used when [WidgetInspectorServiceExtensions.getRootWidgetTree]
/// is called with fullDetails=false. To get the full widget details, including
/// any details provided by subclasses, [toJsonMap] should be used instead.
///
/// See https://github.com/flutter/devtools/issues/8553 for details about this
/// iterative approach.
Map<String, Object?> toJsonMapIterative(
DiagnosticsSerializationDelegate delegate,
) {
final _NodesToJsonifyStack childrenToJsonify =
ListQueue<(DiagnosticsNode, void Function(_JsonDiagnosticsNode))>();
_JsonDiagnosticsNode result = <String, Object?>{};
assert(() {
result = _toJson(
delegate,
childrenToJsonify: childrenToJsonify,
);
_jsonifyNextNodesInStack(
childrenToJsonify,
delegate: delegate,
);
return true;
}());
return result;
}
/// Serializes a [List] of [DiagnosticsNode]s to a JSON list according to /// Serializes a [List] of [DiagnosticsNode]s to a JSON list according to
/// the configuration provided by the [DiagnosticsSerializationDelegate]. /// the configuration provided by the [DiagnosticsSerializationDelegate].
/// ///
@ -1672,9 +1702,8 @@ abstract class DiagnosticsNode {
static List<Map<String, Object?>> toJsonList( static List<Map<String, Object?>> toJsonList(
List<DiagnosticsNode>? nodes, List<DiagnosticsNode>? nodes,
DiagnosticsNode? parent, DiagnosticsNode? parent,
DiagnosticsSerializationDelegate delegate, { DiagnosticsSerializationDelegate delegate,
bool fullDetails = true, ) {
}) {
bool truncated = false; bool truncated = false;
if (nodes == null) { if (nodes == null) {
return const <Map<String, Object?>>[]; return const <Map<String, Object?>>[];
@ -1685,11 +1714,9 @@ abstract class DiagnosticsNode {
nodes.add(DiagnosticsNode.message('...')); nodes.add(DiagnosticsNode.message('...'));
truncated = true; truncated = true;
} }
final List<Map<String, Object?>> json = nodes.map<Map<String, Object?>>((DiagnosticsNode node) { final List<_JsonDiagnosticsNode> json =
return node.toJsonMap( nodes.map<_JsonDiagnosticsNode>((DiagnosticsNode node) {
delegate.delegateForNode(node), return node.toJsonMap(delegate.delegateForNode(node));
fullDetails: fullDetails,
);
}).toList(); }).toList();
if (truncated) { if (truncated) {
json.last['truncated'] = true; json.last['truncated'] = true;
@ -1803,6 +1830,73 @@ abstract class DiagnosticsNode {
}()); }());
return result; return result;
} }
void _jsonifyNextNodesInStack(
_NodesToJsonifyStack toJsonify, {
required DiagnosticsSerializationDelegate delegate,
}) {
while (toJsonify.isNotEmpty) {
final (
DiagnosticsNode nextNode,
void Function(_JsonDiagnosticsNode) callback
) = toJsonify.removeFirst();
final _JsonDiagnosticsNode nodeAsJson = nextNode._toJson(
delegate,
childrenToJsonify: toJsonify,
);
callback(nodeAsJson);
}
}
Map<String, Object?> _toJson(
DiagnosticsSerializationDelegate delegate, {
required _NodesToJsonifyStack childrenToJsonify,
}) {
final List<_JsonDiagnosticsNode> childrenJsonList =
<_JsonDiagnosticsNode>[];
final bool includeChildren =
getChildren().isNotEmpty && delegate.subtreeDepth > 0;
// Collect the children nodes to convert to JSON later.
bool truncated = false;
if (includeChildren) {
List<DiagnosticsNode> childrenNodes =
delegate.filterChildren(getChildren(), this);
final int originalNodeCount = childrenNodes.length;
childrenNodes = delegate.truncateNodesList(childrenNodes, this);
if (childrenNodes.length != originalNodeCount) {
childrenNodes.add(DiagnosticsNode.message('...'));
truncated = true;
}
for (final DiagnosticsNode child in childrenNodes) {
childrenToJsonify.add((
child,
(_JsonDiagnosticsNode jsonChild) {
childrenJsonList.add(jsonChild);
}
));
}
}
final String description = toDescription();
final String widgetRuntimeType =
description == '[root]' ? 'RootWidget' : description.split('-').first;
final bool shouldIndent = style != DiagnosticsTreeStyle.flat &&
style != DiagnosticsTreeStyle.error;
return <String, Object?>{
'description': description,
'shouldIndent': shouldIndent,
// TODO(elliette): This can be removed to reduce the JSON response even
// further once DevTools computes the widget runtime type from the
// description instead, see:
// https://github.com/flutter/devtools/issues/8556
'widgetRuntimeType': widgetRuntimeType,
'truncated': truncated,
...delegate.additionalNodeProperties(this, fullDetails: false),
if (includeChildren) 'children': childrenJsonList,
};
}
} }
/// Debugging message displayed like a property. /// Debugging message displayed like a property.
@ -1872,17 +1966,8 @@ class StringProperty extends DiagnosticsProperty<String> {
final bool quoted; final bool quoted;
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
json['quoted'] = quoted; json['quoted'] = quoted;
return json; return json;
} }
@ -1937,18 +2022,8 @@ abstract class _NumProperty<T extends num> extends DiagnosticsProperty<T> {
}) : super.lazy(); }) : super.lazy();
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (unit != null) { if (unit != null) {
json['unit'] = unit; json['unit'] = unit;
} }
@ -2131,17 +2206,8 @@ class FlagProperty extends DiagnosticsProperty<bool> {
); );
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (ifTrue != null) { if (ifTrue != null) {
json['ifTrue'] = ifTrue; json['ifTrue'] = ifTrue;
} }
@ -2262,17 +2328,8 @@ class IterableProperty<T> extends DiagnosticsProperty<Iterable<T>> {
} }
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (value != null) { if (value != null) {
json['values'] = value!.map<String>((T value) => value.toString()).toList(); json['values'] = value!.map<String>((T value) => value.toString()).toList();
} }
@ -2409,17 +2466,8 @@ class ObjectFlagProperty<T> extends DiagnosticsProperty<T> {
} }
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (ifPresent != null) { if (ifPresent != null) {
json['ifPresent'] = ifPresent; json['ifPresent'] = ifPresent;
} }
@ -2496,17 +2544,8 @@ class FlagsSummary<T> extends DiagnosticsProperty<Map<String, T?>> {
} }
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (value.isNotEmpty) { if (value.isNotEmpty) {
json['values'] = _formattedValues().toList(); json['values'] = _formattedValues().toList();
} }
@ -2625,10 +2664,7 @@ class DiagnosticsProperty<T> extends DiagnosticsNode {
final bool allowNameWrap; final bool allowNameWrap;
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, {
bool fullDetails = true,
}) {
final T? v = value; final T? v = value;
List<Map<String, Object?>>? properties; List<Map<String, Object?>>? properties;
if (delegate.expandPropertyValues && delegate.includeProperties && v is Diagnosticable && getProperties().isEmpty) { if (delegate.expandPropertyValues && delegate.includeProperties && v is Diagnosticable && getProperties().isEmpty) {
@ -2638,16 +2674,9 @@ class DiagnosticsProperty<T> extends DiagnosticsNode {
delegate.filterProperties(v.toDiagnosticsNode().getProperties(), this), delegate.filterProperties(v.toDiagnosticsNode().getProperties(), this),
this, this,
delegate, delegate,
fullDetails: fullDetails,
); );
} }
final Map<String, Object?> json = super.toJsonMap( final Map<String, Object?> json = super.toJsonMap(delegate);
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (properties != null) { if (properties != null) {
json['properties'] = properties; json['properties'] = properties;
} }

View File

@ -495,17 +495,8 @@ class ColorProperty extends DiagnosticsProperty<Color> {
}); });
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (value != null) { if (value != null) {
json['valueProperties'] = <String, Object>{ json['valueProperties'] = <String, Object>{
'red': value!.red, 'red': value!.red,

View File

@ -5382,18 +5382,13 @@ class _ElementDiagnosticableTreeNode extends DiagnosticableTreeNode {
final bool stateful; final bool stateful;
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(delegate, fullDetails: fullDetails,);
final Element element = value as Element; final Element element = value as Element;
if (!element.debugIsDefunct) { if (!element.debugIsDefunct) {
json['widgetRuntimeType'] = element.widget.runtimeType.toString(); json['widgetRuntimeType'] = element.widget.runtimeType.toString();
} }
if (fullDetails) {
json['stateful'] = stateful; json['stateful'] = stateful;
}
return json; return json;
} }
} }

View File

@ -121,17 +121,8 @@ class IconDataProperty extends DiagnosticsProperty<IconData> {
}); });
@override @override
Map<String, Object?> toJsonMap( Map<String, Object?> toJsonMap(DiagnosticsSerializationDelegate delegate) {
DiagnosticsSerializationDelegate delegate, { final Map<String, Object?> json = super.toJsonMap(delegate);
bool fullDetails = true,
}) {
final Map<String, Object?> json = super.toJsonMap(
delegate,
fullDetails: fullDetails,
);
if (!fullDetails) {
return json;
}
if (value != null) { if (value != null) {
json['valueProperties'] = <String, Object>{ json['valueProperties'] = <String, Object>{
'codePoint': value!.codePoint, 'codePoint': value!.codePoint,

View File

@ -1754,7 +1754,15 @@ mixin WidgetInspectorService {
bool fullDetails = true, bool fullDetails = true,
} }
) { ) {
return node?.toJsonMap(delegate, fullDetails: fullDetails); if (fullDetails) {
return node?.toJsonMap(delegate);
} else {
// If we don't need the full details fetched from all the subclasses, we
// can iteratively build the JSON map. This prevents a stack overflow
// exception for particularly large widget trees. For details, see:
// https://github.com/flutter/devtools/issues/8553
return node?.toJsonMapIterative(delegate);
}
} }
bool _isValueCreatedByLocalProject(Object? value) { bool _isValueCreatedByLocalProject(Object? value) {
@ -1824,14 +1832,8 @@ mixin WidgetInspectorService {
List<DiagnosticsNode> nodes, List<DiagnosticsNode> nodes,
InspectorSerializationDelegate delegate, { InspectorSerializationDelegate delegate, {
required DiagnosticsNode? parent, required DiagnosticsNode? parent,
bool fullDetails = true,
}) { }) {
return DiagnosticsNode.toJsonList( return DiagnosticsNode.toJsonList(nodes, parent, delegate);
nodes,
parent,
delegate,
fullDetails: fullDetails,
);
} }
/// Returns a JSON representation of the properties of the [DiagnosticsNode] /// Returns a JSON representation of the properties of the [DiagnosticsNode]

View File

@ -24,10 +24,15 @@ void main() {
}); });
group('Serialization', () { group('Serialization', () {
const List<String> essentialDiagnosticKeys = <String>[ // These are always included.
const List<String> defaultDiagnosticKeys = <String>[
'description', 'description',
];
// These are only included when fullDetails = false.
const List<String> essentialDiagnosticKeys = <String>[
'shouldIndent', 'shouldIndent',
]; ];
// These are only included with fullDetails = true.
const List<String> detailedDiagnosticKeys = <String>[ const List<String> detailedDiagnosticKeys = <String>[
'type', 'type',
'hasChildren', 'hasChildren',
@ -81,7 +86,7 @@ void main() {
expect(result.containsKey('properties'), isFalse); expect(result.containsKey('properties'), isFalse);
expect(result.containsKey('children'), isFalse); expect(result.containsKey('children'), isFalse);
for (final String keyName in essentialDiagnosticKeys) { for (final String keyName in defaultDiagnosticKeys) {
expect( expect(
result.containsKey(keyName), result.containsKey(keyName),
isTrue, isTrue,
@ -97,15 +102,20 @@ void main() {
} }
}); });
test('without full details', () { test('iterative implementation (without full details)', () {
final Map<String, Object?> result = testTree final Map<String, Object?> result = testTree
.toDiagnosticsNode() .toDiagnosticsNode()
.toJsonMap( .toJsonMapIterative(const DiagnosticsSerializationDelegate()
const DiagnosticsSerializationDelegate(), fullDetails: false
); );
expect(result.containsKey('properties'), isFalse); expect(result.containsKey('properties'), isFalse);
expect(result.containsKey('children'), isFalse); expect(result.containsKey('children'), isFalse);
for (final String keyName in defaultDiagnosticKeys) {
expect(
result.containsKey(keyName),
isTrue,
reason: '$keyName is included.',
);
}
for (final String keyName in essentialDiagnosticKeys) { for (final String keyName in essentialDiagnosticKeys) {
expect( expect(
result.containsKey(keyName), result.containsKey(keyName),

View File

@ -2177,7 +2177,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService {
/// Gets the children nodes from the JSON response. /// Gets the children nodes from the JSON response.
List<Object?> childrenFromJsonResponse(Map<String, Object?> json) { List<Object?> childrenFromJsonResponse(Map<String, Object?> json) {
return json['children']! as List<Object?>; return (json['children'] as List<Object?>?) ?? <Object?>[];
} }
/// Gets the children nodes using a call to /// Gets the children nodes using a call to
@ -2571,7 +2571,6 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService {
), ),
isTrue, isTrue,
); );
expect( expect(
allChildrenSatisfyCondition(rootJson, allChildrenSatisfyCondition(rootJson,
condition: wasCreatedByLocalProject, condition: wasCreatedByLocalProject,
@ -5758,7 +5757,6 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService {
node.toJsonMap(const DiagnosticsSerializationDelegate()), node.toJsonMap(const DiagnosticsSerializationDelegate()),
equals(<String, dynamic>{ equals(<String, dynamic>{
'description': 'description of the deep link', 'description': 'description of the deep link',
'shouldIndent': true,
'type': 'DevToolsDeepLinkProperty', 'type': 'DevToolsDeepLinkProperty',
'name': '', 'name': '',
'style': 'singleLine', 'style': 'singleLine',