Skip to content

Commit 40bb168

Browse files
Kevin SmithKevin Smith
Kevin Smith
authored and
Kevin Smith
committed
Get the constructor's resolve function only once in Promise.all
1 parent 76d3624 commit 40bb168

File tree

3 files changed

+125
-53
lines changed

3 files changed

+125
-53
lines changed

‎lib/Runtime/Library/JavascriptPromise.cpp

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -190,19 +190,18 @@ namespace Js
190190
{
191191
// 4. Let iterator be GetIterator(iterable).
192192
RecyclableObject* iterator = JavascriptOperators::GetIterator(iterable, scriptContext);
193-
values = library->CreateArray(0);
194193

195-
JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next)
194+
Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext);
195+
if (!JavascriptConversion::IsCallable(resolveVar))
196196
{
197-
Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext);
198-
199-
if (!JavascriptConversion::IsCallable(resolveVar))
200-
{
201-
JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction);
202-
}
197+
JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction);
198+
}
203199

204-
RecyclableObject* resolveFunc = VarTo<RecyclableObject>(resolveVar);
200+
RecyclableObject* resolveFunc = VarTo<RecyclableObject>(resolveVar);
201+
values = library->CreateArray(0);
205202

203+
JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next)
204+
{
206205
ThreadContext * threadContext = scriptContext->GetThreadContext();
207206
Var nextPromise = nullptr;
208207
BEGIN_SAFE_REENTRANT_CALL(threadContext)
@@ -324,18 +323,17 @@ namespace Js
324323
RecyclableObject* iterator = JavascriptOperators::GetIterator(iterable, scriptContext);
325324

326325
// Abstract operation PerformPromiseAllSettled
327-
values = library->CreateArray(0);
328-
JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next)
326+
Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext);
327+
if (!JavascriptConversion::IsCallable(resolveVar))
329328
{
330-
Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext);
331-
332-
if (!JavascriptConversion::IsCallable(resolveVar))
333-
{
334-
JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction);
335-
}
329+
JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction);
330+
}
336331

337-
RecyclableObject* resolveFunc = VarTo<RecyclableObject>(resolveVar);
332+
RecyclableObject* resolveFunc = VarTo<RecyclableObject>(resolveVar);
333+
values = library->CreateArray(0);
338334

335+
JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next)
336+
{
339337
ThreadContext* threadContext = scriptContext->GetThreadContext();
340338
Var nextPromise = nullptr;
341339
BEGIN_SAFE_REENTRANT_CALL(threadContext)
@@ -500,17 +498,16 @@ namespace Js
500498
// 4. Let iterator be GetIterator(iterable).
501499
RecyclableObject* iterator = JavascriptOperators::GetIterator(iterable, scriptContext);
502500

503-
JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next)
501+
Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext);
502+
if (!JavascriptConversion::IsCallable(resolveVar))
504503
{
505-
Var resolveVar = JavascriptOperators::GetProperty(constructorObject, Js::PropertyIds::resolve, scriptContext);
506-
507-
if (!JavascriptConversion::IsCallable(resolveVar))
508-
{
509-
JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction);
510-
}
504+
JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction);
505+
}
511506

512-
RecyclableObject* resolveFunc = VarTo<RecyclableObject>(resolveVar);
507+
RecyclableObject* resolveFunc = VarTo<RecyclableObject>(resolveVar);
513508

509+
JavascriptOperators::DoIteratorStepAndValue(iterator, scriptContext, [&](Var next)
510+
{
514511
ThreadContext * threadContext = scriptContext->GetThreadContext();
515512
Var nextPromise = nullptr;
516513
BEGIN_SAFE_REENTRANT_CALL(threadContext)

‎test/es6/ES6PromiseAsync.baseline

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,21 @@ Executing test #68 - Promise.allSettled rejects immediately with an iterator tha
7373
Executing test #69 - Promise.allSettled resolve and reject functions cannot be called multiple times
7474
Test #69 - Temp then handler called from Promise.allSettled
7575
Executing test #70 - Promise.allSettled passes all elements of iterable to Promise.resolve
76-
Executing test #71 - Promise.allsettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler
76+
Executing test #71 - Promise.allSettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler
7777
Test #71 - resolve called
7878
Test #71 - reject called: oops
79+
Executing test #72 - Promise.allSettled gets the constructor's resolve function only once
80+
Test #72 - get constructor resolve
81+
Test #72 - constructor resolve called
82+
Test #72 - constructor resolve called
83+
Executing test #73 - Promise.all gets the constructor's resolve function only once
84+
Test #73 - get constructor resolve
85+
Test #73 - constructor resolve called
86+
Test #73 - constructor resolve called
87+
Executing test #74 - Promise.race gets the constructor's resolve function only once
88+
Test #74 - get constructor resolve
89+
Test #74 - constructor resolve called
90+
Test #74 - constructor resolve called
7991

8092
Completion Results:
8193
Test #1 - Success handler called with result = basic:success

‎test/es6/ES6PromiseAsync.js

Lines changed: 89 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -882,20 +882,20 @@ var tests = [
882882
);
883883
}
884884
},
885-
{
886-
name: "Promise.all called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler",
887-
body: function (index) {
888-
function FakePromise(fn) {
889-
function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); }
890-
function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) }
891-
fn(resolve, reject);
892-
this.then = function(onResolve, onReject) {};
893-
}
885+
{
886+
name: "Promise.all called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler",
887+
body: function (index) {
888+
function FakePromise(fn) {
889+
function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); }
890+
function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) }
891+
fn(resolve, reject);
892+
this.then = function(onResolve, onReject) {};
893+
}
894894

895-
FakePromise.resolve = function() {};
896-
Promise.all.call(FakePromise, []);
897-
}
898-
},
895+
FakePromise.resolve = function() {};
896+
Promise.all.call(FakePromise, []);
897+
}
898+
},
899899
{
900900
name: "Promise.resolve called with a thenable calls then on the thenable",
901901
body: function (index) {
@@ -1324,20 +1324,83 @@ var tests = [
13241324
e => echo(`Test #${index} - Failed - ${JSON.stringify(e)}`));
13251325
}
13261326
},
1327-
{
1328-
name: "Promise.allsettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler",
1329-
body: function (index) {
1330-
function FakePromise(fn) {
1331-
function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); }
1332-
function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) }
1333-
fn(resolve, reject);
1334-
this.then = function(onResolve, onReject) {};
1335-
}
1327+
{
1328+
name: "Promise.allSettled called with empty iterator, calls Promise.resolve synchronously and passes abrupt completion to reject handler",
1329+
body: function (index) {
1330+
function FakePromise(fn) {
1331+
function resolve() { echo(`Test #${index} - resolve called`); throw new Error('oops'); }
1332+
function reject(e) { echo(`Test #${index} - reject called: ${e.message}`) }
1333+
fn(resolve, reject);
1334+
this.then = function(onResolve, onReject) {};
1335+
}
1336+
1337+
FakePromise.resolve = function() {};
1338+
Promise.allSettled.call(FakePromise, []);
1339+
}
1340+
},
1341+
{
1342+
name: "Promise.allSettled gets the constructor's resolve function only once",
1343+
body: function(index) {
1344+
function FakePromise(fn) {
1345+
fn(function() {}, function() {});
1346+
this.then = function(onResolve, onReject) {};
1347+
}
1348+
1349+
Object.defineProperty(FakePromise, 'resolve', {
1350+
get: function() {
1351+
echo(`Test #${index} - get constructor resolve`);
1352+
return function(x) {
1353+
echo(`Test #${index} - constructor resolve called`);
1354+
return Promise.resolve(x);
1355+
};
1356+
}
1357+
});
1358+
1359+
Promise.allSettled.call(FakePromise, [1, 2]);
1360+
}
1361+
},
1362+
{
1363+
name: "Promise.all gets the constructor's resolve function only once",
1364+
body: function(index) {
1365+
function FakePromise(fn) {
1366+
fn(function() {}, function() {});
1367+
this.then = function(onResolve, onReject) {};
1368+
}
1369+
1370+
Object.defineProperty(FakePromise, 'resolve', {
1371+
get: function() {
1372+
echo(`Test #${index} - get constructor resolve`);
1373+
return function(x) {
1374+
echo(`Test #${index} - constructor resolve called`);
1375+
return Promise.resolve(x);
1376+
};
1377+
}
1378+
});
1379+
1380+
Promise.all.call(FakePromise, [1, 2]);
1381+
}
1382+
},
1383+
{
1384+
name: "Promise.race gets the constructor's resolve function only once",
1385+
body: function(index) {
1386+
function FakePromise(fn) {
1387+
fn(function() {}, function() {});
1388+
this.then = function(onResolve, onReject) {};
1389+
}
1390+
1391+
Object.defineProperty(FakePromise, 'resolve', {
1392+
get: function() {
1393+
echo(`Test #${index} - get constructor resolve`);
1394+
return function(x) {
1395+
echo(`Test #${index} - constructor resolve called`);
1396+
return Promise.resolve(x);
1397+
};
1398+
}
1399+
});
13361400

1337-
FakePromise.resolve = function() {};
1338-
Promise.allSettled.call(FakePromise, []);
1339-
}
1340-
},
1401+
Promise.race.call(FakePromise, [1, 2]);
1402+
}
1403+
}
13411404
];
13421405

13431406
var index = 1;

0 commit comments

Comments
 (0)