[promises] Refactor debug code

[promises] Refactor debug code

-- Removes remaning debug from promise.js and moves it to c++
-- Changes debug_id to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Changes debug_name to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Adds PromiseDebugActionName and PromiseDebugActionType enums
-- Adds PromiseDebugActionNameToString and
   PromiseDebugActionTypeToString helper methods
-- Changes variable `status` to be int in runtime functions.
-- Changes debug_id to start from 1, not 0 for easier bookkeeping.

BUG=v8:5343
Review-Url: https://codereview.chromium.org/2606093002
Cr-Commit-Position: refs/heads/master@{#42052}
Committed: https://chromium.googlesource.com/v8/v8/+/a2c15ba376840e5849d77907d1213c8794c48392

[gsathya, 2016-12-29 20:57:10]

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

[commit-bot: I haz the power, 2016-12-29 20:57:17]

Dry run: 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-29 21:39:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-29 21:39:00]

Dry run: Try jobs failed on following builders:
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18434)
  v8_linux64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng_triggered...)

[gsathya, 2016-12-29 23:45:15]

Description was changed from

==========
[promises] Move rest of debug code to c++

Also changes variable `status` to be int in runtime functions.

BUG=v8:5343
==========

to

==========
[promises] Refactor debug code

-- Removes remaning debug from promise.js and moves it to c++
-- Changes debug_id to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Changes debug_name to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Adds PromiseDebugActionName and PromiseDebugActionType enums
-- Adds PromiseDebugActionNameToString and
   PromiseDebugActionTypeToString helper methods
-- Changes variable `status` to be int in runtime functions.
-- Changes debug_id to start from 1, not 0 for easier bookkeeping.

BUG=v8:5343
==========

[gsathya, 2016-12-29 23:45:18]

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

[commit-bot: I haz the power, 2016-12-29 23:45:20]

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

[gsathya, 2016-12-29 23:46:49]

Description was changed from

==========
[promises] Refactor debug code

-- Removes remaning debug from promise.js and moves it to c++
-- Changes debug_id to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Changes debug_name to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Adds PromiseDebugActionName and PromiseDebugActionType enums
-- Adds PromiseDebugActionNameToString and
   PromiseDebugActionTypeToString helper methods
-- Changes variable `status` to be int in runtime functions.
-- Changes debug_id to start from 1, not 0 for easier bookkeeping.

BUG=v8:5343
==========

to

==========
[promises] Refactor debug code

-- Removes remaning debug from promise.js and moves it to c++
-- Changes debug_id to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Changes debug_name to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Adds PromiseDebugActionName and PromiseDebugActionType enums
-- Adds PromiseDebugActionNameToString and
   PromiseDebugActionTypeToString helper methods
-- Changes variable `status` to be int in runtime functions.
-- Changes debug_id to start from 1, not 0 for easier bookkeeping.

BUG=v8:5343
==========

[gsathya, 2016-12-29 23:46:49]

gsathya@chromium.org changed reviewers:
+ adamk@chromium.org, yangguo@chromium.org

[commit-bot: I haz the power, 2016-12-29 23:50:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-29 23:50:30]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18438)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)

[gsathya, 2016-12-29 23:53:10]

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

[commit-bot: I haz the power, 2016-12-29 23:53:20]

Dry run: 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-29 23:55:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-29 23:55:50]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/14855)

[adamk, 2016-12-30 00:10:04]

https://codereview.chromium.org/2606093002/diff/10001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2606093002/diff/10001/src/code-stub-assembler...
src/code-stub-assembler.cc:8363: StoreObjectFieldNoWriteBarrier(result,
PromiseReactionJobInfo::kDebugIdOffset,
I think this meant to say kDebugNameOffset.

https://codereview.chromium.org/2606093002/diff/10001/src/objects-debug.cc
File src/objects-debug.cc (left):

https://codereview.chromium.org/2606093002/diff/10001/src/objects-debug.cc#ol...
src/objects-debug.cc:1025: CHECK(debug_name()->IsString() ||
debug_name()->IsUndefined(isolate));
I think these should be replaced with VerifySmiField() calls

https://codereview.chromium.org/2606093002/diff/10001/src/objects-debug.cc#ol...
src/objects-debug.cc:1045: CHECK(debug_name()->IsString() ||
debug_name()->IsUndefined(isolate));
Same here

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
File src/runtime/runtime-promise.cc (right):

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:64: void PromiseFulfillDebugInfo(Isolate*
isolate,
This doesn't "fulfill" anything...should this be called "SetDebugInfo"?

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:88: void AsyncFunctionDebugInfo(Isolate* isolate,
This function name reads oddly to me, it seems to be missing a verb. What does
the function _do_? Maybe "EmitAsyncFunctionDebugInfo"?

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:93: return;
I think it would be clearer to factor these multiple returns as, instead, a
helper function that takes care of determining if we're dealing with an async
function.

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:214: int debug_id = 0;
Please define a constant somewhere for this 0, something like
kInvalidPromiseDebugId.

[gsathya, 2016-12-30 00:28:20]

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

[commit-bot: I haz the power, 2016-12-30 00:28:30]

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

[gsathya, 2016-12-30 00:29:00]

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

[commit-bot: I haz the power, 2016-12-30 00:29:08]

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

[gsathya, 2016-12-30 01:11:10]

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

[commit-bot: I haz the power, 2016-12-30 01:11:16]

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

[gsathya, 2016-12-30 01:19:11]

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

[commit-bot: I haz the power, 2016-12-30 01:19:16]

Dry run: 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-30 01:43:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-30 01:43:21]

Dry run: This issue passed the CQ dry run.

[gsathya, 2016-12-30 01:48:51]

https://codereview.chromium.org/2606093002/diff/10001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2606093002/diff/10001/src/code-stub-assembler...
src/code-stub-assembler.cc:8363: StoreObjectFieldNoWriteBarrier(result,
PromiseReactionJobInfo::kDebugIdOffset,
On 2016/12/30 00:10:04, adamk wrote:
> I think this meant to say kDebugNameOffset.

Done.

https://codereview.chromium.org/2606093002/diff/10001/src/objects-debug.cc
File src/objects-debug.cc (left):

https://codereview.chromium.org/2606093002/diff/10001/src/objects-debug.cc#ol...
src/objects-debug.cc:1025: CHECK(debug_name()->IsString() ||
debug_name()->IsUndefined(isolate));
On 2016/12/30 00:10:04, adamk wrote:
> I think these should be replaced with VerifySmiField() calls

Done.

https://codereview.chromium.org/2606093002/diff/10001/src/objects-debug.cc#ol...
src/objects-debug.cc:1045: CHECK(debug_name()->IsString() ||
debug_name()->IsUndefined(isolate));
On 2016/12/30 00:10:04, adamk wrote:
> Same here

Done.

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
File src/runtime/runtime-promise.cc (right):

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:64: void PromiseFulfillDebugInfo(Isolate*
isolate,
On 2016/12/30 00:10:04, adamk wrote:
> This doesn't "fulfill" anything...should this be called "SetDebugInfo"?

Done.

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:88: void AsyncFunctionDebugInfo(Isolate* isolate,
On 2016/12/30 00:10:04, adamk wrote:
> This function name reads oddly to me, it seems to be missing a verb. What does
> the function _do_? Maybe "EmitAsyncFunctionDebugInfo"?

Done.

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:93: return;
On 2016/12/30 00:10:04, adamk wrote:
> I think it would be clearer to factor these multiple returns as, instead, a
> helper function that takes care of determining if we're dealing with an async
> function.

Done.

https://codereview.chromium.org/2606093002/diff/10001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:214: int debug_id = 0;
On 2016/12/30 00:10:04, adamk wrote:
> Please define a constant somewhere for this 0, something like
> kInvalidPromiseDebugId.

Done.

[adamk, 2017-01-03 18:18:17]

lgtm % naming

https://codereview.chromium.org/2606093002/diff/110001/src/debug/debug.h
File src/debug/debug.h (right):

https://codereview.chromium.org/2606093002/diff/110001/src/debug/debug.h#newc...
src/debug/debug.h:74: const int DEBUG_PROMISE_DEFAULT_ID = 0;
kDebugPromiseDefaultId please. SHOUTING_CASE is only for macros and "legacy"
enums.

Also, is this the "default" id or does this represent something like "no debug
id?" Maybe kDebugPromiseNoId?

[gsathya, 2017-01-03 19:52:21]

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

[commit-bot: I haz the power, 2017-01-03 19:52:30]

Dry run: 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, 2017-01-03 20:10:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-03 20:10:14]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[gsathya, 2017-01-03 20:17:46]

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

[commit-bot: I haz the power, 2017-01-03 20:17:53]

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

[gsathya, 2017-01-03 20:26:58]

On 2017/01/03 18:18:17, adamk wrote:
> lgtm % naming
> 
> https://codereview.chromium.org/2606093002/diff/110001/src/debug/debug.h
> File src/debug/debug.h (right):
> 
>
https://codereview.chromium.org/2606093002/diff/110001/src/debug/debug.h#newc...
> src/debug/debug.h:74: const int DEBUG_PROMISE_DEFAULT_ID = 0;
> kDebugPromiseDefaultId please. SHOUTING_CASE is only for macros and "legacy"
> enums.
> 
> Also, is this the "default" id or does this represent something like "no debug
> id?" Maybe kDebugPromiseNoId?

Done. Changed the enums to camel case as well

[commit-bot: I haz the power, 2017-01-03 20:43:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-03 20:43:03]

Dry run: This issue passed the CQ dry run.

[adamk, 2017-01-03 20:48:17]

lgtm, though you'll need Yang for debug/OWNERS

[Yang, 2017-01-03 20:58:06]

src/debug (mostly rubberstamp) lgtm.

[gsathya, 2017-01-03 21:41:32]

The CQ bit was checked by gsathya@chromium.org

[commit-bot: I haz the power, 2017-01-03 21:41:41]

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, 2017-01-03 21:43:39]

CQ is committing da patch.

Bot data: {"patchset_id": 150001, "attempt_start_ts": 1483479692345980,
"parent_rev": "c523474713e11e98f0b87f2d7137cf3465aab13c", "commit_rev":
"a2c15ba376840e5849d77907d1213c8794c48392"}

[commit-bot: I haz the power, 2017-01-03 21:43:43]

Description was changed from

==========
[promises] Refactor debug code

-- Removes remaning debug from promise.js and moves it to c++
-- Changes debug_id to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Changes debug_name to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Adds PromiseDebugActionName and PromiseDebugActionType enums
-- Adds PromiseDebugActionNameToString and
   PromiseDebugActionTypeToString helper methods
-- Changes variable `status` to be int in runtime functions.
-- Changes debug_id to start from 1, not 0 for easier bookkeeping.

BUG=v8:5343
==========

to

==========
[promises] Refactor debug code

-- Removes remaning debug from promise.js and moves it to c++
-- Changes debug_id to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Changes debug_name to be a smi in PromiseReactionJobInfo and
   PromiseResolveThenableJobInfo.
-- Adds PromiseDebugActionName and PromiseDebugActionType enums
-- Adds PromiseDebugActionNameToString and
   PromiseDebugActionTypeToString helper methods
-- Changes variable `status` to be int in runtime functions.
-- Changes debug_id to start from 1, not 0 for easier bookkeeping.

BUG=v8:5343

Review-Url: https://codereview.chromium.org/2606093002
Cr-Commit-Position: refs/heads/master@{#42052}
Committed:
https://chromium.googlesource.com/v8/v8/+/a2c15ba376840e5849d77907d1213c8794c...
==========

[commit-bot: I haz the power, 2017-01-03 21:43:44]

Committed patchset #9 (id:150001) as
https://chromium.googlesource.com/v8/v8/+/a2c15ba376840e5849d77907d1213c8794c...