[runtime] Fix Promise.all context promise hooks

camillobruni · V8 LUCI CQ · commit fa4cb172cde2 · 2021-05-03T13:23:06.000Z
We have to take the slow path in Promise.all if context promise hooks are set. The fast-path doesn't create intermediate promises by default. Bug: chromium:1204132, v8:11025 Change-Id: Ide92de00a4f6df05e0ddbc8814f6673bd667f426 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2866771 Reviewed-by: Victor Gomes <victorgomes@chromium.org> Commit-Queue: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/master@{#74326}
diff --git a/src/builtins/promise-all.tq b/src/builtins/promise-all.tq
@@ -231,8 +231,7 @@ Reject(Object) {
       // the PromiseReaction (aka we can pass undefined to
       // PerformPromiseThen), since this is only necessary for DevTools and
       // PromiseHooks.
-      if (promiseResolveFunction != Undefined ||
-          IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() ||
+      if (promiseResolveFunction != Undefined || NeedsAnyPromiseHooks() ||
           IsPromiseSpeciesProtectorCellInvalid() || Is<Smi>(nextValue) ||
           !IsPromiseThenLookupChainIntact(
               nativeContext, UnsafeCast<HeapObject>(nextValue).map)) {
diff --git a/test/mjsunit/promise-hooks.js b/test/mjsunit/promise-hooks.js
@@ -246,7 +246,7 @@ exceptions();
 (function regress1126309() {
   function __f_16(test) {
     test();
-    d8.promise.setHooks( undefined, () => {});
+    d8.promise.setHooks(undefined, () => {});
     %PerformMicrotaskCheckpoint();
     d8.promise.setHooks();
   }
@@ -262,3 +262,14 @@ exceptions();
   %PerformMicrotaskCheckpoint();
   d8.promise.setHooks();
 })();
+
+
+(function promiseAll() {
+  let initCount = 0;
+  d8.promise.setHooks(() => { initCount++});
+  Promise.all([Promise.resolve(1)]);
+  %PerformMicrotaskCheckpoint();
+  assertEquals(initCount, 3);
+
+  d8.promise.setHooks();
+})();
