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

Review URL: https://codereview.chromium.org/1136733002

Cr-Commit-Position: refs/heads/master@{#28356}

include/v8-debug.h

@@ -248,7 +248,8 @@ class V8_EXPORT Debug {
    * Debugger is running in its own context which is entered while debugger
    * messages are being dispatched. This is an explicit getter for this
    * debugger context. Note that the content of the debugger context is subject
-   * to change.
+   * to change. The Context exists only when the debugger is active, i.e. at
+   * least one DebugEventListener or MessageHandler is set.
    */
   static Local<Context> GetDebugContext();
 

src/api.cc

@@ -7384,7 +7384,7 @@ void Debug::ProcessDebugMessages() {
 Local<Context> Debug::GetDebugContext() {
   i::Isolate* isolate = i::Isolate::Current();
   ENTER_V8(isolate);
-  return Utils::ToLocal(i::Isolate::Current()->debug()->GetDebugContext());
+  return Utils::ToLocal(isolate->debug()->GetDebugContext());
 }
 
 

src/debug.cc

@@ -2803,6 +2803,7 @@ void Debug::ProcessCompileEventInDebugScope(v8::DebugEvent event,
 
 
 Handle<Context> Debug::GetDebugContext() {
+  if (!is_loaded()) return Handle<Context>();
   DebugScope debug_scope(this);
   if (debug_scope.failed()) return Handle<Context>();
   // The global handle may be destroyed soon after.  Return it reboxed.

src/runtime/runtime-debug.cc

@@ -2833,7 +2833,11 @@ RUNTIME_FUNCTION(Runtime_ExecuteInDebugContext) {
 RUNTIME_FUNCTION(Runtime_GetDebugContext) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 0);
-  Handle<Context> context = isolate->debug()->GetDebugContext();
+  Handle<Context> context;
+  {
+    DebugScope debug_scope(isolate->debug());
+    context = isolate->debug()->GetDebugContext();
+  }
   if (context.is_null()) return isolate->heap()->undefined_value();
   context->set_security_token(isolate->native_context()->security_token());
   return context->global_proxy();

test/cctest/test-debug.cc

@@ -6925,6 +6925,13 @@ TEST(DebugContextIsPreservedBetweenAccesses) {
 }
 
 
+TEST(NoDebugContextWhenDebuggerDisabled) {
+  v8::HandleScope scope(CcTest::isolate());
+  v8::Local<v8::Context> context = v8::Debug::GetDebugContext();
+  CHECK(context.IsEmpty());
+}
+
+
 static v8::Handle<v8::Value> expected_callback_data;
 static void DebugEventContextChecker(const v8::Debug::EventDetails& details) {
   CHECK(details.GetEventContext() == expected_context);

test/mjsunit/regress/regress-3960.js

@@ -4,7 +4,7 @@
 
 // Flags: --allow-natives-syntax
 
-// Test that setting break point is works correctly when the debugger is
+// Test that setting break point works correctly when the debugger is
 // activated late, which leads to duplicate shared function infos.
 
 (function() {