Skip to content

Commit f66e9ab

Browse files
alexkozytargos
authored andcommitted
inspector: implemented V8InspectorClient::resourceNameToUrl
This method is required by inspector to report normalized urls over the protocol. Backport-PR-URL: #22918Fixes#22223 PR-URL: #22251 Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
1 parent fa83382 commit f66e9ab

10 files changed

+90
-16
lines changed

‎src/inspector_agent.cc‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include"inspector/tracing_agent.h"
77
#include"node/inspector/protocol/Protocol.h"
88
#include"node_internals.h"
9+
#include"node_url.h"
910
#include"v8-inspector.h"
1011
#include"v8-platform.h"
1112

@@ -375,6 +376,25 @@ void NotifyClusterWorkersDebugEnabled(Environment* env){
375376
MakeCallback(env->isolate(), process_object, emit_fn.As<Function>(),
376377
arraysize(argv), argv,{0, 0});
377378
}
379+
380+
#ifdef _WIN32
381+
boolIsFilePath(const std::string& path){
382+
// '\\'
383+
if (path.length() > 2 && path[0] == '\\' && path[1] == '\\')
384+
returntrue;
385+
// '[A-Z]:[/\\]'
386+
if (path.length() < 3)
387+
returnfalse;
388+
if ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z'))
389+
return path[1] == ':' && (path[2] == '/' || path[2] == '\\');
390+
returnfalse;
391+
}
392+
#else
393+
boolIsFilePath(const std::string& path){
394+
return path.length() && path[0] == '/';
395+
}
396+
#endif// __POSIX__
397+
378398
} // namespace
379399

380400
classNodeInspectorClient : publicV8InspectorClient{
@@ -601,6 +621,18 @@ class NodeInspectorClient : public V8InspectorClient{
601621
return env_->isolate_data()->platform()->CurrentClockTimeMillis();
602622
}
603623

624+
std::unique_ptr<StringBuffer> resourceNameToUrl(
625+
const StringView& resource_name_view) override{
626+
std::string resource_name =
627+
protocol::StringUtil::StringViewToUtf8(resource_name_view);
628+
if (!IsFilePath(resource_name))
629+
returnnullptr;
630+
node::url::URL url = node::url::URL::FromFilePath(resource_name);
631+
// TODO(ak239spb): replace this code with url.href().
632+
// Refs: https://github.com/nodejs/node/issues/22610
633+
returnUtf8ToStringView(url.protocol() + "//" + url.path());
634+
}
635+
604636
node::Environment* env_;
605637
bool is_main_;
606638
bool running_nested_loop_ = false;

‎test/cctest/test_url.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,6 @@ TEST_F(URLTest, FromFilePath){
124124
file_url = URL::FromFilePath("b:\\a\\%%.js");
125125
EXPECT_EQ("file:", file_url.protocol());
126126
EXPECT_EQ("/b:/a/%25%25.js", file_url.path());
127-
128-
file_url = URL::FromFilePath("\\\\host\\a\\b\\c");
129-
EXPECT_EQ("file:", file_url.protocol());
130-
EXPECT_EQ("host/a/b/c", file_url.path());
131127
#else
132128
file_url = URL::FromFilePath("/");
133129
EXPECT_EQ("file:", file_url.protocol());

‎test/parallel/test-inspector-multisession-js.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
constcommon=require('../common');
34

@@ -6,6 +7,7 @@ common.skipIfInspectorDisabled();
67
constassert=require('assert');
78
const{ Session }=require('inspector');
89
constpath=require('path');
10+
const{ pathToFileURL }=require('url');
911

1012
functiondebugged(){
1113
return42;
@@ -32,8 +34,8 @@ async function test(){
3234

3335
awaitnewPromise((resolve,reject)=>{
3436
session1.post('Debugger.setBreakpointByUrl',{
35-
'lineNumber': 9,
36-
'url': path.resolve(__dirname,__filename),
37+
'lineNumber': 12,
38+
'url': pathToFileURL(path.resolve(__dirname,__filename)).toString(),
3739
'columnNumber': 0,
3840
'condition': ''
3941
},(error,result)=>{

‎test/parallel/test-v8-coverage.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ function getFixtureCoverage(fixtureFile, coverageDirectory){
9797
for(constcoverageFileofcoverageFiles){
9898
constcoverage=require(path.join(coverageDirectory,coverageFile));
9999
for(constfixtureCoverageofcoverage.result){
100-
if(fixtureCoverage.url.indexOf(`${path.sep}${fixtureFile}`)!==-1){
100+
if(fixtureCoverage.url.indexOf(`/${fixtureFile}`)!==-1){
101101
returnfixtureCoverage;
102102
}
103103
}

‎test/sequential/test-inspector-bindings.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
// Flags: --expose-internals
12
'use strict';
23
constcommon=require('../common');
34
common.skipIfInspectorDisabled();
45
constassert=require('assert');
56
constinspector=require('inspector');
67
constpath=require('path');
8+
const{ pathToFileURL }=require('url');
79

810
// This test case will set a breakpoint 4 lines below
911
functiondebuggedFunction(){
@@ -84,8 +86,8 @@ function testSampleDebugSession(){
8486
},TypeError);
8587
session.post('Debugger.enable',()=>cbAsSecondArgCalled=true);
8688
session.post('Debugger.setBreakpointByUrl',{
87-
'lineNumber': 12,
88-
'url': path.resolve(__dirname,__filename),
89+
'lineNumber': 14,
90+
'url': pathToFileURL(path.resolve(__dirname,__filename)).toString(),
8991
'columnNumber': 0,
9092
'condition': ''
9193
});

‎test/sequential/test-inspector-break-when-eval.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ common.skipIfInspectorDisabled();
55
constassert=require('assert');
66
const{ NodeInstance }=require('../common/inspector-helper.js');
77
constfixtures=require('../common/fixtures');
8+
const{ pathToFileURL }=require('url');
89

910
constscript=fixtures.path('inspector-global-function.js');
1011

@@ -26,7 +27,7 @@ async function breakOnLine(session){
2627
constcommands=[
2728
{'method': 'Debugger.setBreakpointByUrl',
2829
'params': {'lineNumber': 9,
29-
'url': script,
30+
'url': pathToFileURL(script).toString(),
3031
'columnNumber': 0,
3132
'condition': ''
3233
}
@@ -45,7 +46,7 @@ async function breakOnLine(session){
4546
}
4647
];
4748
session.send(commands);
48-
awaitsession.waitForBreakOnLine(9,script);
49+
awaitsession.waitForBreakOnLine(9,pathToFileURL(script).toString());
4950
}
5051

5152
asyncfunctionstepOverConsoleStatement(session){

‎test/sequential/test-inspector-debug-brk-flag.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ async function testBreakpointOnStart(session){
2424
];
2525

2626
session.send(commands);
27-
awaitsession.waitForBreakOnLine(0,session.scriptPath());
27+
awaitsession.waitForBreakOnLine(0,session.scriptURL());
2828
}
2929

3030
asyncfunctionrunTests(){

‎test/sequential/test-inspector-exception.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ common.skipIfInspectorDisabled();
77

88
constassert=require('assert');
99
const{ NodeInstance }=require('../common/inspector-helper.js');
10+
const{ pathToFileURL }=require('url');
1011

1112
constscript=fixtures.path('throws_error.js');
1213

@@ -29,7 +30,7 @@ async function testBreakpointOnStart(session){
2930
];
3031

3132
awaitsession.send(commands);
32-
awaitsession.waitForBreakOnLine(0,script);
33+
awaitsession.waitForBreakOnLine(0,pathToFileURL(script).toString());
3334
}
3435

3536

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
4+
common.skipIfInspectorDisabled();
5+
6+
(asyncfunctiontest(){
7+
const{ strictEqual }=require('assert');
8+
const{ Session }=require('inspector');
9+
const{ promisify }=require('util');
10+
constvm=require('vm');
11+
constsession=newSession();
12+
session.connect();
13+
session.post=promisify(session.post);
14+
awaitsession.post('Debugger.enable');
15+
awaitcheck('http://example.com','http://example.com');
16+
awaitcheck(undefined,'evalmachine.<anonymous>');
17+
awaitcheck('file:///foo.js','file:///foo.js');
18+
awaitcheck('file:///foo.js','file:///foo.js');
19+
awaitcheck('foo.js','foo.js');
20+
awaitcheck('[eval]','[eval]');
21+
awaitcheck('%.js','%.js');
22+
23+
if(common.isWindows){
24+
awaitcheck('C:\\foo.js','file:///C:/foo.js');
25+
awaitcheck('C:\\a\\b\\c\\foo.js','file:///C:/a/b/c/foo.js');
26+
awaitcheck('a:\\%.js','file:///a:/%25.js');
27+
}else{
28+
awaitcheck('/foo.js','file:///foo.js');
29+
awaitcheck('/a/b/c/d/foo.js','file:///a/b/c/d/foo.js');
30+
awaitcheck('/%%%.js','file:///%25%25%25.js');
31+
}
32+
33+
asyncfunctioncheck(filename,expected){
34+
constpromise=
35+
newPromise((resolve)=>session.once('inspectorNotification',resolve));
36+
newvm.Script('42',{ filename }).runInThisContext();
37+
const{params: { url }}=awaitpromise;
38+
strictEqual(url,expected);
39+
}
40+
})();

‎test/sequential/test-inspector.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ async function testBreakpointOnStart(session){
6868
];
6969

7070
awaitsession.send(commands);
71-
awaitsession.waitForBreakOnLine(0,session.scriptPath());
71+
awaitsession.waitForBreakOnLine(0,session.scriptURL());
7272
}
7373

7474
asyncfunctiontestBreakpoint(session){
7575
console.log('[test]','Setting a breakpoint and verifying it is hit');
7676
constcommands=[
7777
{'method': 'Debugger.setBreakpointByUrl',
7878
'params': {'lineNumber': 5,
79-
'url': session.scriptPath(),
79+
'url': session.scriptURL(),
8080
'columnNumber': 0,
8181
'condition': ''
8282
}
@@ -91,7 +91,7 @@ async function testBreakpoint(session){
9191
`Script source is wrong: ${scriptSource}`);
9292

9393
awaitsession.waitForConsoleOutput('log',['A message',5]);
94-
constpaused=awaitsession.waitForBreakOnLine(5,session.scriptPath());
94+
constpaused=awaitsession.waitForBreakOnLine(5,session.scriptURL());
9595
constscopeId=paused.params.callFrames[0].scopeChain[0].object.objectId;
9696

9797
console.log('[test]','Verify we can read current application state');

0 commit comments

Comments
(0)