Ignore pause on debugger statement when breakpoints are disabled

Ignore pause on debugger statement when breakpoints are disabled

This behavior was changed in https://codereview.chromium.org/1402913002.
It's pretty usefull to have ability to disable debugger statement for our users.

BUG=chromium:583515
LOG=N
R=yangguo@chromium.org

Committed: https://crrev.com/4b54c07dd11c99f3532fb059eaac4e4edba22dab
Cr-Commit-Position: refs/heads/master@{#33960}

[kozy, 2016-02-12 03:11:43]

Yang, please take a look!

[Yang, 2016-02-12 05:56:23]

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

https://codereview.chromium.org/1690173002/diff/1/src/debug/debug.cc#newcode2220
src/debug/debug.cc:2220: (!break_points_active_ &&
thread_local_.last_step_action_ == StepNone))
Why do we need to check last_step_action_? Stepping in V8 does not go this code
path. I think DevTools does for async stepping, as it uses debug break
interrupts to break asap when it executes in V8? Leaking that implementation
detail over to V8 sounds like a bad choice. And there is no test case for that
in V8.

Since this is only related to the debugger statement, could we simply do the
check for break_points_active_ in Runtime_HandleDebuggerStatement?

[kozy, 2016-02-12 17:22:04]

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

https://codereview.chromium.org/1690173002/diff/1/src/debug/debug.cc#newcode2220
src/debug/debug.cc:2220: (!break_points_active_ &&
thread_local_.last_step_action_ == StepNone))
On 2016/02/12 05:56:23, Yang wrote:
> Why do we need to check last_step_action_? Stepping in V8 does not go this
code
> path. I think DevTools does for async stepping, as it uses debug break
> interrupts to break asap when it executes in V8? Leaking that implementation
> detail over to V8 sounds like a bad choice. And there is no test case for that
> in V8.
> 
> Since this is only related to the debugger statement, could we simply do the
> check for break_points_active_ in Runtime_HandleDebuggerStatement?
> 

Hm ,thank you for explanation. I thought that this check will help me to don't
change behavior on paused. I'm going to review devtools async stepping
implementation.

For this patch - yes, we could. Moved check.

[Yang, 2016-02-12 20:17:41]

lgtm with nit

https://codereview.chromium.org/1690173002/diff/20001/src/runtime/runtime-deb...
File src/runtime/runtime-debug.cc (right):

https://codereview.chromium.org/1690173002/diff/20001/src/runtime/runtime-deb...
src/runtime/runtime-debug.cc:33: if (isolate->debug()->break_points_active())
Please add curly brackets.

[kozy, 2016-02-12 20:29:08]

Thanks!
All done.

https://codereview.chromium.org/1690173002/diff/20001/src/runtime/runtime-deb...
File src/runtime/runtime-debug.cc (right):

https://codereview.chromium.org/1690173002/diff/20001/src/runtime/runtime-deb...
src/runtime/runtime-debug.cc:33: if (isolate->debug()->break_points_active())
On 2016/02/12 20:17:41, Yang wrote:
> Please add curly brackets.

Done.

[kozy, 2016-02-12 20:29:17]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-02-12 20:29:18]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1690173002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-02-12 20:29:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1690173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1690173002/40001

[commit-bot: I haz the power, 2016-02-12 20:55:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-12 20:55:43]

Description was changed from

==========
Ignore pause on debugger statement when breakpoints are disabled

This behavior was changed in https://codereview.chromium.org/1402913002.
It's pretty usefull to have ability to disable debugger statement for our users.

BUG=chromium:583515
LOG=N
R=yangguo@chromium.org
==========

to

==========
Ignore pause on debugger statement when breakpoints are disabled

This behavior was changed in https://codereview.chromium.org/1402913002.
It's pretty usefull to have ability to disable debugger statement for our users.

BUG=chromium:583515
LOG=N
R=yangguo@chromium.org

Committed: https://crrev.com/4b54c07dd11c99f3532fb059eaac4e4edba22dab
Cr-Commit-Position: refs/heads/master@{#33960}
==========

[commit-bot: I haz the power, 2016-02-12 20:55:43]

Patchset 3 (id:??) landed as
https://crrev.com/4b54c07dd11c99f3532fb059eaac4e4edba22dab
Cr-Commit-Position: refs/heads/master@{#33960}