Elide tree walks (#48413)
diff --git a/packages/flutter/lib/src/foundation/assertions.dart b/packages/flutter/lib/src/foundation/assertions.dart index 7381a7b..6f8c868 100644 --- a/packages/flutter/lib/src/foundation/assertions.dart +++ b/packages/flutter/lib/src/foundation/assertions.dart
@@ -20,6 +20,130 @@ /// and other callbacks that collect information describing an error. typedef InformationCollector = Iterable<DiagnosticsNode> Function(); +/// Partial information from a stack frame for stack filtering purposes. +/// +/// See also: +/// +/// * [SubStackFilter], which uses this class to compare against [StackFrame]s. +@immutable +class PartialStackFrame { + /// Creates a new [PartialStackFrame] instance. All arguments are required and + /// must not be null. + const PartialStackFrame({ + @required this.package, + @required this.className, + @required this.method, + }) : assert(className != null), + assert(method != null), + assert(package != null); + + /// An `<asynchronous suspension>` line in a stack trace. + static const PartialStackFrame asynchronousSuspension = PartialStackFrame( + package: '', + className: '', + method: 'asynchronous suspension', + ); + + /// The package to match, e.g. `package:flutter/src/foundation/assertions.dart`, + /// or `dart:ui/window.dart`. + final Pattern package; + + /// The class name for the method. + /// + /// On web, this is ignored, since class names are not available. + /// + /// On all platforms, top level methods should use the empty string. + final String className; + + /// The method name for this frame line. + /// + /// On web, private methods are wrapped with `[]`. + final String method; + + /// Tests whether the [StackFrame] matches the information in this + /// [PartialStackFrame]. + bool matches(StackFrame stackFrame) { + final String stackFramePackage = '${stackFrame.packageScheme}:${stackFrame.package}/${stackFrame.packagePath}'; + // Ideally this wouldn't be necessary. + // TODO(dnfield): https://github.com/dart-lang/sdk/issues/40117 + if (kIsWeb) { + return package.allMatches(stackFramePackage).isNotEmpty + && stackFrame.method == (method.startsWith('_') ? '[$method]' : method); + } + return package.allMatches(stackFramePackage).isNotEmpty + && stackFrame.method == method + && stackFrame.className == className; + } +} + +/// A class that filters stack frames for additional filtering on +/// [FlutterError.defaultStackFilter]. +abstract class StackFilter { + /// A const constructor to allow subclasses to be const. + const StackFilter(); + + /// Filters the list of [StackFrame]s by updating corrresponding indices in + /// `reasons`. + /// + /// To elide a frame or number of frames, set the string + void filter(List<StackFrame> stackFrames, List<String> reasons); +} + + +/// A [StackFilter] that filters based on repeating lists of +/// [PartialStackFrame]s. +/// +/// See also: +/// +/// * [FlutterError.addDefaultStackFilter], a method to register additional +/// stack filters for [FlutterError.defaultStackFilter]. +/// * [StackFrame], a class that can help with parsing stack frames. +/// * [PartialStackFrame], a class that helps match partial method information +/// to a stack frame. +class RepetitiveStackFrameFilter extends StackFilter { + /// Creates a new SubStackFilter. All parameters are required and must not be + /// null. + const RepetitiveStackFrameFilter({ + @required this.frames, + @required this.replacement, + }) : assert(frames != null), + assert(replacement != null); + + /// The shape of this repetative stack pattern. + final List<PartialStackFrame> frames; + + /// The number of frames in this pattern. + int get numFrames => frames.length; + + /// The string to replace the frames with. + /// + /// If the same replacement string is used multiple times in a row, the + /// [FlutterError.defaultStackFilter] will simply update a counter after this + /// line rather than repeating it. + final String replacement; + + List<String> get _replacements => List<String>.filled(numFrames, replacement); + + @override + void filter(List<StackFrame> stackFrames, List<String> reasons) { + for (int index = 0; index < stackFrames.length; index += 1) { + if (_matchesFrames(stackFrames.skip(index).take(numFrames).toList())) { + reasons.setRange(index, index + numFrames, _replacements); + index += numFrames; + } + } + } + + bool _matchesFrames(List<StackFrame> stackFrames) { + for (int index = 0; index < stackFrames.length; index++) { + if (!frames[index].matches(stackFrames[index])) { + return false; + } + } + return true; + } +} + abstract class _ErrorDiagnostic extends DiagnosticsProperty<List<Object>> { /// This constructor provides a reliable hook for a kernel transformer to find /// error messages that need to be rewritten to include object references for @@ -653,6 +777,19 @@ _errorCount += 1; } + static final List<StackFilter> _stackFilters = <StackFilter>[]; + + /// Adds a stack filtering function to [defaultStackFilter]. + /// + /// For example, the framework adds common patterns of element building to + /// elide tree-walking patterns in the stacktrace. + /// + /// Added filters are checked in order of addition. The first matching filter + /// wins, and subsequent filters will not be checked. + static void addDefaultStackFilter(StackFilter filter) { + _stackFilters.add(filter); + } + /// Converts a stack to a string that is more readable by omitting stack /// frames that correspond to Dart internals. /// @@ -665,38 +802,76 @@ /// format but the frame numbers will not be consecutive (frames are elided) /// and the final line may be prose rather than a stack frame. static Iterable<String> defaultStackFilter(Iterable<String> frames) { - const Set<String> filteredPackages = <String>{ - 'dart:async-patch', - 'dart:async', - 'package:stack_trace', + final Map<String, int> removedPackagesAndClasses = <String, int>{ + 'dart:async-patch': 0, + 'dart:async': 0, + 'package:stack_trace': 0, + 'class _AssertionError': 0, + 'class _FakeAsync': 0, + 'class _FrameCallbackEntry': 0, + 'class _Timer': 0, + 'class _RawReceivePortImpl': 0, }; - const Set<String> filteredClasses = <String>{ - '_AssertionError', - '_FakeAsync', - '_FrameCallbackEntry', - }; - final List<String> result = <String>[]; - final List<String> skipped = <String>[]; - for (final String line in frames) { - final StackFrame frameLine = StackFrame.fromStackTraceLine(line); - if (filteredClasses.contains(frameLine.className)) { - skipped.add('class ${frameLine.className}'); - } else if (filteredPackages.contains(frameLine.packageScheme + ':' + frameLine.package)) { - skipped.add('package ${frameLine.packageScheme == 'dart' ? 'dart:' : ''}${frameLine.package}'); - } else { - result.add(line); + int skipped = 0; + + final List<StackFrame> parsedFrames = StackFrame.fromStackString(frames.join('\n')); + + for (int index = 0; index < parsedFrames.length; index += 1) { + final StackFrame frame = parsedFrames[index]; + final String className = 'class ${frame.className}'; + final String package = '${frame.packageScheme}:${frame.package}'; + if (removedPackagesAndClasses.containsKey(className)) { + skipped += 1; + removedPackagesAndClasses[className] += 1; + parsedFrames.removeAt(index); + index -= 1; + } else if (removedPackagesAndClasses.containsKey(package)) { + skipped += 1; + removedPackagesAndClasses[package] += 1; + parsedFrames.removeAt(index); + index -= 1; } } - if (skipped.length == 1) { - result.add('(elided one frame from ${skipped.single})'); - } else if (skipped.length > 1) { - final List<String> where = Set<String>.from(skipped).toList()..sort(); + final List<String> reasons = List<String>(parsedFrames.length); + for (final StackFilter filter in _stackFilters) { + filter.filter(parsedFrames, reasons); + } + + final List<String> result = <String>[]; + + // Collapse duplicated reasons. + for (int index = 0; index < parsedFrames.length; index += 1) { + final int start = index; + while (index < reasons.length - 1 && reasons[index] != null && reasons[index + 1] == reasons[index]) { + index++; + } + String suffix = ''; + if (reasons[index] != null) { + if (index != start) { + suffix = ' (${index - start + 2} frames)'; + } else { + suffix = ' (1 frame)'; + } + } + final String resultLine = '${reasons[index] ?? parsedFrames[index].source}$suffix'; + result.add(resultLine); + } + + // Only include packages we actually elided from. + final List<String> where = <String>[ + for (MapEntry<String, int> entry in removedPackagesAndClasses.entries) + if (entry.value > 0) + entry.key + ]..sort(); + if (skipped == 1) { + result.add('(elided one frame from ${where.single})'); + } else if (skipped > 1) { if (where.length > 1) where[where.length - 1] = 'and ${where.last}'; if (where.length > 2) { - result.add('(elided ${skipped.length} frames from ${where.join(", ")})'); + result.add('(elided $skipped frames from ${where.join(", ")})'); } else { - result.add('(elided ${skipped.length} frames from ${where.join(" ")})'); + result.add('(elided $skipped frames from ${where.join(" ")})'); } } return result;
diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index 8e3d813..ab7af49 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart
@@ -254,6 +254,12 @@ void initInstances() { super.initInstances(); _instance = this; + + assert(() { + _debugAddStackFilters(); + return true; + }()); + // Initialization of [_buildOwner] has to be done after // [super.initInstances] is called, as it requires [ServicesBinding] to // properly setup the [defaultBinaryMessenger] instance. @@ -265,6 +271,84 @@ FlutterErrorDetails.propertiesTransformers.add(transformDebugCreator); } + void _debugAddStackFilters() { + const PartialStackFrame elementInflateWidget = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'Element', method: 'inflateWidget'); + const PartialStackFrame elementUpdateChild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'Element', method: 'updateChild'); + const PartialStackFrame elementRebuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'Element', method: 'rebuild'); + const PartialStackFrame componentElementPerformRebuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'ComponentElement', method: 'performRebuild'); + const PartialStackFrame componentElementFristBuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'ComponentElement', method: '_firstBuild'); + const PartialStackFrame componentElementMount = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'ComponentElement', method: 'mount'); + const PartialStackFrame statefulElementFristBuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'StatefulElement', method: '_firstBuild'); + const PartialStackFrame singleChildMount = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'SingleChildRenderObjectElement', method: 'mount'); + + const String replacementString = '... Normal element mounting'; + + // ComponentElement variations + FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter( + frames: <PartialStackFrame>[ + elementInflateWidget, + elementUpdateChild, + componentElementPerformRebuild, + elementRebuild, + componentElementFristBuild, + componentElementMount, + ], + replacement: replacementString, + )); + FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter( + frames: <PartialStackFrame>[ + elementUpdateChild, + componentElementPerformRebuild, + elementRebuild, + componentElementFristBuild, + componentElementMount, + ], + replacement: replacementString, + )); + + // StatefulElement variations + FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter( + frames: <PartialStackFrame>[ + elementInflateWidget, + elementUpdateChild, + componentElementPerformRebuild, + elementRebuild, + componentElementFristBuild, + statefulElementFristBuild, + componentElementMount, + ], + replacement: replacementString, + )); + FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter( + frames: <PartialStackFrame>[ + elementUpdateChild, + componentElementPerformRebuild, + elementRebuild, + componentElementFristBuild, + statefulElementFristBuild, + componentElementMount, + ], + replacement: replacementString, + )); + + // SingleChildRenderObjectElement variations + FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter( + frames: <PartialStackFrame>[ + elementInflateWidget, + elementUpdateChild, + singleChildMount, + ], + replacement: replacementString, + )); + FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter( + frames: <PartialStackFrame>[ + elementUpdateChild, + singleChildMount, + ], + replacement: replacementString, + )); + } + /// The current [WidgetsBinding], if one has been created. /// /// If you need the binding to be constructed before calling [runApp],
diff --git a/packages/flutter/test/foundation/error_reporting_test.dart b/packages/flutter/test/foundation/error_reporting_test.dart index 42d5eab..07bb10f 100644 --- a/packages/flutter/test/foundation/error_reporting_test.dart +++ b/packages/flutter/test/foundation/error_reporting_test.dart
@@ -81,7 +81,7 @@ '#2 getSampleStack \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n' '#3 main \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n' '(.+\n)+' // TODO(ianh): when fixing #4021, also filter out frames from the test infrastructure below the first call to our main() - '\\(elided [0-9]+ frames from package dart:async\\)\n' + '\\(elided [0-9]+ frames from class _RawReceivePortImpl and dart:async\\)\n' '\n' 'line 1 of extra information\n' 'line 2 of extra information\n' @@ -153,7 +153,7 @@ '#2 getSampleStack \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n' '#3 main \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n' '(.+\n)+' // TODO(ianh): when fixing #4021, also filter out frames from the test infrastructure below the first call to our main() - '\\(elided [0-9]+ frames from package dart:async\\)\n' + '\\(elided [0-9]+ frames from class _RawReceivePortImpl and dart:async\\)\n' '\n' 'line 1 of extra information\n' 'line 2 of extra information\n'
diff --git a/packages/flutter/test/foundation/stack_frame_test.dart b/packages/flutter/test/foundation/stack_frame_test.dart index 7fc7724..5429878 100644 --- a/packages/flutter/test/foundation/stack_frame_test.dart +++ b/packages/flutter/test/foundation/stack_frame_test.dart
@@ -71,7 +71,8 @@ }, skip: isBrowser); } -const String stackString = '''#0 _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:42:39) +const String stackString = ''' +#0 _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:42:39) #1 _AssertionError._throwNew (dart:core-patch/errors_patch.dart:38:5) #2 new Text (package:flutter/src/widgets/text.dart:287:10) #3 _MyHomePageState.build (package:hello_flutter/main.dart:72:16) @@ -104,7 +105,8 @@ ]; -const String asyncStackString = '''#0 getSampleStack.<anonymous closure> (file:///path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart:40:57) +const String asyncStackString = ''' +#0 getSampleStack.<anonymous closure> (file:///path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart:40:57) #1 new Future.sync (dart:async/future.dart:224:31) #2 getSampleStack (file:///path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart:40:10) #3 main (file:///path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart:46:40) @@ -190,7 +192,8 @@ StackFrame(number: 38, className: '_RawReceivePortImpl', method: '_handleMessage', packageScheme: 'dart', package: 'isolate-patch', packagePath: 'isolate_patch.dart', line: 174, column: 12, source: '#38 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:174:12)'), ]; -const String stackFrameNoCols = '''#0 blah (package:assertions/main.dart:4) +const String stackFrameNoCols = ''' +#0 blah (package:assertions/main.dart:4) #1 main (package:assertions/main.dart:8) #2 _runMainZoned.<anonymous closure>.<anonymous closure> (dart:ui/hooks.dart:239) #3 _rootRun (dart:async/zone.dart:1126) @@ -214,7 +217,8 @@ StackFrame(number: 9, className: '_RawReceivePortImpl', method: '_handleMessage', packageScheme: 'dart', package: 'isolate-patch', packagePath: 'isolate-patch.dart', line: 174, column: -1, source: '#9 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:174)'), ]; -const String webStackTrace = r'''package:dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 196:49 throw_ +const String webStackTrace = r''' +package:dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 196:49 throw_ package:assertions/main.dart 4:3 blah package:assertions/main.dart 8:5 main$ package:assertions/main_web_entrypoint.dart 9:3 main$
diff --git a/packages/flutter/test/foundation/stack_trace_test.dart b/packages/flutter/test/foundation/stack_trace_test.dart index 77b102f..9099668 100644 --- a/packages/flutter/test/foundation/stack_trace_test.dart +++ b/packages/flutter/test/foundation/stack_trace_test.dart
@@ -16,13 +16,13 @@ expect(filtered[0], matches(r'^#0 +main\.<anonymous closure> \(.*stack_trace_test\.dart:[0-9]+:[0-9]+\)$')); expect(filtered[1], matches(r'^#1 +Declarer\.test\.<anonymous closure>.<anonymous closure>.<anonymous closure> \(package:test_api/.+:[0-9]+:[0-9]+\)$')); expect(filtered[2], equals('<asynchronous suspension>')); - expect(filtered.last, matches(r'^\(elided [1-9][0-9]+ frames from package dart:async(, package dart:async-patch,)? and package stack_trace\)$')); + expect(filtered.last, matches(r'^\(elided [1-9][0-9]+ frames from class _RawReceivePortImpl, class _Timer, dart:async(, dart:async-patch,)? and package:stack_trace\)$')); }); test('FlutterError.defaultStackFilter (async test body)', () async { final List<String> filtered = FlutterError.defaultStackFilter(StackTrace.current.toString().trimRight().split('\n')).toList(); expect(filtered.length, greaterThanOrEqualTo(3)); expect(filtered[0], matches(r'^#0 +main\.<anonymous closure> \(.*stack_trace_test\.dart:[0-9]+:[0-9]+\)$')); - expect(filtered.last, matches(r'^\(elided [1-9][0-9]+ frames from package dart:async(, package dart:async-patch,)? and package stack_trace\)$')); + expect(filtered.last, matches(r'^\(elided [1-9][0-9]+ frames from class _RawReceivePortImpl, class _Timer, dart:async(, dart:async-patch,)? and package:stack_trace\)$')); }); }
diff --git a/packages/flutter/test/widgets/binding_test.dart b/packages/flutter/test/widgets/binding_test.dart index 362a380..c769636 100644 --- a/packages/flutter/test/widgets/binding_test.dart +++ b/packages/flutter/test/widgets/binding_test.dart
@@ -2,8 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:typed_data'; +import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; @@ -137,6 +139,12 @@ tester.binding.scheduleWarmUpFrame(); // this actually tests flutter_test's implementation expect(tester.binding.hasScheduledFrame, isFalse); expect(frameCount, 1); + + // Get the tester back to a resumed state for subsequent tests. + message = const StringCodec().encodeMessage('AppLifecycleState.resumed'); + await defaultBinaryMessenger.handlePlatformMessage('flutter/lifecycle', message, (_) { }); + expect(tester.binding.hasScheduledFrame, isTrue); + await tester.pump(); }); testWidgets('scheduleFrameCallback error control test', (WidgetTester tester) async { @@ -170,4 +178,34 @@ ' argument.\n' ); }); + + testWidgets('defaultStackFilter elides framework Element mounting stacks', (WidgetTester tester) async { + final FlutterExceptionHandler oldHandler = FlutterError.onError; + String filteredStack; + FlutterError.onError = (FlutterErrorDetails details) { + expect(details.exception, isAssertionError); + expect(filteredStack, isNull); + filteredStack = details.toString(); + }; + await tester.pumpWidget(Directionality( + textDirection: TextDirection.ltr, + child: FocusableActionDetector( + child: Builder( + builder: (BuildContext context) { + return Opacity( + opacity: .5, + child: Builder( + builder: (BuildContext context) => Text(null), + ), + ); + }, + ), + ), + )); + // We don't elide the root or the last element. + FlutterError.onError = oldHandler; + expect(tester.allElements.length, 10); + print(filteredStack); + expect(filteredStack, contains('... Normal element mounting (42 frames)')); + }); }