[api] add API for Promise status and result.

[api] add API for Promise status and result.

Currently, to find out a Promise's status and result, one has to use the
debug context. This is for example done in Node.js. This new API is a
better replacement, also in the context of the debug context being
deprecated eventually.

R=franzih@chromium.org, gsathya@chromium.org, jochen@chromium.org
BUG=v8:5764
Review-Url: https://codereview.chromium.org/2589113002
Cr-Commit-Position: refs/heads/master@{#41855}
Committed: https://chromium.googlesource.com/v8/v8/+/28432588c231019c2bb0dfbba8eba4304779706b

[Yang, 2016-12-20 06:51:41]

Please take a look. This is required for migrating Node.js off the debug context
API.

[Yang, 2016-12-20 06:53:03]

Description was changed from

==========
[api] add API for Promise status and result.

Currently, to find out a Promise's status and result, one has to use the
debug context. This is for example done in Node.js. This new API is a
better replacement, also in the context of the debug context being
deprecated eventually.

R=franzih@chromium.org, gsathya@chromium.org, jochen@chromium.org
==========

to

==========
[api] add API for Promise status and result.

Currently, to find out a Promise's status and result, one has to use the
debug context. This is for example done in Node.js. This new API is a
better replacement, also in the context of the debug context being
deprecated eventually.

R=franzih@chromium.org, gsathya@chromium.org, jochen@chromium.org
BUG=v8:5764
==========

[gsathya, 2016-12-20 06:57:25]

lgtm

[Yang, 2016-12-20 07:38:37]

The CQ bit was checked by yangguo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 07:38:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jochen (gone - plz use gerrit), 2016-12-20 07:49:52]

https://codereview.chromium.org/2589113002/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/2589113002/diff/1/include/v8.h#newcode3702
include/v8.h:3702: enum PromiseStatus { kPending, kFulfilled, kRejected };
why not an enum class?

also, please add a comment explaining what the values mean

https://codereview.chromium.org/2589113002/diff/1/include/v8.h#newcode3760
include/v8.h:3760: MaybeLocal<Value> Result();

please also add a comment to document this API (and Status() below)

https://codereview.chromium.org/2589113002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7296
src/api.cc:7296: ENTER_V8(isolate);
this method (and Status) don't execute script, so we don't need to enter here

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7297
src/api.cc:7297: Utils::ApiCheck(promise->IsJSPromise(), "v8_Promise_Result",
"Not a Promise");
that check should never trigger, so I'd suggest to remove it

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7303
src/api.cc:7303: return MaybeLocal<Value>();
APIs should return empty handles iff an exception was thrown.

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7313
src/api.cc:7313: STATIC_ASSERT(static_cast<int>(i::kPromisePending) ==
kPending);
an alternative approach was to use the external enum internally

[Yang, 2016-12-20 11:38:27]

Addressed comments. Please take a look.

https://codereview.chromium.org/2589113002/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/2589113002/diff/1/include/v8.h#newcode3702
include/v8.h:3702: enum PromiseStatus { kPending, kFulfilled, kRejected };
On 2016/12/20 07:49:52, jochen wrote:
> why not an enum class?
> 
> also, please add a comment explaining what the values mean

enum class requires static_cast all over the place because we use this enum as
an integer value when storing as a smi.

https://codereview.chromium.org/2589113002/diff/1/include/v8.h#newcode3760
include/v8.h:3760: MaybeLocal<Value> Result();
On 2016/12/20 07:49:52, jochen wrote:
> 
> please also add a comment to document this API (and Status() below)

Done.

https://codereview.chromium.org/2589113002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7296
src/api.cc:7296: ENTER_V8(isolate);
On 2016/12/20 07:49:52, jochen wrote:
> this method (and Status) don't execute script, so we don't need to enter here

Done.

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7297
src/api.cc:7297: Utils::ApiCheck(promise->IsJSPromise(), "v8_Promise_Result",
"Not a Promise");
On 2016/12/20 07:49:52, jochen wrote:
> that check should never trigger, so I'd suggest to remove it

Done.

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7303
src/api.cc:7303: return MaybeLocal<Value>();
On 2016/12/20 07:49:52, jochen wrote:
> APIs should return empty handles iff an exception was thrown.

Done. Now requiring status not to be pending guarded by ApiCheck.

https://codereview.chromium.org/2589113002/diff/1/src/api.cc#newcode7313
src/api.cc:7313: STATIC_ASSERT(static_cast<int>(i::kPromisePending) ==
kPending);
On 2016/12/20 07:49:52, jochen wrote:
> an alternative approach was to use the external enum internally

Done.

[jochen (gone - plz use gerrit), 2016-12-20 11:41:21]

lgtm

[Yang, 2016-12-20 11:43:22]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-12-20 11:43:23]

The patchset sent to the CQ was uploaded after l-g-t-m from gsathya@chromium.org
Link to the patchset: https://codereview.chromium.org/2589113002/#ps60001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-12-20 11:43:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 12:13:30]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482234202755810,
"parent_rev": "55c88e586badc6b29215c9a068900df13ea88721", "commit_rev":
"28432588c231019c2bb0dfbba8eba4304779706b"}

[commit-bot: I haz the power, 2016-12-20 12:13:36]

Description was changed from

==========
[api] add API for Promise status and result.

Currently, to find out a Promise's status and result, one has to use the
debug context. This is for example done in Node.js. This new API is a
better replacement, also in the context of the debug context being
deprecated eventually.

R=franzih@chromium.org, gsathya@chromium.org, jochen@chromium.org
BUG=v8:5764
==========

to

==========
[api] add API for Promise status and result.

Currently, to find out a Promise's status and result, one has to use the
debug context. This is for example done in Node.js. This new API is a
better replacement, also in the context of the debug context being
deprecated eventually.

R=franzih@chromium.org, gsathya@chromium.org, jochen@chromium.org
BUG=v8:5764

Review-Url: https://codereview.chromium.org/2589113002
Cr-Commit-Position: refs/heads/master@{#41855}
Committed:
https://chromium.googlesource.com/v8/v8/+/28432588c231019c2bb0dfbba8eba430477...
==========

[commit-bot: I haz the power, 2016-12-20 12:13:36]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/28432588c231019c2bb0dfbba8eba430477...