Revert "[vm] Treat Future.then(..., onError:...) as catch all handler"

This reverts commit 38e0046cad.

Reason for revert: https://github.com/flutter/flutter/issues/133677

Original change's description:
> [vm] Treat Future.then(..., onError:...) as catch all handler
>
> Commit a52f2b9 which reworked awaiter stack unwinding and its
> integration with debugger introduced the following regression:
> it stopped treating `Future` listeners which had both `onValue`
> and `onError` callbacks as catch all exception handlers. Only
> listners with `onError` callback (those created with
> `Future.onError`) were treated as a catch all handler.
>
> This meant that debugger started to treat exceptions in the
> code below as uncaught:
>
> ```
> Future<void> foo() {
>   await 0;
>   throw '';
> }
>
> await foo().then(..., onError: (e, st) {
>
> });
> ```
>
> This change fixes this regression by checking if
> `FutureListener.state & stateCatchError != 0` instead of
> more narrow `FutureListener.state == stateCatchError` which
> only detects listeners which only catch errors but do not
> handle values. The new predicate matches
> `FutureListener.handlesError`.
>
> Fixes https://github.com/dart-lang/sdk/issues/53334
>
> TEST=service/pause_on_unhandled_async_exceptions_test
>
> Fixed: 53334
> Change-Id: I374114b7a7b2ef86b7ed6bf7d5e7cf71ba6054dc
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323280
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Slava Egorov <vegorov@google.com>

Change-Id: Id20a6929aac93511173c7467caee91e2488696ed
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323429
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>

runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart

@@ -12,49 +12,31 @@ import 'service_test_common.dart';
 //
 // Update these constants by running:
 //
-// dart runtime/observatory/tests/service/update_line_numbers.dart <test.dart>
+//     dart runtime/observatory/tests/service/test_helper.dart update-lines
 //
-const int LINE_A = 59;
+const int LINE_A = 41;
 // AUTOGENERATED END
 
 class Foo {}
 
-String doThrow() {
+doThrow() {
   throw "TheException";
   return "end of doThrow";
 }
 
-Future<void> asyncThrower() async {
+asyncThrower() async {
   await 0; // force async gap
   doThrow();
 }
 
 testeeMain() async {
   try {
-    // This is a regression case for https://dartbug.com/53334:
-    // we should recognize `then(..., onError: ...)` as a catch
-    // all exception handler.
-    await asyncThrower().then((v) => v, onError: (e, st) {
-      // Caught and ignored.
-    });
-
-    await asyncThrower().onError((error, stackTrace) {
-      // Caught and ignored.
-    });
-
+    // caught.
     try {
       await asyncThrower();
-    } on String catch (e) {
-      // Caught and ignored.
-    }
+    } catch (e) {}
 
-    try {
-      await asyncThrower();
-    } catch (e) {
-      // Caught and ignored.
-    }
-
-    // This does not catch the exception.
+    // uncaught.
     try {
       await asyncThrower(); // LINE_A.
     } on double catch (e) {}
@@ -68,15 +50,11 @@ var tests = <IsolateTest>[
     var stack = await isolate.getStack();
     expect(stack['asyncCausalFrames'], isNotNull);
     var asyncStack = stack['asyncCausalFrames'];
-    expect(asyncStack.length, greaterThanOrEqualTo(4));
-    await expectFrame(asyncStack[0], functionName: 'doThrow');
-    await expectFrame(asyncStack[1], functionName: 'asyncThrower');
-    await expectFrame(asyncStack[2], kind: M.FrameKind.asyncSuspensionMarker);
-    await expectFrame(asyncStack[3],
-        kind: M.FrameKind.asyncCausal,
-        functionName: 'testeeMain',
-        line: LINE_A);
-  },
+    expect(asyncStack.length, greaterThanOrEqualTo(2));
+    expect(asyncStack[0].toString(), contains('doThrow'));
+    expect(asyncStack[1].toString(), contains('asyncThrower'));
+    // There was no await'er for "doThrow()".
+  }
 ];
 
 main(args) => runIsolateTests(args, tests,

runtime/observatory/tests/service/service_test_common.dart

@@ -270,21 +270,6 @@ IsolateTest setBreakpointAtUriAndLine(String uri, int line) {
   };
 }
 
-Future<void> expectFrame(Frame frame,
-    {M.FrameKind kind = M.FrameKind.regular,
-    String? functionName,
-    int? line}) async {
-  expect(frame.kind, equals(kind));
-  if (functionName != null) {
-    expect(frame.function?.name, equals(functionName));
-  }
-  if (line != null) {
-    final script = await frame.location!.script.load() as Script;
-    expect(frame.location, isNotNull);
-    expect(script.tokenToLine(frame.location!.tokenPos), equals(line));
-  }
-}
-
 IsolateTest stoppedAtLine(int line) {
   return (Isolate isolate) async {
     print("Checking we are at line $line");

runtime/vm/stack_trace.cc

@@ -393,17 +393,7 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
   }
   awaiter_frame_.closure =
       Closure::RawCast(Get_FutureListener_callback(listener));
-
-  // If the Future has catchError callback attached through either
-  // `Future.onError` or `Future.then(..., onError: ...)` then we should
-  // treat this listener as a catch all exception handler. However we should
-  // ignore the case when these callbacks are forwarding errors into a
-  // suspended async function, because it will be represented by its own async
-  // frame.
-  if ((state & k_FutureListener_stateCatchError) != 0 &&
-      (awaiter_frame_.closure.IsNull() ||
-       !StackTraceUtils::GetSuspendState(awaiter_frame_.closure,
-                                         &suspend_state_))) {
+  if (state == k_FutureListener_stateCatchError) {
     awaiter_frame_.has_catch_error = true;
   }
 }