[api] Make v8::Promise::Catch call the built-in Promise#then

Bug: chromium:1157692
Bug: chromium:1157386
Change-Id: I3525c5ea648bca6c2fb03bb910dbe9d673996da7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2587603
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71743}

src/api/api.cc

@@ -7381,10 +7381,14 @@ MaybeLocal<Promise> Promise::Catch(Local<Context> context,
                                    Local<Function> handler) {
   PREPARE_FOR_EXECUTION(context, Promise, Catch, Promise);
   auto self = Utils::OpenHandle(this);
-  i::Handle<i::Object> argv[] = {Utils::OpenHandle(*handler)};
+  i::Handle<i::Object> argv[] = {isolate->factory()->undefined_value(),
+                                 Utils::OpenHandle(*handler)};
   i::Handle<i::Object> result;
+  // Do not call the built-in Promise.prototype.catch!
+  // v8::Promise should not call out to a monkeypatched Promise.prototype.then
+  // as the implementation of Promise.prototype.catch does.
   has_pending_exception =
-      !i::Execution::CallBuiltin(isolate, isolate->promise_catch(), self,
+      !i::Execution::CallBuiltin(isolate, isolate->promise_then(), self,
                                  arraysize(argv), argv)
            .ToHandle(&result);
   RETURN_ON_FAILED_EXECUTION(Promise);

src/init/bootstrapper.cc

@@ -2424,10 +2424,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         isolate_, prototype, "then", Builtins::kPromisePrototypeThen, 2, true);
     native_context()->set_promise_then(*promise_then);
 
-    Handle<JSFunction> promise_catch =
     InstallFunctionWithBuiltinId(isolate_, prototype, "catch",
                                  Builtins::kPromisePrototypeCatch, 1, true);
-    native_context()->set_promise_catch(*promise_catch);
 
     InstallFunctionWithBuiltinId(isolate_, prototype, "finally",
                                  Builtins::kPromisePrototypeFinally, 1, true);

src/objects/contexts.h

@@ -316,7 +316,6 @@ enum ContextLookupFlags {
   V(OBJECT_VALUE_OF_FUNCTION_INDEX, JSFunction, object_value_of_function)      \
   V(PROMISE_ALL_INDEX, JSFunction, promise_all)                                \
   V(PROMISE_ANY_INDEX, JSFunction, promise_any)                                \
-  V(PROMISE_CATCH_INDEX, JSFunction, promise_catch)                            \
   V(PROMISE_FUNCTION_INDEX, JSFunction, promise_function)                      \
   V(RANGE_ERROR_FUNCTION_INDEX, JSFunction, range_error_function)              \
   V(REFERENCE_ERROR_FUNCTION_INDEX, JSFunction, reference_error_function)      \

test/cctest/test-api.cc

@@ -22950,6 +22950,34 @@ TEST(PromiseThen2) {
                     .FromJust());
 }
 
+TEST(PromiseCatchCallsBuiltin) {
+  LocalContext context;
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::HandleScope scope(isolate);
+  Local<Object> global = context->Global();
+
+  v8::Local<v8::Promise::Resolver> resolver =
+      v8::Promise::Resolver::New(context.local()).ToLocalChecked();
+  v8::Local<v8::Promise> promise = resolver->GetPromise();
+
+  resolver->Reject(context.local(), v8::Integer::New(isolate, 1)).FromJust();
+
+  CompileRun(
+      "var x1 = 0;\n"
+      "function f(x) { x1 = x; }\n"
+      "Promise.prototype.then = function () { throw 'unreachable'; };\n");
+  Local<Function> f = Local<Function>::Cast(
+      global->Get(context.local(), v8_str("f")).ToLocalChecked());
+
+  // Catch should not call monkey-patched Promise.prototype.then.
+  promise->Catch(context.local(), f).ToLocalChecked();
+  isolate->PerformMicrotaskCheckpoint();
+  CHECK_EQ(1, global->Get(context.local(), v8_str("x1"))
+                  .ToLocalChecked()
+                  ->Int32Value(context.local())
+                  .FromJust());
+}
+
 TEST(PromiseStateAndValue) {
   LocalContext context;
   v8::Isolate* isolate = context->GetIsolate();