Implement issue 554 Add "ProcessDebuggerRequests" call to Debug Agent API

Implement issue 554 Add "ProcessDebuggerRequests" call to Debug Agent API

[Peter Rybin, 2010-01-14 20:59:50]

Hi Sorren

This solution seems to be working. I am not sure it is 100% correct though.

Peter

[Søren Thygesen Gjesse, 2010-01-15 09:10:40]

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.

[Peter Rybin, 2010-01-15 20:00:05]

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
On 2010/01/15 09:10:40, Søren Gjesse wrote:
> 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).

I redid it.
However you may find it now too general.

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);
On 2010/01/15 09:10:40, Søren Gjesse wrote:
> 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.

My bad :(
Could be a problem.
Done

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.
On 2010/01/15 09:10:40, Søren Gjesse wrote:
> I don't see how this comment relate to the actual test.

Done