From 40bb1684a228f4332a2a6e19002ff7e2590ff840 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Thu, 6 Jun 2019 11:18:36 -0700 Subject: [PATCH] Get the constructor's resolve function only once in Promise.all --- lib/Runtime/Library/JavascriptPromise.cpp | 49 +++++---- test/es6/ES6PromiseAsync.baseline | 14 ++- test/es6/ES6PromiseAsync.js | 115 +++++++++++++++++----- 3 files changed, 125 insertions(+), 53 deletions(-) diff --git a/lib/Runtime/Library/JavascriptPromise.cpp b/lib/Runtime/Library/JavascriptPromise.cpp index 6a327b5a480..1f82af764df 100644 --- a/lib/Runtime/Library/JavascriptPromise.cpp +++ b/lib/Runtime/Library/JavascriptPromise.cpp @@ -190,19 +190,18 @@ namespace Js { // 4. Let iterator be GetIterator(iterable). RecyclableObject* iterator = JavascriptOperators::GetIterator(iterable, scriptContext); - values = library->CreateArray(0); - JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next) + Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext); + if (!JavascriptConversion::IsCallable(resolveVar)) { - Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext); - - if (!JavascriptConversion::IsCallable(resolveVar)) - { - JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); - } + JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); + } - RecyclableObject* resolveFunc = VarTo(resolveVar); + RecyclableObject* resolveFunc = VarTo(resolveVar); + values = library->CreateArray(0); + JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next) + { ThreadContext * threadContext = scriptContext->GetThreadContext(); Var nextPromise = nullptr; BEGIN_SAFE_REENTRANT_CALL(threadContext) @@ -324,18 +323,17 @@ namespace Js RecyclableObject* iterator = JavascriptOperators::GetIterator(iterable, scriptContext); // Abstract operation PerformPromiseAllSettled - values = library->CreateArray(0); - JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next) + Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext); + if (!JavascriptConversion::IsCallable(resolveVar)) { - Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext); - - if (!JavascriptConversion::IsCallable(resolveVar)) - { - JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); - } + JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); + } - RecyclableObject* resolveFunc = VarTo(resolveVar); + RecyclableObject* resolveFunc = VarTo(resolveVar); + values = library->CreateArray(0); + JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next) + { ThreadContext* threadContext = scriptContext->GetThreadContext(); Var nextPromise = nullptr; BEGIN_SAFE_REENTRANT_CALL(threadContext) @@ -500,17 +498,16 @@ namespace Js // 4. Let iterator be GetIterator(iterable). RecyclableObject* iterator = JavascriptOperators::GetIterator(iterable, scriptContext); - JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next) + Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext); + if (!JavascriptConversion::IsCallable(resolveVar)) { - Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext); - - if (!JavascriptConversion::IsCallable(resolveVar)) - { - JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); - } + JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); + } - RecyclableObject* resolveFunc = VarTo(resolveVar); + RecyclableObject* resolveFunc = VarTo(resolveVar); + JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next) + { ThreadContext * threadContext = scriptContext->GetThreadContext(); Var nextPromise = nullptr; BEGIN_SAFE_REENTRANT_CALL(threadContext) diff --git a/test/es6/ES6PromiseAsync.baseline b/test/es6/ES6PromiseAsync.baseline index b0cb5008317..f9882383cf3 100644 --- a/test/es6/ES6PromiseAsync.baseline +++ b/test/es6/ES6PromiseAsync.baseline @@ -73,9 +73,21 @@ Executing test #68 - Promise.allSettled rejects immediately with an iterator tha Executing test #69 - Promise.allSettled resolve and reject functions cannot be called multiple times Test #69 - Temp then handler called from Promise.allSettled Executing test #70 - Promise.allSettled passes all elements of iterable to Promise.resolve -Executing test #71 - Promise.allsettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler +Executing test #71 - Promise.allSettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler Test #71 - resolve called Test #71 - reject called: oops +Executing test #72 - Promise.allSettled gets the constructor's resolve function only once +Test #72 - get constructor resolve +Test #72 - constructor resolve called +Test #72 - constructor resolve called +Executing test #73 - Promise.all gets the constructor's resolve function only once +Test #73 - get constructor resolve +Test #73 - constructor resolve called +Test #73 - constructor resolve called +Executing test #74 - Promise.race gets the constructor's resolve function only once +Test #74 - get constructor resolve +Test #74 - constructor resolve called +Test #74 - constructor resolve called Completion Results: Test #1 - Success handler called with result = basic:success diff --git a/test/es6/ES6PromiseAsync.js b/test/es6/ES6PromiseAsync.js index 3959d777e2b..cc797f23965 100644 --- a/test/es6/ES6PromiseAsync.js +++ b/test/es6/ES6PromiseAsync.js @@ -882,20 +882,20 @@ var tests = [ ); } }, - { - name: "Promise.all called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler", - body: function (index) { - function FakePromise(fn) { - function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); } - function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) } - fn(resolve, reject); - this.then = function(onResolve, onReject) {}; - } + { + name: "Promise.all called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler", + body: function (index) { + function FakePromise(fn) { + function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); } + function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) } + fn(resolve, reject); + this.then = function(onResolve, onReject) {}; + } - FakePromise.resolve = function() {}; - Promise.all.call(FakePromise, []); - } - }, + FakePromise.resolve = function() {}; + Promise.all.call(FakePromise, []); + } + }, { name: "Promise.resolve called with a thenable calls then on the thenable", body: function (index) { @@ -1324,20 +1324,83 @@ var tests = [ e => echo(`Test #${index} - Failed - ${JSON.stringify(e)}`)); } }, - { - name: "Promise.allsettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler", - body: function (index) { - function FakePromise(fn) { - function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); } - function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) } - fn(resolve, reject); - this.then = function(onResolve, onReject) {}; - } + { + name: "Promise.allSettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler", + body: function (index) { + function FakePromise(fn) { + function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); } + function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) } + fn(resolve, reject); + this.then = function(onResolve, onReject) {}; + } + + FakePromise.resolve = function() {}; + Promise.allSettled.call(FakePromise, []); + } + }, + { + name: "Promise.allSettled gets the constructor's resolve function only once", + body: function(index) { + function FakePromise(fn) { + fn(function() {}, function() {}); + this.then = function(onResolve, onReject) {}; + } + + Object.defineProperty(FakePromise, 'resolve', { + get: function() { + echo(`Test #${index} - get constructor resolve`); + return function(x) { + echo(`Test #${index} - constructor resolve called`); + return Promise.resolve(x); + }; + } + }); + + Promise.allSettled.call(FakePromise, [1, 2]); + } + }, + { + name: "Promise.all gets the constructor's resolve function only once", + body: function(index) { + function FakePromise(fn) { + fn(function() {}, function() {}); + this.then = function(onResolve, onReject) {}; + } + + Object.defineProperty(FakePromise, 'resolve', { + get: function() { + echo(`Test #${index} - get constructor resolve`); + return function(x) { + echo(`Test #${index} - constructor resolve called`); + return Promise.resolve(x); + }; + } + }); + + Promise.all.call(FakePromise, [1, 2]); + } + }, + { + name: "Promise.race gets the constructor's resolve function only once", + body: function(index) { + function FakePromise(fn) { + fn(function() {}, function() {}); + this.then = function(onResolve, onReject) {}; + } + + Object.defineProperty(FakePromise, 'resolve', { + get: function() { + echo(`Test #${index} - get constructor resolve`); + return function(x) { + echo(`Test #${index} - constructor resolve called`); + return Promise.resolve(x); + }; + } + }); - FakePromise.resolve = function() {}; - Promise.allSettled.call(FakePromise, []); - } - }, + Promise.race.call(FakePromise, [1, 2]); + } + } ]; var index = 1;