[V8] Report v8::AfterCompile and v8::CompileError to listener on pause

[V8] Report v8::AfterCompile and v8::CompileError to listener on pause

V8 didn't report compile events on pause before this patch. These events can be important for listener. For example, DevTools allows user to execute some JS code on pause and needs to show correct stack trace in message from it.

BUG=396013
R=yurys@chromium.org

[kozy, 2014-12-07 15:42:48]

Yury, please take a look!

[yurys, 2014-12-08 07:19:28]

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

https://codereview.chromium.org/781623004/diff/1/src/debug.cc#newcode2485
src/debug.cc:2485: Handle<GlobalObject> global = in_debug_scope() ?
Handle<GlobalObject>(debug_context()->global_object()) :
Handle<GlobalObject>(isolate_->global_object());
This way we will create debugger objects in the user context. This is dangerous
since the user code can override almost everything and we may at some point end
up executing user code in the debugger context. Please make sure to enter
debugger context first if you need to create an object for the debugger.

[yurys, 2014-12-08 10:55:51]

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

https://codereview.chromium.org/781623004/diff/1/src/debug.cc#newcode2860
src/debug.cc:2860: if (event_listener_.is_null()) return;
I believe you can reenter debug context here if it is not on the top of the
stack. Also it should be possible to use SuppressDebug scope here.

[kozy, 2014-12-08 12:40:58]

Patchset #2 (id:20001) has been deleted

[kozy, 2014-12-08 14:19:30]

Added pushing debug context to top of stack by entering debug scope and
suppressing debug events in process compile event function.

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

https://codereview.chromium.org/781623004/diff/1/src/debug.cc#newcode2485
src/debug.cc:2485: Handle<GlobalObject> global = in_debug_scope() ?
Handle<GlobalObject>(debug_context()->global_object()) :
Handle<GlobalObject>(isolate_->global_object());
On 2014/12/08 07:19:28, yurys wrote:
> This way we will create debugger objects in the user context. This is
dangerous
> since the user code can override almost everything and we may at some point
end
> up executing user code in the debugger context. Please make sure to enter
> debugger context first if you need to create an object for the debugger.

Done.

https://codereview.chromium.org/781623004/diff/1/src/debug.cc#newcode2860
src/debug.cc:2860: if (event_listener_.is_null()) return;
On 2014/12/08 10:55:51, yurys wrote:
> I believe you can reenter debug context here if it is not on the top of the
> stack. Also it should be possible to use SuppressDebug scope here.

Done.

[yurys, 2014-12-08 14:31:22]

lgtm

https://codereview.chromium.org/781623004/diff/40001/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/781623004/diff/40001/src/debug.cc#newcode2864
src/debug.cc:2864: DebugScope debug_scope(this);
You might want to skip this if debug context is on top of the stack but we most
likely won't receive compile events from debug context here, so I think it
should be fine as is.

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-clear...
File test/mjsunit/debug-clearbreakpointgroup.js (right):

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-clear...
test/mjsunit/debug-clearbreakpointgroup.js:43: muteListener = true;
try/finally

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-compi...
File test/mjsunit/debug-compile-event.js (right):

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-compi...
test/mjsunit/debug-compile-event.js:86: var msg = eval('(' + json + ')');
try/finally

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-evalu...
File test/mjsunit/debug-evaluate-with-context.js (right):

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-evalu...
test/mjsunit/debug-evaluate-with-context.js:35: if (event ==
Debug.DebugEvent.AfterCompile ||
should it be  

if (event !== Debug.DebugEvent.DebugBreak)
    return
 ?

[kozy, 2014-12-08 14:53:01]

https://codereview.chromium.org/781623004/diff/40001/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/781623004/diff/40001/src/debug.cc#newcode2864
src/debug.cc:2864: DebugScope debug_scope(this);
On 2014/12/08 14:31:22, yurys wrote:
> You might want to skip this if debug context is on top of the stack but we
most
> likely won't receive compile events from debug context here, so I think it
> should be fine as is.

Acknowledged.

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-clear...
File test/mjsunit/debug-clearbreakpointgroup.js (right):

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-clear...
test/mjsunit/debug-clearbreakpointgroup.js:43: muteListener = true;
On 2014/12/08 14:31:22, yurys wrote:
> try/finally

Done.

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-compi...
File test/mjsunit/debug-compile-event.js (right):

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-compi...
test/mjsunit/debug-compile-event.js:86: var msg = eval('(' + json + ')');
On 2014/12/08 14:31:22, yurys wrote:
> try/finally

Done.

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-evalu...
File test/mjsunit/debug-evaluate-with-context.js (right):

https://codereview.chromium.org/781623004/diff/40001/test/mjsunit/debug-evalu...
test/mjsunit/debug-evaluate-with-context.js:35: if (event ==
Debug.DebugEvent.AfterCompile ||
On 2014/12/08 14:31:22, yurys wrote:
> should it be  
> 
> if (event !== Debug.DebugEvent.DebugBreak)
>     return
>  ? 

Done.

[kozy, 2014-12-08 14:53:22]

kozyatinskiy@chromium.org changed reviewers:
+ yangguo@chromium.org

[kozy, 2014-12-08 14:53:23]

Yang, please take a look.

[Yang, 2014-12-10 20:50:10]

On 2014/12/08 14:53:23, kozyatinskiy_chromium wrote:
> Yang, please take a look.

lgtm.

Btw. you can use CQ to commit.

[kozy, 2014-12-11 10:33:35]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2014-12-11 10:34:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/781623004/60001

[commit-bot: I haz the power, 2014-12-11 10:58:54]

Committed patchset #3 (id:60001)