LGTM

http://codereview.chromium.org/549057/diff/3001/2006
File include/v8-debug.h (right):

http://codereview.chromium.org/549057/diff/3001/2006#newcode268
include/v8-debug.h:268: * Makes V8 process all pending debug commands.
It is equivalent to running
The comment from "It is equivalent..." is a bit internal I think. How
about removing that part and instead describe when to use it (something
about debugger requests are not being processed unless V8 is running
some JavaScript or this function is called).

Maybe add a comment on the threading issues, or maybe we need a general
comment at the top on which functions can be called from any thread and
which needs a V8 thread (possibly using Locker).

Maybe also some comment on what happens of there is a suspend or
evaluate request (with regard to entered context).

http://codereview.chromium.org/549057/diff/3001/2008
File src/execution.cc (right):

http://codereview.chromium.org/549057/diff/3001/2008#newcode649
src/execution.cc:649: StackGuard::Continue(DEBUGBREAK);
I am not sure whether the DEBUGBREAK flag should be cleared when this is
called on behalf of ProcessDebuggerRequests. Would that not cause calls
to v8::Debug::DebugBreak() to be cancled when ProcessDebuggerRequests is
called? Might be that the use of v8::Debug::DebugBreak() does not work
well with the use of the debugger agent anyway.

http://codereview.chromium.org/549057/diff/3001/2010
File test/cctest/test-debug.cc (right):

http://codereview.chromium.org/549057/diff/3001/2010#newcode5665
test/cctest/test-debug.cc:5665: // debug-delay.js is executed.
I don't see how this comment relate to the actual test.

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

Reply via email to