inspector: implemented V8InspectorClient::resourceNameToUrl

alexkozy · targos · commit f66e9abcb365 · 2018-09-25T08:57:57.000+02:00
This method is required by inspector to report normalized urls over the protocol. Backport-PR-URL: #22918 Fixes #22223 PR-URL: #22251 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -6,6 +6,7 @@
 #include "inspector/tracing_agent.h"
 #include "node/inspector/protocol/Protocol.h"
 #include "node_internals.h"
+#include "node_url.h"
 #include "v8-inspector.h"
 #include "v8-platform.h"
 
@@ -375,6 +376,25 @@ void NotifyClusterWorkersDebugEnabled(Environment* env) {
   MakeCallback(env->isolate(), process_object, emit_fn.As<Function>(),
                arraysize(argv), argv, {0, 0});
 }
+
+#ifdef _WIN32
+bool IsFilePath(const std::string& path) {
+  // '\\'
+  if (path.length() > 2 && path[0] == '\\' && path[1] == '\\')
+    return true;
+  // '[A-Z]:[/\\]'
+  if (path.length() < 3)
+    return false;
+  if ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z'))
+    return path[1] == ':' && (path[2] == '/' || path[2] == '\\');
+  return false;
+}
+#else
+bool IsFilePath(const std::string& path) {
+  return path.length() && path[0] == '/';
+}
+#endif  // __POSIX__
+
 }  // namespace
 
 class NodeInspectorClient : public V8InspectorClient {
@@ -601,6 +621,18 @@ class NodeInspectorClient : public V8InspectorClient {
     return env_->isolate_data()->platform()->CurrentClockTimeMillis();
   }
 
+  std::unique_ptr<StringBuffer> resourceNameToUrl(
+      const StringView& resource_name_view) override {
+    std::string resource_name =
+        protocol::StringUtil::StringViewToUtf8(resource_name_view);
+    if (!IsFilePath(resource_name))
+      return nullptr;
+    node::url::URL url = node::url::URL::FromFilePath(resource_name);
+    // TODO(ak239spb): replace this code with url.href().
+    // Refs: https://github.com/nodejs/node/issues/22610
+    return Utf8ToStringView(url.protocol() + "//" + url.path());
+  }
+
   node::Environment* env_;
   bool is_main_;
   bool running_nested_loop_ = false;
diff --git a/test/cctest/test_url.cc b/test/cctest/test_url.cc
@@ -124,10 +124,6 @@ TEST_F(URLTest, FromFilePath) {
   file_url = URL::FromFilePath("b:\\a\\%%.js");
   EXPECT_EQ("file:", file_url.protocol());
   EXPECT_EQ("/b:/a/%25%25.js", file_url.path());
-
-  file_url = URL::FromFilePath("\\\\host\\a\\b\\c");
-  EXPECT_EQ("file:", file_url.protocol());
-  EXPECT_EQ("host/a/b/c", file_url.path());
 #else
   file_url = URL::FromFilePath("/");
   EXPECT_EQ("file:", file_url.protocol());
diff --git a/test/parallel/test-inspector-multisession-js.js b/test/parallel/test-inspector-multisession-js.js
@@ -1,3 +1,4 @@
+// Flags: --expose-internals
 'use strict';
 const common = require('../common');
 
@@ -6,6 +7,7 @@ common.skipIfInspectorDisabled();
 const assert = require('assert');
 const { Session } = require('inspector');
 const path = require('path');
+const { pathToFileURL } = require('url');
 
 function debugged() {
   return 42;
@@ -32,8 +34,8 @@ async function test() {
 
   await new Promise((resolve, reject) => {
     session1.post('Debugger.setBreakpointByUrl', {
-      'lineNumber': 9,
-      'url': path.resolve(__dirname, __filename),
+      'lineNumber': 12,
+      'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
       'columnNumber': 0,
       'condition': ''
     }, (error, result) => {
diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js
@@ -97,7 +97,7 @@ function getFixtureCoverage(fixtureFile, coverageDirectory) {
   for (const coverageFile of coverageFiles) {
     const coverage = require(path.join(coverageDirectory, coverageFile));
     for (const fixtureCoverage of coverage.result) {
-      if (fixtureCoverage.url.indexOf(`${path.sep}${fixtureFile}`) !== -1) {
+      if (fixtureCoverage.url.indexOf(`/${fixtureFile}`) !== -1) {
         return fixtureCoverage;
       }
     }
diff --git a/test/sequential/test-inspector-bindings.js b/test/sequential/test-inspector-bindings.js
@@ -1,9 +1,11 @@
+// Flags: --expose-internals
 'use strict';
 const common = require('../common');
 common.skipIfInspectorDisabled();
 const assert = require('assert');
 const inspector = require('inspector');
 const path = require('path');
+const { pathToFileURL } = require('url');
 
 // This test case will set a breakpoint 4 lines below
 function debuggedFunction() {
@@ -84,8 +86,8 @@ function testSampleDebugSession() {
   }, TypeError);
   session.post('Debugger.enable', () => cbAsSecondArgCalled = true);
   session.post('Debugger.setBreakpointByUrl', {
-    'lineNumber': 12,
-    'url': path.resolve(__dirname, __filename),
+    'lineNumber': 14,
+    'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
     'columnNumber': 0,
     'condition': ''
   });
diff --git a/test/sequential/test-inspector-break-when-eval.js b/test/sequential/test-inspector-break-when-eval.js
@@ -5,6 +5,7 @@ common.skipIfInspectorDisabled();
 const assert = require('assert');
 const { NodeInstance } = require('../common/inspector-helper.js');
 const fixtures = require('../common/fixtures');
+const { pathToFileURL } = require('url');
 
 const script = fixtures.path('inspector-global-function.js');
 
@@ -26,7 +27,7 @@ async function breakOnLine(session) {
   const commands = [
     { 'method': 'Debugger.setBreakpointByUrl',
       'params': { 'lineNumber': 9,
-                  'url': script,
+                  'url': pathToFileURL(script).toString(),
                   'columnNumber': 0,
                   'condition': ''
       }
@@ -45,7 +46,7 @@ async function breakOnLine(session) {
     }
   ];
   session.send(commands);
-  await session.waitForBreakOnLine(9, script);
+  await session.waitForBreakOnLine(9, pathToFileURL(script).toString());
 }
 
 async function stepOverConsoleStatement(session) {
diff --git a/test/sequential/test-inspector-debug-brk-flag.js b/test/sequential/test-inspector-debug-brk-flag.js
@@ -24,7 +24,7 @@ async function testBreakpointOnStart(session) {
   ];
 
   session.send(commands);
-  await session.waitForBreakOnLine(0, session.scriptPath());
+  await session.waitForBreakOnLine(0, session.scriptURL());
 }
 
 async function runTests() {
diff --git a/test/sequential/test-inspector-exception.js b/test/sequential/test-inspector-exception.js
@@ -7,6 +7,7 @@ common.skipIfInspectorDisabled();
 
 const assert = require('assert');
 const { NodeInstance } = require('../common/inspector-helper.js');
+const { pathToFileURL } = require('url');
 
 const script = fixtures.path('throws_error.js');
 
@@ -29,7 +30,7 @@ async function testBreakpointOnStart(session) {
   ];
 
   await session.send(commands);
-  await session.waitForBreakOnLine(0, script);
+  await session.waitForBreakOnLine(0, pathToFileURL(script).toString());
 }
 
 
diff --git a/test/sequential/test-inspector-resource-name-to-url.js b/test/sequential/test-inspector-resource-name-to-url.js
@@ -0,0 +1,40 @@
+'use strict';
+const common = require('../common');
+
+common.skipIfInspectorDisabled();
+
+(async function test() {
+  const { strictEqual } = require('assert');
+  const { Session } = require('inspector');
+  const { promisify } = require('util');
+  const vm = require('vm');
+  const session = new Session();
+  session.connect();
+  session.post = promisify(session.post);
+  await session.post('Debugger.enable');
+  await check('http://example.com', 'http://example.com');
+  await check(undefined, 'evalmachine.<anonymous>');
+  await check('file:///foo.js', 'file:///foo.js');
+  await check('file:///foo.js', 'file:///foo.js');
+  await check('foo.js', 'foo.js');
+  await check('[eval]', '[eval]');
+  await check('%.js', '%.js');
+
+  if (common.isWindows) {
+    await check('C:\\foo.js', 'file:///C:/foo.js');
+    await check('C:\\a\\b\\c\\foo.js', 'file:///C:/a/b/c/foo.js');
+    await check('a:\\%.js', 'file:///a:/%25.js');
+  } else {
+    await check('/foo.js', 'file:///foo.js');
+    await check('/a/b/c/d/foo.js', 'file:///a/b/c/d/foo.js');
+    await check('/%%%.js', 'file:///%25%25%25.js');
+  }
+
+  async function check(filename, expected) {
+    const promise =
+      new Promise((resolve) => session.once('inspectorNotification', resolve));
+    new vm.Script('42', { filename }).runInThisContext();
+    const { params: { url } } = await promise;
+    strictEqual(url, expected);
+  }
+})();
diff --git a/test/sequential/test-inspector.js b/test/sequential/test-inspector.js
@@ -68,15 +68,15 @@ async function testBreakpointOnStart(session) {
   ];
 
   await session.send(commands);
-  await session.waitForBreakOnLine(0, session.scriptPath());
+  await session.waitForBreakOnLine(0, session.scriptURL());
 }
 
 async function testBreakpoint(session) {
   console.log('[test]', 'Setting a breakpoint and verifying it is hit');
   const commands = [
     { 'method': 'Debugger.setBreakpointByUrl',
       'params': { 'lineNumber': 5,
-                  'url': session.scriptPath(),
+                  'url': session.scriptURL(),
                   'columnNumber': 0,
                   'condition': ''
       }
@@ -91,7 +91,7 @@ async function testBreakpoint(session) {
          `Script source is wrong: ${scriptSource}`);
 
   await session.waitForConsoleOutput('log', ['A message', 5]);
-  const paused = await session.waitForBreakOnLine(5, session.scriptPath());
+  const paused = await session.waitForBreakOnLine(5, session.scriptURL());
   const scopeId = paused.params.callFrames[0].scopeChain[0].object.objectId;
 
   console.log('[test]', 'Verify we can read current application state');

Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,7 @@ function getFixtureCoverage(fixtureFile, coverageDirectory) {`
`97`	`97`	`for (const coverageFile of coverageFiles) {`
`98`	`98`	`const coverage = require(path.join(coverageDirectory, coverageFile));`
`99`	`99`	`for (const fixtureCoverage of coverage.result) {`
`100`		- if (fixtureCoverage.url.indexOf(`${path.sep}${fixtureFile}`) !== -1) {
	`100`	+ if (fixtureCoverage.url.indexOf(`/${fixtureFile}`) !== -1) {
`101`	`101`	`return fixtureCoverage;`
`102`	`102`	`}`
`103`	`103`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ async function testBreakpointOnStart(session) {`
`24`	`24`	`];`
`25`	`25`
`26`	`26`	`session.send(commands);`
`27`		`- await session.waitForBreakOnLine(0, session.scriptPath());`
	`27`	`+ await session.waitForBreakOnLine(0, session.scriptURL());`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`async function runTests() {`