Revert "Revert "Fix remaining holes in stack trace demangling (#60478)" (#60916)" (#60996)

This reverts commit 2c7e5dd93548d84dad61bf2de171cdb89fa3efc6.

packages/flutter/lib/src/foundation/assertions.dart

@@ -28,6 +28,14 @@ typedef DiagnosticPropertiesTransformer = Iterable<DiagnosticsNode> Function(Ite
 /// and other callbacks that collect information describing an error.
 typedef InformationCollector = Iterable<DiagnosticsNode> Function();
 
+/// Signature for a function that demangles [StackTrace] objects into a format
+/// that can be parsed by [StackFrame].
+///
+/// See also:
+///
+///   * [FlutterError.demangleStackTrace], which shows an example implementation.
+typedef StackTraceDemangler = StackTrace Function(StackTrace details);
+
 /// Partial information from a stack frame for stack filtering purposes.
 ///
 /// See also:
@@ -651,7 +659,7 @@ class FlutterErrorDetails with Diagnosticable {
         // If not: Error is in user code (user violated assertion in framework).
         // If so:  Error is in Framework. We either need an assertion higher up
         //         in the stack, or we've violated our own assertions.
-        final List<StackFrame> stackFrames = StackFrame.fromStackTrace(stack)
+        final List<StackFrame> stackFrames = StackFrame.fromStackTrace(FlutterError.demangleStackTrace(stack))
                                                        .skipWhile((StackFrame frame) => frame.packageScheme == 'dart')
                                                        .toList();
         final bool ourFault =  stackFrames.length >= 2
@@ -863,6 +871,31 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
   /// recommended.
   static FlutterExceptionHandler onError = (FlutterErrorDetails details) => presentError(details);
 
+  /// Called by the Flutter framework before attempting to parse a [StackTrace].
+  ///
+  /// Some [StackTrace] implementations have a different toString format from
+  /// what the framework expects, like ones from package:stack_trace. To make
+  /// sure we can still parse and filter mangled [StackTrace]s, the framework
+  /// first calls this function to demangle them.
+  ///
+  /// This should be set in any environment that could propagate a non-standard
+  /// stack trace to the framework. Otherwise, the default behavior is to assume
+  /// all stack traces are in a standard format.
+  ///
+  /// The following example demangles package:stack_trace traces by converting
+  /// them into vm traces, which the framework is able to parse:
+  ///
+  /// ```dart
+  /// FlutterError.demangleStackTrace = (StackTrace stackTrace) {
+  ///   if (stack is stack_trace.Trace)
+  //      return stack.vmTrace;
+  //    if (stack is stack_trace.Chain)
+  //      return stack.toTrace().vmTrace;
+  //    return stack;
+  /// };
+  /// ```
+  static StackTraceDemangler demangleStackTrace = (StackTrace stackTrace) => stackTrace;
+
   /// Called whenever the Flutter framework wants to present an error to the
   /// users.
   ///
@@ -1069,7 +1102,11 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
 void debugPrintStack({StackTrace stackTrace, String label, int maxFrames}) {
   if (label != null)
     debugPrint(label);
-  stackTrace ??= StackTrace.current;
+  if (stackTrace == null) {
+    stackTrace = StackTrace.current;
+  } else {
+    stackTrace = FlutterError.demangleStackTrace(stackTrace);
+  }
   Iterable<String> lines = stackTrace.toString().trimRight().split('\n');
   if (kIsWeb && lines.isNotEmpty) {
     // Remove extra call to StackTrace.current for web platform.
@@ -1105,11 +1142,7 @@ class DiagnosticsStackTrace extends DiagnosticsBlock {
   }) : super(
     name: name,
     value: stack,
-    properties: stack == null
+    properties: _applyStackFilter(stack, stackFilter),
-        ? <DiagnosticsNode>[]
-        : (stackFilter ?? FlutterError.defaultStackFilter)(stack.toString().trimRight().split('\n'))
-              .map<DiagnosticsNode>(_createStackFrame)
-              .toList(),
     style: DiagnosticsTreeStyle.flat,
     showSeparator: showSeparator,
     allowTruncate: true,
@@ -1127,6 +1160,17 @@ class DiagnosticsStackTrace extends DiagnosticsBlock {
     showSeparator: showSeparator,
   );
 
+  static List<DiagnosticsNode> _applyStackFilter(
+    StackTrace stack,
+    IterableFilter<String> stackFilter,
+  ) {
+    if (stack == null)
+      return <DiagnosticsNode>[];
+    final IterableFilter<String> filter = stackFilter ?? FlutterError.defaultStackFilter;
+    final Iterable<String> frames = filter('${FlutterError.demangleStackTrace(stack)}'.trimRight().split('\n'));
+    return frames.map<DiagnosticsNode>(_createStackFrame).toList();
+  }
+
   static DiagnosticsNode _createStackFrame(String frame) {
     return DiagnosticsNode.message(frame, allowWrap: false);
   }

packages/flutter/lib/src/foundation/stack_frame.dart

@@ -190,6 +190,13 @@ class StackFrame {
       return stackOverFlowElision;
     }
 
+    assert(
+      line != '===== asynchronous gap ===========================',
+      'Got a stack frame from package:stack_trace, where a vm or web frame was expected. '
+      'This can happen if FlutterError.demangleStackTrace was not set in an environment '
+      'that propagates non-standard stack traces to the framework, such as during tests.'
+    );
+
     // Web frames.
     if (!line.startsWith('#')) {
       return _parseWebFrame(line);

packages/flutter/lib/src/scheduler/binding.dart

@@ -621,7 +621,9 @@ mixin SchedulerBinding on BindingBase {
         debugPrint('When the current transient callback was registered, this was the stack:');
         debugPrint(
           FlutterError.defaultStackFilter(
-            _FrameCallbackEntry.debugCurrentCallbackStack.toString().trimRight().split('\n')
+            FlutterError.demangleStackTrace(
+              _FrameCallbackEntry.debugCurrentCallbackStack,
+            ).toString().trimRight().split('\n')
           ).join('\n')
         );
       } else {

packages/flutter/test/foundation/stack_frame_test.dart

@@ -73,6 +73,16 @@ void main() {
   }, skip: isBrowser); // The VM test harness can handle a stack overflow, but
   // the browser cannot - running this test in a browser will cause it to become
   // unresponsive.
+
+  test('Traces from package:stack_trace throw assertion', () {
+    try {
+      StackFrame.fromStackString(mangledStackString);
+      assert(false, 'StackFrame.fromStackString did not throw on a mangled stack trace');
+    } catch (e) {
+      expect(e, isA<AssertionError>());
+      expect('$e', contains('Got a stack frame from package:stack_trace'));
+    }
+  });
 }
 
 const String stackString = '''
@@ -152,6 +162,21 @@ const String asyncStackString = '''
 #37     _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:307:19)
 #38     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:174:12)''';
 
+const String mangledStackString = '''
+dart:async/future_impl.dart 23:44                              _Completer.completeError
+test\\bindings_async_gap_test.dart 42:17                        main.<fn>.<fn>
+package:flutter_test/src/binding.dart 744:19                   TestWidgetsFlutterBinding._runTestBody
+===== asynchronous gap ===========================
+dart:async/zone.dart 1121:19                                   _CustomZone.registerUnaryCallback
+dart:async-patch/async_patch.dart 83:23                        _asyncThenWrapperHelper
+dart:async/zone.dart 1222:13                                   _rootRunBinary
+dart:async/zone.dart 1107:19                                   _CustomZone.runBinary
+package:flutter_test/src/binding.dart 724:14                   TestWidgetsFlutterBinding._runTest
+package:flutter_test/src/binding.dart 1124:24                  AutomatedTestWidgetsFlutterBinding.runTest.<fn>
+package:fake_async/fake_async.dart 177:54                      FakeAsync.run.<fn>.<fn>
+dart:async/zone.dart 1190:13                                   _rootRun
+''';
+
 const List<StackFrame> asyncStackFrames = <StackFrame>[
   StackFrame(number: 0,  className: '',                    method: 'getSampleStack', packageScheme: 'file',    package: '<unknown>',       packagePath: '/path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart', line: 40,   column: 57, source: '#0      getSampleStack.<anonymous closure> (file:///path/to/flutter/packages/flutter/test/foundation/error_reporting_test.dart:40:57)'),
   StackFrame(number: 1,  className: 'Future',              method: 'sync',           packageScheme: 'dart',    package: 'async',           packagePath: 'future.dart',                                                                 line: 224,  column: 31, isConstructor: true, source: '#1      new Future.sync (dart:async/future.dart:224:31)'),

packages/flutter_test/lib/src/binding.dart

@@ -516,6 +516,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
     return result;
   }
   FlutterExceptionHandler _oldExceptionHandler;
+  StackTraceDemangler _oldStackTraceDemangler;
   FlutterErrorDetails _pendingExceptionDetails;
 
   static const TextStyle _messageStyle = TextStyle(
@@ -579,10 +580,6 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
       // our main future completing.
       assert(Zone.current == _parentZone);
       if (_pendingExceptionDetails != null) {
-        assert(
-          _unmangle(_pendingExceptionDetails.stack) == _pendingExceptionDetails.stack,
-          'The test binding presented an unmangled stack trace to the framework.',
-        );
         debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the error!
         reportTestException(_pendingExceptionDetails, testDescription);
         _pendingExceptionDetails = null;
@@ -613,9 +610,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
     assert(description != null);
     assert(inTest);
     _oldExceptionHandler = FlutterError.onError;
+    _oldStackTraceDemangler = FlutterError.demangleStackTrace;
     int _exceptionCount = 0; // number of un-taken exceptions
     FlutterError.onError = (FlutterErrorDetails details) {
-      details = details.copyWith(stack: _unmangle(details.stack));
       if (_pendingExceptionDetails != null) {
         debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the errors!
         if (_exceptionCount == 0) {
@@ -634,6 +631,17 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
         _pendingExceptionDetails = details;
       }
     };
+    FlutterError.demangleStackTrace = (StackTrace stack) {
+      // package:stack_trace uses ZoneSpecification.errorCallback to add useful
+      // information to stack traces, in this case the Trace and Chain classes
+      // can be present. Because these StackTrace implementations do not follow
+      // the format the framework expects, we covert them to a vm trace here.
+      if (stack is stack_trace.Trace)
+        return stack.vmTrace;
+      if (stack is stack_trace.Chain)
+        return stack.toTrace().vmTrace;
+      return stack;
+    };
     final Completer<void> testCompleter = Completer<void>();
     final VoidCallback testCompletionHandler = _createTestCompletionHandler(description, testCompleter);
     void handleUncaughtError(dynamic exception, StackTrace stack) {
@@ -647,7 +655,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
         debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the error!
         FlutterError.dumpErrorToConsole(FlutterErrorDetails(
           exception: exception,
-          stack: _unmangle(stack),
+          stack: stack,
           context: ErrorDescription('running a test (but after the test had completed)'),
           library: 'Flutter test framework',
         ), forceReport: true);
@@ -694,7 +702,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
       final int stackLinesToOmit = reportExpectCall(stack, omittedFrames);
       FlutterError.reportError(FlutterErrorDetails(
         exception: exception,
-        stack: _unmangle(stack),
+        stack: stack,
         context: ErrorDescription('running a test'),
         library: 'Flutter test framework',
         stackFilter: (Iterable<String> frames) {
@@ -842,6 +850,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
   void postTest() {
     assert(inTest);
     FlutterError.onError = _oldExceptionHandler;
+    FlutterError.demangleStackTrace = _oldStackTraceDemangler;
     _pendingExceptionDetails = null;
     _parentZone = null;
     buildOwner.focusManager = FocusManager();
@@ -1713,11 +1722,3 @@ class _LiveTestRenderView extends RenderView {
     _label?.paint(context.canvas, offset - const Offset(0.0, 10.0));
   }
 }
-
-StackTrace _unmangle(StackTrace stack) {
-  if (stack is stack_trace.Trace)
-    return stack.vmTrace;
-  if (stack is stack_trace.Chain)
-    return stack.toTrace().vmTrace;
-  return stack;
-}

packages/flutter_test/test/bindings_async_gap_test.dart

@@ -49,4 +49,4 @@ Future<void> main() async {
 
 class CustomException implements Exception {
   const CustomException();
-}
+}

packages/flutter_test/test/demangle_test.dart

@@ -0,0 +1,73 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:flutter/foundation.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:stack_trace/stack_trace.dart' as stack_trace;
+
+Future<void> main() async {
+  // We use AutomatedTestWidgetsFlutterBinding to allow the test binding to set
+  // FlutterError.demangleStackTrace and FlutterError.onError without testWidgets.
+  final AutomatedTestWidgetsFlutterBinding binding = AutomatedTestWidgetsFlutterBinding();
+
+  test('FlutterErrorDetails demangles', () async {
+    await binding.runTest(() async {
+      // When we call toString on a FlutterErrorDetails, it attempts to parse and
+      // filter the stack trace, which fails if demangleStackTrace returns a
+      // mangled stack trace.
+      FlutterErrorDetails(
+        exception: const CustomException(),
+        stack: await getMangledStack(),
+      ).toString();
+
+      // Additional logic is used to parse assertion stack traces.
+      FlutterErrorDetails(
+        exception: AssertionError('Some assertion'),
+        stack: await getMangledStack(),
+      ).toString();
+    }, () {});
+    binding.postTest();
+  });
+
+  test('debugPrintStack demangles', () async {
+    await binding.runTest(() async {
+      final DebugPrintCallback oldDebugPrint = debugPrint;
+      try {
+        debugPrint = (String message, {int wrapWidth}) {};
+        debugPrintStack(
+          stackTrace: await getMangledStack(),
+        );
+      } finally {
+        debugPrint = oldDebugPrint;
+      }
+    }, () {});
+    binding.postTest();
+  });
+}
+
+Future<StackTrace> getMangledStack() {
+  // package:test uses package:stack_trace to wrap tests in a Zone that overrides
+  // errorCallback, the error callback transforms any StackTrace propagated
+  // to futures into a Chain, which has a format different from the vm.
+  final Completer<StackTrace> stackCompleter = Completer<StackTrace>();
+  final Completer<void> completer = Completer<void>();
+  completer.future.then(
+    (void value) {
+      assert(false);
+    },
+    onError: (Object error, StackTrace stack) {
+      expect(error, isA<CustomException>());
+      expect(stack, isA<stack_trace.Chain>());
+      stackCompleter.complete(stack);
+    },
+  );
+  completer.completeError(const CustomException());
+  return stackCompleter.future;
+}
+
+class CustomException implements Exception {
+  const CustomException();
+}