inspector: support extra contexts

This enables inspector support for contexts created using the vm module. Fixes: nodejs#7593 Fixes: nodejs#12096 Refs: nodejs#9272
jgoz · Jun 20, 2017 · a4d4715 · a4d4715
1 parent 1fcb76e
commit a4d4715
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 1 deletion.
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -259,6 +259,15 @@ inline void Environment::TickInfo::set_index(uint32_t value) {
 
 inline void Environment::AssignToContext(v8::Local<v8::Context> context) {
   context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
+#if HAVE_INSPECTOR
+  inspector_agent()->ContextCreated(context);
+#endif  // HAVE_INSPECTOR
+}
+
+inline void Environment::UnassignFromContext(v8::Local<v8::Context> context) {
+#if HAVE_INSPECTOR
+  inspector_agent()->ContextDestroyed(context);
+#endif  // HAVE_INSPECTOR
 }
 
 inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {

diff --git a/src/env.h b/src/env.h
@@ -530,6 +530,7 @@ class Environment {
              const char* const* exec_argv,
              bool start_profiler_idle_notifier);
   void AssignToContext(v8::Local<v8::Context> context);
+  void UnassignFromContext(v8::Local<v8::Context> context);
   void CleanupHandles();
 
   void StartProfilerIdleNotifier();

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -12,6 +12,7 @@
 #include "libplatform/libplatform.h"
 
 #include <string.h>
+#include <sstream>
 #include <vector>
 
 #ifdef __POSIX__
@@ -539,7 +540,8 @@ class NodeInspectorClient : public v8_inspector::V8InspectorClient {
 Agent::Agent(Environment* env) : parent_env_(env),
                                  client_(nullptr),
                                  platform_(nullptr),
-                                 enabled_(false) {}
+                                 enabled_(false),
+                                 next_context_number_(1) {}
 
 // Destructor needs to be defined here in implementation file as the header
 // does not have full definition of some classes.
@@ -669,6 +671,19 @@ void Agent::PauseOnNextJavascriptStatement(const std::string& reason) {
     channel->schedulePauseOnNextStatement(reason);
 }
 
+void Agent::ContextCreated(Local<Context> context) {
+  if (!IsStarted()) // This happens for a main context
+    return;
+  std::ostringstream name;
+  name << "VM Context " << next_context_number_++;
+  client_->contextCreated(context, name.str());
+}
+
+void Agent::ContextDestroyed(Local<Context> context) {
+  CHECK_NE(client_, nullptr);
+  client_->contextDestroyed(context);
+}
+
 void Open(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   inspector::Agent* agent = env->inspector_agent();

diff --git a/src/inspector_agent.h b/src/inspector_agent.h
@@ -79,6 +79,9 @@ class Agent {
   bool enabled() { return enabled_; }
   void PauseOnNextJavascriptStatement(const std::string& reason);
 
+  void ContextCreated(v8::Local<v8::Context> context);
+  void ContextDestroyed(v8::Local<v8::Context> context);
+
   // Initialize 'inspector' module bindings
   static void InitInspector(v8::Local<v8::Object> target,
                             v8::Local<v8::Value> unused,
@@ -105,6 +108,7 @@ class Agent {
   bool enabled_;
   std::string path_;
   DebugOptions debug_options_;
+  int next_context_number_;
 };
 
 }  // namespace inspector

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -357,6 +357,8 @@ class ContextifyContext {
 
   static void WeakCallback(const WeakCallbackInfo<ContextifyContext>& data) {
     ContextifyContext* context = data.GetParameter();
+    context->env()->UnassignFromContext(context->context());
+
     delete context;
   }
 

diff --git a/test/inspector/test-contexts.js b/test/inspector/test-contexts.js
@@ -0,0 +1,47 @@
+'use strict';
+const common = require('../common');
+common.skipIfInspectorDisabled();
+const assert = require('assert');
+const helper = require('./inspector-helper.js');
+
+function setupExpectBreakInContext() {
+  return function(message) {
+    if ('Debugger.paused' === message['method']) {
+      const callFrame = message['params']['callFrames'][0];
+      assert.strictEqual(callFrame['functionName'], 'testfn');
+      return true;
+    }
+  };
+}
+
+function testBreakpointInContext(session) {
+  console.log('[test]',
+              'Verifying debugger stops on start (--inspect-brk option)');
+  const commands = [
+    { 'method': 'Runtime.enable' },
+    { 'method': 'Debugger.enable' },
+    { 'method': 'Runtime.runIfWaitingForDebugger' }
+  ];
+  session
+    .sendInspectorCommands(commands)
+    .expectMessages(setupExpectBreakInContext());
+}
+
+function resume(session) {
+  session
+    .sendInspectorCommands({ 'method': 'Debugger.resume'})
+    .disconnect(true);
+}
+
+function runTests(harness) {
+  harness
+    .runFrontendSession([
+      testBreakpointInContext,
+      resume,
+    ]).expectShutDown(0);
+}
+
+const script = 'require("vm").runInNewContext(' +
+               '"function testfn() {debugger};testfn();")';
+
+helper.startNodeForInspectorTest(runTests, '--inspect-brk', script);