Debugger: preserve stepping state after evaluating breakpoint condition.

Debugger: preserve stepping state after evaluating breakpoint condition.

R=ulan@chromium.org, yurys@chromium.org
BUG=chromium:467180
LOG=N

Committed: https://crrev.com/ee6666a55afa4241c93a7023e5931c19b2a9aeeb
Cr-Commit-Position: refs/heads/master@{#28432}

[yurys, 2015-05-15 22:04:42]

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

https://codereview.chromium.org/1132643004/diff/1/src/debug.cc#newcode903
src/debug.cc:903: PreserveDebugState state(this);
I wonder how evaluating breakpoint condition can change the stepping state. The
conditional expression should be isolated from the debugger bindings and I
wouldn't expect it to affect the state. Could you educate me on this?

[Yang, 2015-05-15 22:06:18]

On 2015/05/15 22:04:42, yurys_slow wrote:
> https://codereview.chromium.org/1132643004/diff/1/src/debug.cc
> File src/debug.cc (right):
> 
> https://codereview.chromium.org/1132643004/diff/1/src/debug.cc#newcode903
> src/debug.cc:903: PreserveDebugState state(this);
> I wonder how evaluating breakpoint condition can change the stepping state.
The
> conditional expression should be isolated from the debugger bindings and I
> wouldn't expect it to affect the state. Could you educate me on this?

the condition is evaluated via DebugEvaluate, which clears stepping upfront. We
want this so that we don't break while we break :)

[yurys, 2015-05-15 22:11:57]

On 2015/05/15 22:06:18, Yang wrote:
> On 2015/05/15 22:04:42, yurys_slow wrote:
> > https://codereview.chromium.org/1132643004/diff/1/src/debug.cc
> > File src/debug.cc (right):
> > 
> > https://codereview.chromium.org/1132643004/diff/1/src/debug.cc#newcode903
> > src/debug.cc:903: PreserveDebugState state(this);
> > I wonder how evaluating breakpoint condition can change the stepping state.
> The
> > conditional expression should be isolated from the debugger bindings and I
> > wouldn't expect it to affect the state. Could you educate me on this?
> 
> the condition is evaluated via DebugEvaluate, which clears stepping upfront.
We
> want this so that we don't break while we break :)

Oh, now I see. LGTM.

[Yang, 2015-05-15 22:12:51]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-05-15 22:13:06]

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

[yurys, 2015-05-15 22:17:27]

Well, I think if there is e.g. debugger; statement in the condition expression
we will stop above on the stack and may well adjust debugger state. Restoring it
completely may be confusing.

[commit-bot: I haz the power, 2015-05-15 22:51:14]

Committed patchset #1 (id:1)

[Michael Achenbach, 2015-05-17 06:21:28]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1130123007/ by machenbach@chromium.org.

The reason for reverting is: [Sheriff] This breaks TSAN (makes some tests marked
as flaky permanently fail):
http://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN/builds/3882.

[commit-bot: I haz the power, 2015-05-18 11:03:47]

Patchset 1 (id:??) landed as
https://crrev.com/ee6666a55afa4241c93a7023e5931c19b2a9aeeb
Cr-Commit-Position: refs/heads/master@{#28432}