From 1ceffd28cd90592dacd664799a2b8eb746d2544f Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll <43759233+kenzieschmoll@users.noreply.github.com> Date: Thu, 28 Jan 2021 18:24:08 -0800 Subject: [PATCH] Only show devtools deep links for render overflow errors (#74916) --- .../lib/src/widgets/widget_inspector.dart | 53 ++++++++--- .../test/widgets/widget_inspector_test.dart | 95 +++++++++++++++++++ 2 files changed, 137 insertions(+), 11 deletions(-) diff --git a/packages/flutter/lib/src/widgets/widget_inspector.dart b/packages/flutter/lib/src/widgets/widget_inspector.dart index 63bfe0fd07d..dffeab6e3c2 100644 --- a/packages/flutter/lib/src/widgets/widget_inspector.dart +++ b/packages/flutter/lib/src/widgets/widget_inspector.dart @@ -2873,12 +2873,19 @@ bool _isDebugCreator(DiagnosticsNode node) => node is DiagnosticsDebugCreator; /// in [WidgetsBinding.initInstances]. Iterable transformDebugCreator(Iterable properties) sync* { final List pending = []; + ErrorSummary? errorSummary; + for (final DiagnosticsNode node in properties) { + if (node is ErrorSummary) { + errorSummary = node; + break; + } + } bool foundStackTrace = false; for (final DiagnosticsNode node in properties) { if (!foundStackTrace && node is DiagnosticsStackTrace) foundStackTrace = true; if (_isDebugCreator(node)) { - yield* _parseDiagnosticsNode(node)!; + yield* _parseDiagnosticsNode(node, errorSummary)!; } else { if (foundStackTrace) { pending.add(node); @@ -2893,15 +2900,21 @@ Iterable transformDebugCreator(Iterable proper /// Transform the input [DiagnosticsNode]. /// /// Return null if input [DiagnosticsNode] is not applicable. -Iterable? _parseDiagnosticsNode(DiagnosticsNode node) { +Iterable? _parseDiagnosticsNode( + DiagnosticsNode node, + ErrorSummary? errorSummary, +) { if (!_isDebugCreator(node)) return null; final DebugCreator debugCreator = node.value! as DebugCreator; final Element element = debugCreator.element; - return _describeRelevantUserCode(element); + return _describeRelevantUserCode(element, errorSummary); } -Iterable _describeRelevantUserCode(Element element) { +Iterable _describeRelevantUserCode( + Element element, + ErrorSummary? errorSummary, +) { if (!WidgetInspectorService.instance.isWidgetCreationTracked()) { return [ ErrorDescription( @@ -2912,20 +2925,38 @@ Iterable _describeRelevantUserCode(Element element) { ErrorSpacer(), ]; } + + bool isOverflowError() { + if (errorSummary != null && errorSummary.value.isNotEmpty) { + final Object summary = errorSummary.value.first; + if (summary is String && summary.startsWith('A RenderFlex overflowed by')) { + return true; + } + } + return false; + } + final List nodes = []; bool processElement(Element target) { // TODO(chunhtai): should print out all the widgets that are about to cross // package boundaries. if (debugIsLocalCreationLocation(target)) { + DiagnosticsNode? devToolsDiagnostic; - final String? devToolsInspectorUri = - WidgetInspectorService.instance._devToolsInspectorUriForElement(target); - if (devToolsInspectorUri != null) { - devToolsDiagnostic = DevToolsDeepLinkProperty( - 'To inspect this widget in Flutter DevTools, visit: $devToolsInspectorUri', - devToolsInspectorUri, - ); + + // TODO(kenz): once the inspector is better at dealing with broken trees, + // we can enable deep links for more errors than just RenderFlex overflow + // errors. See https://github.com/flutter/flutter/issues/74918. + if (isOverflowError()) { + final String? devToolsInspectorUri = + WidgetInspectorService.instance._devToolsInspectorUriForElement(target); + if (devToolsInspectorUri != null) { + devToolsDiagnostic = DevToolsDeepLinkProperty( + 'To inspect this widget in Flutter DevTools, visit: $devToolsInspectorUri', + devToolsInspectorUri, + ); + } } nodes.addAll([ diff --git a/packages/flutter/test/widgets/widget_inspector_test.dart b/packages/flutter/test/widgets/widget_inspector_test.dart index 50de0114445..d8fe8956dcd 100644 --- a/packages/flutter/test/widgets/widget_inspector_test.dart +++ b/packages/flutter/test/widgets/widget_inspector_test.dart @@ -1044,6 +1044,101 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { expect(nodes[4].runtimeType, DiagnosticsStackTrace); }, skip: WidgetInspectorService.instance.isWidgetCreationTracked()); // Test requires --no-track-widget-creation flag. + testWidgets('test transformDebugCreator will add DevToolsDeepLinkProperty for overflow errors', (WidgetTester tester) async { + activeDevToolsServerAddress = 'http://127.0.0.1:9100'; + connectedVmServiceUri = 'http://127.0.0.1:55269/798ay5al_FM=/'; + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Stack( + children: const [ + Text('a'), + Text('b', textDirection: TextDirection.ltr), + Text('c', textDirection: TextDirection.ltr), + ], + ), + ), + ); + final Element elementA = find.text('a').evaluate().first; + + final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder(); + builder.add(ErrorSummary('A RenderFlex overflowed by 273 pixels on the bottom')); + builder.add(DiagnosticsDebugCreator(DebugCreator(elementA))); + builder.add(StringProperty('dummy2', 'value')); + + final List nodes = List.from(transformDebugCreator(builder.properties)); + expect(nodes.length, 6); + expect(nodes[0].runtimeType, ErrorSummary); + expect(nodes[1].runtimeType, DiagnosticsBlock); + expect(nodes[2].runtimeType, ErrorSpacer); + expect(nodes[3].runtimeType, DevToolsDeepLinkProperty); + expect(nodes[4].runtimeType, ErrorSpacer); + expect(nodes[5].runtimeType, StringProperty); + }, skip: !WidgetInspectorService.instance.isWidgetCreationTracked()); + + 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=/'; + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Stack( + children: const [ + Text('a'), + Text('b', textDirection: TextDirection.ltr), + Text('c', textDirection: TextDirection.ltr), + ], + ), + ), + ); + final Element elementA = find.text('a').evaluate().first; + + final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder(); + builder.add(ErrorSummary('some other error')); + builder.add(DiagnosticsDebugCreator(DebugCreator(elementA))); + builder.add(StringProperty('dummy2', 'value')); + + final List nodes = List.from(transformDebugCreator(builder.properties)); + expect(nodes.length, 4); + expect(nodes[0].runtimeType, ErrorSummary); + expect(nodes[1].runtimeType, DiagnosticsBlock); + expect(nodes[2].runtimeType, ErrorSpacer); + expect(nodes[3].runtimeType, StringProperty); + }, skip: !WidgetInspectorService.instance.isWidgetCreationTracked()); + + 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=/'; + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Stack( + children: const [ + Text('a'), + Text('b', textDirection: TextDirection.ltr), + Text('c', textDirection: TextDirection.ltr), + ], + ), + ), + ); + final Element elementA = find.text('a').evaluate().first; + + final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder(); + builder.add(ErrorSummary('A RenderFlex overflowed by 273 pixels on the bottom')); + builder.add(DiagnosticsDebugCreator(DebugCreator(elementA))); + builder.add(StringProperty('dummy2', 'value')); + + final List nodes = List.from(transformDebugCreator(builder.properties)); + expect(nodes.length, 4); + expect(nodes[0].runtimeType, ErrorSummary); + expect(nodes[1].runtimeType, DiagnosticsBlock); + expect(nodes[2].runtimeType, ErrorSpacer); + expect(nodes[3].runtimeType, StringProperty); + }, skip: !WidgetInspectorService.instance.isWidgetCreationTracked()); + testWidgets('WidgetInspectorService setPubRootDirectories', (WidgetTester tester) async { await tester.pumpWidget( Directionality(