Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1531073004:
[promise] Make Promise.reject match spec, and validate promise capabilities (Closed)
|
Created:
5 years ago by caitp (gmail) Modified:
4 years, 11 months ago CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[promise] Make Promise.reject match spec, and validate promise capabilities
Correctly validate promise capabilities in NewPromiseCapabilities() and in
GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and
similar cases in the spec, rather than passing tests asserting these steps
are taken in NewPromiseCapability
Also changes Promise.reject to match specification.
Fixes the following test262 tests:
- built-ins/Promise/all/capability-executor-called-twice.js
- built-ins/Promise/all/capability-executor-not-callable.js
- built-ins/Promise/prototype/then/capability-executor-called-twice.js
- built-ins/Promise/prototype/then/capability-executor-not-callable.js
- built-ins/Promise/reject/capability-executor-called-twice.js
- built-ins/Promise/reject/capability-executor-not-callable.js
- built-ins/Promise/resolve/capability-executor-called-twice.js
- built-ins/Promise/resolve/capability-executor-not-callable.js
- built-ins/Promise/race/capability-executor-called-twice.js
- built-ins/Promise/race/capability-executor-not-callable.js
- built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js
- built-ins/Promise/race/S25.4.4.3_A3.1_T2.js
Per v8:3641, mjsunit/es6/debug-promises/throw-with-undefined-reject.js becomes invalid. The exception is thrown before the chain handler is ever invoked, and is caught externally by d8's own handler --- thus evading the uncaught exception event.
BUG=v8:4633, v8:4631, v8:4243, v8:3641
LOG=N
R=littledan@chromium.org, cbruni@chromium.org
Committed: https://crrev.com/e4af5cdbf9ae081c6b69261ea7d7addf95e1e524
Cr-Commit-Position: refs/heads/master@{#33128}
Patch Set 1 : #Patch Set 2 : Minor adjustments + test262 status fixes #Patch Set 3 : Delete invalid test + add ignition skips #Patch Set 4 : Re-add and skip test #
Total comments: 3
Messages
Total messages: 34 (15 generated)
another quick fix --- there's some unnecessary test/throwing in NewPromiseCapabilities, but I think they're not too bad considering it's the slow path anyways
Patchset #1 (id:1) has been deleted
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1531073004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1531073004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) v8_linux_arm_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/11953) v8_linux_dbg on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg/builds/12549) v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/13659)
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js BUG=v8:4633 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js BUG=v8:4633 LOG=N R=littledan@chromium.org, cbruni@chromium.org ==========
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js BUG=v8:4633 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js BUG=v8:4633 LOG=N R=littledan@chromium.org, cbruni@chromium.org ==========
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js BUG=v8:4633 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js BUG=v8:4633, v8:4631, v8:4243 LOG=N R=littledan@chromium.org, cbruni@chromium.org ==========
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js BUG=v8:4633, v8:4631, v8:4243 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js BUG=v8:4633, v8:4631, v8:4243 LOG=N R=littledan@chromium.org, cbruni@chromium.org ==========
I think the debugger test is just invalid, because the assertions within it
depend on a willful spec violation where we don't eagerly throw.
this should be just an `assertThrows(function() { p.chain/p.then() },
TypeError)`, without needing to touch any of the debugger API. The behaviour is
asserted by some of the test262 bits, but maybe we could make this a willful
spec violation if people prefer that.
Thoughts?
On 2016/01/05 18:08:47, caitp wrote:
> I think the debugger test is just invalid, because the assertions within it
> depend on a willful spec violation where we don't eagerly throw.
>
> this should be just an `assertThrows(function() { p.chain/p.then() },
> TypeError)`, without needing to touch any of the debugger API. The behaviour
is
> asserted by some of the test262 bits, but maybe we could make this a willful
> spec violation if people prefer that.
>
> Thoughts?
Per v8:3641 / the comment in mjsunit.status, it looks like it's accepted that
this test is invalid, and I think it's alright to delete it, since the actual
thing that should be asserted does not really involve debug events, and is
already tested by test262
On 2016/01/05 at 18:59:25, caitpotter88 wrote:
> On 2016/01/05 18:08:47, caitp wrote:
> > I think the debugger test is just invalid, because the assertions within it
> > depend on a willful spec violation where we don't eagerly throw.
> >
> > this should be just an `assertThrows(function() { p.chain/p.then() },
> > TypeError)`, without needing to touch any of the debugger API. The behaviour
is
> > asserted by some of the test262 bits, but maybe we could make this a willful
> > spec violation if people prefer that.
> >
> > Thoughts?
>
> Per v8:3641 / the comment in mjsunit.status, it looks like it's accepted that
this test is invalid, and I think it's alright to delete it, since the actual
thing that should be asserted does not really involve debug events, and is
already tested by test262
Rather than deleting the test, could you add it to mjsunit.status with the
others? The best long-term change would be to fix the tests, as we actually do
want to have good test coverage for the debugger. In particular, this test seems
to get at the debugger's stack frame tracking working correctly (which doesn't
have perfect testing elsewhere).
On 2016/01/05 19:10:35, Dan Ehrenberg wrote:
> On 2016/01/05 at 18:59:25, caitpotter88 wrote:
> > On 2016/01/05 18:08:47, caitp wrote:
> > > I think the debugger test is just invalid, because the assertions within
it
> > > depend on a willful spec violation where we don't eagerly throw.
> > >
> > > this should be just an `assertThrows(function() { p.chain/p.then() },
> > > TypeError)`, without needing to touch any of the debugger API. The
behaviour
> is
> > > asserted by some of the test262 bits, but maybe we could make this a
willful
> > > spec violation if people prefer that.
> > >
> > > Thoughts?
> >
> > Per v8:3641 / the comment in mjsunit.status, it looks like it's accepted
that
> this test is invalid, and I think it's alright to delete it, since the actual
> thing that should be asserted does not really involve debug events, and is
> already tested by test262
>
> Rather than deleting the test, could you add it to mjsunit.status with the
> others? The best long-term change would be to fix the tests, as we actually do
> want to have good test coverage for the debugger. In particular, this test
seems
> to get at the debugger's stack frame tracking working correctly (which doesn't
> have perfect testing elsewhere).
I don't think there's any real "fix" for this test, because the exception
happens during the initial execution of the script, and is caught by d8's own
TryCatch --- so the uncaught exception event never occurs. Once the change is
made, there's basically no way to make it actually do what it's trying to do, as
far as I can tell. Maybe yang could get it working?
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js BUG=v8:4633, v8:4631, v8:4243 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js Per v8:3641, mjsunit/es6/debug-promises/throw-with-undefined-reject.js becomes invalid. The exception is thrown before the chain handler is ever invoked, and is caught externally by d8's own handler --- thus evading the uncaught exception event. BUG=v8:4633, v8:4631, v8:4243, v8:3641 LOG=N R=littledan@chromium.org, cbruni@chromium.org ==========
caitpotter88@gmail.com changed reviewers: + yangguo@chromium.org
On 2016/01/05 at 19:13:40, caitpotter88 wrote:
> On 2016/01/05 19:10:35, Dan Ehrenberg wrote:
> > On 2016/01/05 at 18:59:25, caitpotter88 wrote:
> > > On 2016/01/05 18:08:47, caitp wrote:
> > > > I think the debugger test is just invalid, because the assertions within
it
> > > > depend on a willful spec violation where we don't eagerly throw.
> > > >
> > > > this should be just an `assertThrows(function() { p.chain/p.then() },
> > > > TypeError)`, without needing to touch any of the debugger API. The
behaviour
> > is
> > > > asserted by some of the test262 bits, but maybe we could make this a
willful
> > > > spec violation if people prefer that.
> > > >
> > > > Thoughts?
> > >
> > > Per v8:3641 / the comment in mjsunit.status, it looks like it's accepted
that
> > this test is invalid, and I think it's alright to delete it, since the
actual
> > thing that should be asserted does not really involve debug events, and is
> > already tested by test262
> >
> > Rather than deleting the test, could you add it to mjsunit.status with the
> > others? The best long-term change would be to fix the tests, as we actually
do
> > want to have good test coverage for the debugger. In particular, this test
seems
> > to get at the debugger's stack frame tracking working correctly (which
doesn't
> > have perfect testing elsewhere).
>
> I don't think there's any real "fix" for this test, because the exception
happens during the initial execution of the script, and is caught by d8's own
TryCatch --- so the uncaught exception event never occurs. Once the change is
made, there's basically no way to make it actually do what it's trying to do, as
far as I can tell. Maybe yang could get it working?
Well, you could use the uncaught Promise rejection tracker, for example. It
would be a different test, but it's still good to have coverage of this path,
and I'm not sure we have that coverage anywhere else (maybe I'm wrong). Any of
us could make the change to get it working. Thanks for switching it to disabled.
The CQ bit was checked by littledan@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1531073004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1531073004/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/9460)
littledan@chromium.org changed reviewers: + adamk@chromium.org
Adam, could you do an OWNERS review?
lgtm besides one nit https://codereview.chromium.org/1531073004/diff/80001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/1531073004/diff/80001/src/js/promise.js#newco... src/js/promise.js:269: %_Call(promiseCapability.reject, UNDEFINED, r); Any reason to use %_Call here instead of just making the call directly?
https://codereview.chromium.org/1531073004/diff/80001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/1531073004/diff/80001/src/js/promise.js#newco... src/js/promise.js:269: %_Call(promiseCapability.reject, UNDEFINED, r); On 2016/01/05 20:33:09, adamk wrote: > Any reason to use %_Call here instead of just making the call directly? The `promiseCapability` object is emulating a Record type in the spec, and is not meant to be exposed to userland JS. It's possible to see it as the method receiver when the callback is not created by v8's implementation
https://codereview.chromium.org/1531073004/diff/80001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/1531073004/diff/80001/src/js/promise.js#newco... src/js/promise.js:269: %_Call(promiseCapability.reject, UNDEFINED, r); On 2016/01/05 20:39:54, caitp wrote: > On 2016/01/05 20:33:09, adamk wrote: > > Any reason to use %_Call here instead of just making the call directly? > > The `promiseCapability` object is emulating a Record type in the spec, and is > not meant to be exposed to userland JS. > > It's possible to see it as the method receiver when the callback is not created > by v8's implementation You could do: var reject = promiseCapability.reject; reject(r); but I guess that's not a huge gain, given you have access to %_Call.
The CQ bit was checked by caitpotter88@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1531073004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1531073004/80001
Message was sent while issue was closed.
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js Per v8:3641, mjsunit/es6/debug-promises/throw-with-undefined-reject.js becomes invalid. The exception is thrown before the chain handler is ever invoked, and is caught externally by d8's own handler --- thus evading the uncaught exception event. BUG=v8:4633, v8:4631, v8:4243, v8:3641 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js Per v8:3641, mjsunit/es6/debug-promises/throw-with-undefined-reject.js becomes invalid. The exception is thrown before the chain handler is ever invoked, and is caught externally by d8's own handler --- thus evading the uncaught exception event. BUG=v8:4633, v8:4631, v8:4243, v8:3641 LOG=N R=littledan@chromium.org, cbruni@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js Per v8:3641, mjsunit/es6/debug-promises/throw-with-undefined-reject.js becomes invalid. The exception is thrown before the chain handler is ever invoked, and is caught externally by d8's own handler --- thus evading the uncaught exception event. BUG=v8:4633, v8:4631, v8:4243, v8:3641 LOG=N R=littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] Make Promise.reject match spec, and validate promise capabilities Correctly validate promise capabilities in NewPromiseCapabilities() and in GetCapabilitiesExtractor(). Also explicitly follows Promise.race step 2 and similar cases in the spec, rather than passing tests asserting these steps are taken in NewPromiseCapability Also changes Promise.reject to match specification. Fixes the following test262 tests: - built-ins/Promise/all/capability-executor-called-twice.js - built-ins/Promise/all/capability-executor-not-callable.js - built-ins/Promise/prototype/then/capability-executor-called-twice.js - built-ins/Promise/prototype/then/capability-executor-not-callable.js - built-ins/Promise/reject/capability-executor-called-twice.js - built-ins/Promise/reject/capability-executor-not-callable.js - built-ins/Promise/resolve/capability-executor-called-twice.js - built-ins/Promise/resolve/capability-executor-not-callable.js - built-ins/Promise/race/capability-executor-called-twice.js - built-ins/Promise/race/capability-executor-not-callable.js - built-ins/Promise/reject/S25.4.4.4_A3.1_T1.js - built-ins/Promise/race/S25.4.4.3_A3.1_T2.js Per v8:3641, mjsunit/es6/debug-promises/throw-with-undefined-reject.js becomes invalid. The exception is thrown before the chain handler is ever invoked, and is caught externally by d8's own handler --- thus evading the uncaught exception event. BUG=v8:4633, v8:4631, v8:4243, v8:3641 LOG=N R=littledan@chromium.org, cbruni@chromium.org Committed: https://crrev.com/e4af5cdbf9ae081c6b69261ea7d7addf95e1e524 Cr-Commit-Position: refs/heads/master@{#33128} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e4af5cdbf9ae081c6b69261ea7d7addf95e1e524 Cr-Commit-Position: refs/heads/master@{#33128}
Message was sent while issue was closed.
On 2016/01/05 at 22:19:48, commit-bot wrote: > Patchset 4 (id:??) landed as https://crrev.com/e4af5cdbf9ae081c6b69261ea7d7addf95e1e524 > Cr-Commit-Position: refs/heads/master@{#33128} A partial rollback of this patch was done at https://codereview.chromium.org/1578893002/ |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
