LGTM with some comments. I'll land this if you change the API wrt implicit
Isolate parameter and my comments :)


https://codereview.chromium.org/11142013/diff/10001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/11142013/diff/10001/include/v8.h#newcode3424
include/v8.h:3424: static void ResumeExecution(Isolate* isolate = NULL);
On 2012/12/06 15:07:34, Sven Panne wrote:
In general, we are moving into the direction of requiring Isolate
parameters
when the implementation of the API function needs an Isolate. So I
would very
much prefer making the parameter non-optional and avoid introducing
yet another
old-style API entry.

Agreed.

https://codereview.chromium.org/11142013/diff/10001/src/execution.cc
File src/execution.cc (right):

https://codereview.chromium.org/11142013/diff/10001/src/execution.cc#newcode426
src/execution.cc:426: thread_local_.interrupt_flags_ &=
~static_cast<int>(TERMINATE);
If I'm not mistaken, this is already being done in
Execution::HandleStackGuard. An ASSERT would be enough.

https://codereview.chromium.org/11142013/diff/10001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/11142013/diff/10001/src/isolate.cc#newcode947
src/isolate.cc:947: && pending_exception() ==
heap_.termination_exception()) {
Please put the && at the end of the line and align pending_exception()
with has_pending_exception()

https://codereview.chromium.org/11142013/diff/10001/src/isolate.cc#newcode952
src/isolate.cc:952: && scheduled_exception() ==
heap_.termination_exception()) {
Ditto.

https://codereview.chromium.org/11142013/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to