Don't create debug context if debug listener is not set

Don't create debug context if debug listener is not set

If there had been no debug listener v8::Debug::GetDebugContext would have created new context and wouln't have kept reference to it. This way we may well end up with several debug contexts and disabled debugger.

As a side effect this change allows to efficiently distinguish debug context from blink contexts by simply comparing handles.

BUG=chromium:482290
LOG=Y

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

[yurys, 2015-05-08 14:11:52]

The CQ bit was checked by yurys@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-08 14:12:02]

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

[yurys, 2015-05-08 14:13:36]

yurys@chromium.org changed reviewers:
+ haraken@chromium.org, kozyatinskiy@chromium.org, yangguo@chromium.org

[commit-bot: I haz the power, 2015-05-08 14:22:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-08 14:22:02]

Dry run: Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/2571)

[Yang, 2015-05-11 06:44:19]

lgtm if test fixed.

https://codereview.chromium.org/1136733002/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-3960.js (right):

https://codereview.chromium.org/1136733002/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-3960.js:5: // Flags: --allow-natives-syntax
--expose-debug-as debug
This makes this test pointless (see comment). Since %GetDebugContext is only
used in tests, please change it to set a dummy listener to activate the debugger
before getting the debug context (and reset it after).

[yurys, 2015-05-12 06:46:59]

https://codereview.chromium.org/1136733002/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-3960.js (right):

https://codereview.chromium.org/1136733002/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-3960.js:5: // Flags: --allow-natives-syntax
--expose-debug-as debug
On 2015/05/11 06:44:19, Yang wrote:
> This makes this test pointless (see comment). Since %GetDebugContext is only
> used in tests, please change it to set a dummy listener to activate the
debugger
> before getting the debug context (and reset it after).

Done. I added DebugScope to Runtime_GetDebugContext.

This sounds weird to me that we allow setting breakpoints when there is no debug
listener. I'd rather we cleared all breakpoints on removing debug listener and
require the client to restore all breakpoints at the beginning of the next debug
session. With current behavior we can set breakpoints during the first debug
session and then hit them in the second session without even knowing that there
are breakpoints. I vaguely recall a bug on this in WebKit but I cannot find it
at the moment.

[yurys, 2015-05-12 06:47:23]

The CQ bit was checked by yurys@chromium.org

[yurys, 2015-05-12 06:47:24]

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/1136733002/#ps20001
(title: "Addressed review comment")

[commit-bot: I haz the power, 2015-05-12 06:47:35]

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

[Yang, 2015-05-12 06:53:19]

On 2015/05/12 06:46:59, yurys wrote:
>
https://codereview.chromium.org/1136733002/diff/1/test/mjsunit/regress/regres...
> File test/mjsunit/regress/regress-3960.js (right):
> 
>
https://codereview.chromium.org/1136733002/diff/1/test/mjsunit/regress/regres...
> test/mjsunit/regress/regress-3960.js:5: // Flags: --allow-natives-syntax
> --expose-debug-as debug
> On 2015/05/11 06:44:19, Yang wrote:
> > This makes this test pointless (see comment). Since %GetDebugContext is only
> > used in tests, please change it to set a dummy listener to activate the
> debugger
> > before getting the debug context (and reset it after).
> 
> Done. I added DebugScope to Runtime_GetDebugContext.
> 
> This sounds weird to me that we allow setting breakpoints when there is no
debug
> listener. I'd rather we cleared all breakpoints on removing debug listener and
> require the client to restore all breakpoints at the beginning of the next
debug
> session. With current behavior we can set breakpoints during the first debug
> session and then hit them in the second session without even knowing that
there
> are breakpoints. I vaguely recall a bug on this in WebKit but I cannot find it
> at the moment.

Yes, we should not allow setting break points when no listener is attached.
However, there are a few tests that still do this, and I had no time yet to
address this issue.

The regression test (regress-3960.js) however simply tests that setting break
point works if the debugger is activated late, i.e. not at bootstrapping.

[commit-bot: I haz the power, 2015-05-12 07:33:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-12 07:33:36]

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