Trigger exception debug event for promises at the throw site.

Trigger exception debug event for promises at the throw site.

R=rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21092

[Yang, 2014-04-30 12:06:06]

On 2014/04/30 12:04:25, Yang wrote:

Main changes are:
- Trigger the debug event for uncaught exceptions in promises at the throw site.
- Folded PendingExceptionInPromise event into Exception. The difference being
whether there is a promise attached.
- If calling deferred.reject throws, we use the newly thrown exception in the
debug event.

[Yang, 2014-04-30 12:07:01]

I'll work on making the performance impact for non-debugging less in an upcoming
CL.

[rossberg, 2014-04-30 12:55:17]

https://codereview.chromium.org/260723002/diff/1/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/260723002/diff/1/src/debug.cc#newcode1334
src/debug.cc:1334: current_promise_catch_handler_ = NULL;
Are we sure that PromiseHandle can never be reentered recursively?

https://codereview.chromium.org/260723002/diff/1/src/debug.h
File src/debug.h (right):

https://codereview.chromium.org/260723002/diff/1/src/debug.h#newcode239
src/debug.h:239: void PromiseHandlePrologue(Handle<JSFunction> promise);
Perhaps name the argument get_promise for clarity.

https://codereview.chromium.org/260723002/diff/1/src/promise.js
File src/promise.js (left):

https://codereview.chromium.org/260723002/diff/1/src/promise.js#oldcode320
src/promise.js:320: function GetPromiseStatus(promise) {
As discussed offline, I'd prefer to keep the accessors.

https://codereview.chromium.org/260723002/diff/1/src/promise.js
File src/promise.js (right):

https://codereview.chromium.org/260723002/diff/1/src/promise.js#newcode44
src/promise.js:44: %DebugPromiseHandleEpilogue();
Put his in 'finally' clause (also below).

https://codereview.chromium.org/260723002/diff/1/src/promise.js#newcode180
src/promise.js:180: %DebugPromiseHandlePrologue(function() { return
deferred.promise });
It's a bit weird that the prologue is executed twice. Doesn't it have to match
up with the Epilogue?

[aandrey, 2014-04-30 13:01:07]

looks good

https://codereview.chromium.org/260723002/diff/1/test/mjsunit/es6/debug-promi...
File test/mjsunit/es6/debug-promises-throw-in-reject.js (right):

https://codereview.chromium.org/260723002/diff/1/test/mjsunit/es6/debug-promi...
test/mjsunit/es6/debug-promises-throw-in-reject.js:46: assertEquals(q,
event_data.promise());
assertTrue(exec_state.frame(0).sourceLineText().indexOf('throw new
Error("reject")') > 0);

https://codereview.chromium.org/260723002/diff/1/test/mjsunit/es6/debug-promi...
File test/mjsunit/es6/debug-promises-undefined-reject.js (right):

https://codereview.chromium.org/260723002/diff/1/test/mjsunit/es6/debug-promi...
test/mjsunit/es6/debug-promises-undefined-reject.js:43: assertEquals(q,
event_data.promise());
check frame 0 source line?

[Yang, 2014-04-30 13:20:14]

https://codereview.chromium.org/260723002/diff/1/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/260723002/diff/1/src/debug.cc#newcode1334
src/debug.cc:1334: current_promise_catch_handler_ = NULL;
On 2014/04/30 12:55:17, rossberg wrote:
> Are we sure that PromiseHandle can never be reentered recursively?

I'm asserting now that the prologue always gets the uninitialized state.

https://codereview.chromium.org/260723002/diff/1/src/debug.h
File src/debug.h (right):

https://codereview.chromium.org/260723002/diff/1/src/debug.h#newcode239
src/debug.h:239: void PromiseHandlePrologue(Handle<JSFunction> promise);
On 2014/04/30 12:55:17, rossberg wrote:
> Perhaps name the argument get_promise for clarity.

Done.

https://codereview.chromium.org/260723002/diff/1/src/promise.js
File src/promise.js (left):

https://codereview.chromium.org/260723002/diff/1/src/promise.js#oldcode320
src/promise.js:320: function GetPromiseStatus(promise) {
On 2014/04/30 12:55:17, rossberg wrote:
> As discussed offline, I'd prefer to keep the accessors.

Done.

https://codereview.chromium.org/260723002/diff/1/src/promise.js
File src/promise.js (right):

https://codereview.chromium.org/260723002/diff/1/src/promise.js#newcode44
src/promise.js:44: %DebugPromiseHandleEpilogue();
On 2014/04/30 12:55:17, rossberg wrote:
> Put his in 'finally' clause (also below).

Done.

https://codereview.chromium.org/260723002/diff/1/src/promise.js#newcode180
src/promise.js:180: %DebugPromiseHandlePrologue(function() { return
deferred.promise });
On 2014/04/30 12:55:17, rossberg wrote:
> It's a bit weird that the prologue is executed twice. Doesn't it have to match
> up with the Epilogue?

Done.

https://codereview.chromium.org/260723002/diff/1/test/mjsunit/es6/debug-promi...
File test/mjsunit/es6/debug-promises-undefined-reject.js (right):

https://codereview.chromium.org/260723002/diff/1/test/mjsunit/es6/debug-promi...
test/mjsunit/es6/debug-promises-undefined-reject.js:43: assertEquals(q,
event_data.promise());
On 2014/04/30 13:01:07, aandrey wrote:
> check frame 0 source line?

There is really no way to do that. The resulting error stack trace is entirely
native code. This happens because the promise object is a custom one and the
reject handler that was queued as microtask is not a valid function.

[rossberg, 2014-04-30 13:55:38]

lgtm

[aandrey, 2014-04-30 13:58:25]

https://codereview.chromium.org/260723002/diff/20001/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/260723002/diff/20001/src/debug.cc#newcode1331
src/debug.cc:1331: current_promise_catch_handler_ = NULL;
maybe add ASSERT(!current_promise_getter_.is_null());

[Yang, 2014-04-30 14:14:58]

https://codereview.chromium.org/260723002/diff/20001/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/260723002/diff/20001/src/debug.cc#newcode1331
src/debug.cc:1331: current_promise_catch_handler_ = NULL;
On 2014/04/30 13:58:26, aandrey wrote:
> maybe add ASSERT(!current_promise_getter_.is_null());

would make sense, but some runtime fuzzing tests would fail.

[Yang, 2014-04-30 14:17:57]

Committed patchset #2 manually as r21092 (presubmit successful).