https://codereview.chromium.org/812583003/diff/1/include/v8-debug.h
File include/v8-debug.h (right):

https://codereview.chromium.org/812583003/diff/1/include/v8-debug.h#newcode188
include/v8-debug.h:188: // VM thread. The task is deleted after
execution.
On 2014/12/17 14:25:02, yurys wrote:
Please update the comment to reflect that the task will be deleted
regardless of
the execution.

Done.

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

https://codereview.chromium.org/812583003/diff/1/src/debug.cc#newcode3110
src/debug.cc:3110: if (!isolate_->debug()->in_debug_scope()) {
On 2014/12/17 14:25:02, yurys wrote:
if (!in_debug_scope())

Done.

https://codereview.chromium.org/812583003/diff/1/src/debug.cc#newcode3153
src/debug.cc:3153: isolate_->stack_guard()->ClearDebugCommand();
On 2014/12/17 14:25:02, yurys wrote:
It seems fine to reuse the same interrupt bit as for debug commands
but we might
want to reconsider it later.

Acknowledged.

https://codereview.chromium.org/812583003/diff/1/src/debug.cc#newcode3535
src/debug.cc:3535: }
On 2014/12/17 14:25:02, yurys wrote:
empty line?

Surprisingly git cl format does not want an empty line here.

https://codereview.chromium.org/812583003/diff/1/test/cctest/test-debug.cc
File test/cctest/test-debug.cc (right):

https://codereview.chromium.org/812583003/diff/1/test/cctest/test-debug.cc#newcode7407
test/cctest/test-debug.cc:7407: v8::base::OS::Sleep(delay_ms_);
On 2014/12/17 14:25:03, yurys wrote:
We can avoid this Sleep. E.g. "while" loop below can call a native
function that
would on first call start this thread and after that would return
false until
StopExecutionTask is executed and flips some flag.

I would like to make sure the BreakAndRun is engaged when the VM loops
in a pure JS code (i.e. having no native callbacks).
I realize Sleep does not guarantee that, but it makes the signal fall
into random points of VM execution, i.e. it covers more cases. The test
should not fail in any case, so it is not a source of flakiness.

https://codereview.chromium.org/812583003/diff/1/test/cctest/test-debug.cc#newcode7408
test/cctest/test-debug.cc:7408: v8::Debug::BreakAndRun(isolate_, task_);
On 2014/12/17 14:25:03, yurys wrote:
task_ = NULL?

Done.

https://codereview.chromium.org/812583003/diff/1/test/cctest/test-debug.cc#newcode7422
test/cctest/test-debug.cc:7422: context_->Global()->Set(v8_str("stop"),
v8::True(context_->GetIsolate()));
On 2014/12/17 14:25:03, yurys wrote:
We can use a static C++ variable instead, it would indicate that we
can break
the loop.

See my comment above. I'd like it to be a pure JS loop.

https://codereview.chromium.org/812583003/diff/1/test/cctest/test-debug.cc#newcode7435
test/cctest/test-debug.cc:7435: DelayedTask
delayed_task(env->GetIsolate(), task, 100);
On 2014/12/17 14:25:03, yurys wrote:
Or can we at least reduce the delay to 10 or something?

Acknowledged.

https://codereview.chromium.org/812583003/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to