From 02180176ec66b87740b27f5a49cee9fd51961808 Mon Sep 17 00:00:00 2001 From: Alexey Kozyatinskiy Date: Fri, 10 Aug 2018 13:09:49 -0700 Subject: [PATCH 1/3] deps: cherry-pick dbfcc48 from upstream V8 Original commit message: ``` [inspector] added V8InspectorClient::resourceNameToUrl Some clients (see Node.js) use platform path as ScriptOrigin. Reporting platform path in protocol makes using protocol much harder. This CL introduced V8InspectorClient::resourceNameToUrl method that is called for any reported using protocol url. V8Inspector uses url internally as well so protocol client may generate pattern for blackboxing with file urls only and does not need to build complicated regexp that covers files urls and platform paths on different platforms. R=lushnikov@chromium.org TBR=yangguo@chromium.org Bug: none Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Iff302e7441df922fa5d689fe510f5a9bfd470b9b Reviewed-on: https://chromium-review.googlesource.com/1164624 Commit-Queue: Aleksey Kozyatinskiy Reviewed-by: Alexei Filippov Cr-Commit-Position: refs/heads/master@{#55029} ``` Refs: v8/v8@dbfcc48 Needed for #22223 --- common.gypi | 2 +- deps/v8/include/v8-inspector.h | 5 + .../src/inspector/v8-debugger-agent-impl.cc | 5 +- deps/v8/src/inspector/v8-debugger-script.cc | 38 +++--- deps/v8/src/inspector/v8-debugger-script.h | 11 +- deps/v8/src/inspector/v8-debugger.cc | 13 +- .../src/inspector/v8-profiler-agent-impl.cc | 46 ++++--- deps/v8/src/inspector/v8-stack-trace-impl.cc | 24 +++- deps/v8/src/inspector/v8-stack-trace-impl.h | 4 +- .../resource-name-to-url-expected.txt | 122 ++++++++++++++++++ .../debugger/resource-name-to-url.js | 49 +++++++ deps/v8/test/inspector/inspector-test.cc | 15 +++ deps/v8/test/inspector/isolate-data.cc | 29 +++++ deps/v8/test/inspector/isolate-data.h | 4 + 14 files changed, 318 insertions(+), 49 deletions(-) create mode 100644 deps/v8/test/inspector/debugger/resource-name-to-url-expected.txt create mode 100644 deps/v8/test/inspector/debugger/resource-name-to-url.js diff --git a/common.gypi b/common.gypi index 25e28c94fa0d0e..ae8f1935d98d83 100644 --- a/common.gypi +++ b/common.gypi @@ -29,7 +29,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.10', + 'v8_embedder_string': '-node.11', # Enable disassembler for `--print-code` v8 options 'v8_enable_disassembler': 1, diff --git a/deps/v8/include/v8-inspector.h b/deps/v8/include/v8-inspector.h index d879a94373e427..ad04d01bd21258 100644 --- a/deps/v8/include/v8-inspector.h +++ b/deps/v8/include/v8-inspector.h @@ -215,6 +215,11 @@ class V8_EXPORT V8InspectorClient { virtual bool canExecuteScripts(int contextGroupId) { return true; } virtual void maxAsyncCallStackDepthChanged(int depth) {} + + virtual std::unique_ptr resourceNameToUrl( + const StringView& resourceName) { + return nullptr; + } }; // These stack trace ids are intended to be passed between debuggers and be diff --git a/deps/v8/src/inspector/v8-debugger-agent-impl.cc b/deps/v8/src/inspector/v8-debugger-agent-impl.cc index e4e6492b67a0c5..d9cb49b1d4d2ef 100644 --- a/deps/v8/src/inspector/v8-debugger-agent-impl.cc +++ b/deps/v8/src/inspector/v8-debugger-agent-impl.cc @@ -1396,7 +1396,7 @@ void V8DebuggerAgentImpl::didParseSource( protocol::StringUtil::parseJSON(inspected->auxData())); } bool isLiveEdit = script->isLiveEdit(); - bool hasSourceURL = script->hasSourceURL(); + bool hasSourceURLComment = script->hasSourceURLComment(); bool isModule = script->isModule(); String16 scriptId = script->scriptId(); String16 scriptURL = script->sourceURL(); @@ -1416,7 +1416,8 @@ void V8DebuggerAgentImpl::didParseSource( Maybe executionContextAuxDataParam( std::move(executionContextAuxData)); const bool* isLiveEditParam = isLiveEdit ? &isLiveEdit : nullptr; - const bool* hasSourceURLParam = hasSourceURL ? &hasSourceURL : nullptr; + const bool* hasSourceURLParam = + hasSourceURLComment ? &hasSourceURLComment : nullptr; const bool* isModuleParam = isModule ? &isModule : nullptr; std::unique_ptr stack = V8StackTraceImpl::capture(m_inspector->debugger(), contextGroupId, 1); diff --git a/deps/v8/src/inspector/v8-debugger-script.cc b/deps/v8/src/inspector/v8-debugger-script.cc index c40477ae2af74d..d861265e148559 100644 --- a/deps/v8/src/inspector/v8-debugger-script.cc +++ b/deps/v8/src/inspector/v8-debugger-script.cc @@ -6,6 +6,7 @@ #include "src/inspector/inspected-context.h" #include "src/inspector/string-util.h" +#include "src/inspector/v8-inspector-impl.h" #include "src/inspector/wasm-translation.h" #include "src/utils.h" @@ -110,9 +111,9 @@ class ActualScript : public V8DebuggerScript { public: ActualScript(v8::Isolate* isolate, v8::Local script, - bool isLiveEdit) + bool isLiveEdit, V8InspectorClient* client) : V8DebuggerScript(isolate, String16::fromInteger(script->Id()), - GetNameOrSourceUrl(script)), + GetScriptURL(script, client)), m_isLiveEdit(isLiveEdit) { Initialize(script); } @@ -218,10 +219,18 @@ class ActualScript : public V8DebuggerScript { } private: - String16 GetNameOrSourceUrl(v8::Local script) { - v8::Local name; - if (script->Name().ToLocal(&name) || script->SourceURL().ToLocal(&name)) - return toProtocolString(name); + String16 GetScriptURL(v8::Local script, + V8InspectorClient* client) { + v8::Local sourceURL; + if (script->SourceURL().ToLocal(&sourceURL) && sourceURL->Length() > 0) + return toProtocolString(sourceURL); + v8::Local v8Name; + if (script->Name().ToLocal(&v8Name) && v8Name->Length() > 0) { + String16 name = toProtocolString(v8Name); + std::unique_ptr url = + client->resourceNameToUrl(toStringView(name)); + return url ? toString16(url->string()) : name; + } return String16(); } @@ -231,7 +240,8 @@ class ActualScript : public V8DebuggerScript { void Initialize(v8::Local script) { v8::Local tmp; - if (script->SourceURL().ToLocal(&tmp)) m_sourceURL = toProtocolString(tmp); + m_hasSourceURLComment = + script->SourceURL().ToLocal(&tmp) && tmp->Length() > 0; if (script->SourceMappingURL().ToLocal(&tmp)) m_sourceMappingURL = toProtocolString(tmp); m_startLine = script->LineOffset(); @@ -398,9 +408,9 @@ class WasmVirtualScript : public V8DebuggerScript { std::unique_ptr V8DebuggerScript::Create( v8::Isolate* isolate, v8::Local scriptObj, - bool isLiveEdit) { + bool isLiveEdit, V8InspectorClient* client) { return std::unique_ptr( - new ActualScript(isolate, scriptObj, isLiveEdit)); + new ActualScript(isolate, scriptObj, isLiveEdit, client)); } std::unique_ptr V8DebuggerScript::CreateWasm( @@ -418,12 +428,11 @@ V8DebuggerScript::V8DebuggerScript(v8::Isolate* isolate, String16 id, V8DebuggerScript::~V8DebuggerScript() {} -const String16& V8DebuggerScript::sourceURL() const { - return m_sourceURL.isEmpty() ? m_url : m_sourceURL; -} - void V8DebuggerScript::setSourceURL(const String16& sourceURL) { - m_sourceURL = sourceURL; + if (sourceURL.length() > 0) { + m_hasSourceURLComment = true; + m_url = sourceURL; + } } bool V8DebuggerScript::setBreakpoint(const String16& condition, @@ -431,5 +440,4 @@ bool V8DebuggerScript::setBreakpoint(const String16& condition, v8::HandleScope scope(m_isolate); return script()->SetBreakpoint(toV8String(m_isolate, condition), loc, id); } - } // namespace v8_inspector diff --git a/deps/v8/src/inspector/v8-debugger-script.h b/deps/v8/src/inspector/v8-debugger-script.h index e0e7d93b202752..38e6448f48d8e8 100644 --- a/deps/v8/src/inspector/v8-debugger-script.h +++ b/deps/v8/src/inspector/v8-debugger-script.h @@ -40,13 +40,14 @@ namespace v8_inspector { // Forward declaration. +class V8InspectorClient; class WasmTranslation; class V8DebuggerScript { public: static std::unique_ptr Create( v8::Isolate* isolate, v8::Local script, - bool isLiveEdit); + bool isLiveEdit, V8InspectorClient* client); static std::unique_ptr CreateWasm( v8::Isolate* isolate, WasmTranslation* wasmTranslation, v8::Local underlyingScript, String16 id, @@ -55,9 +56,9 @@ class V8DebuggerScript { virtual ~V8DebuggerScript(); const String16& scriptId() const { return m_id; } - const String16& url() const { return m_url; } - bool hasSourceURL() const { return !m_sourceURL.isEmpty(); } - const String16& sourceURL() const; + bool hasSourceURLComment() const { return m_hasSourceURLComment; } + const String16& sourceURL() const { return m_url; } + virtual const String16& sourceMappingURL() const = 0; virtual const String16& source() const = 0; virtual const String16& hash() const = 0; @@ -95,7 +96,7 @@ class V8DebuggerScript { String16 m_id; String16 m_url; - String16 m_sourceURL; + bool m_hasSourceURLComment = false; int m_executionContextId = 0; v8::Isolate* m_isolate; diff --git a/deps/v8/src/inspector/v8-debugger.cc b/deps/v8/src/inspector/v8-debugger.cc index 1ceb4210f7f476..7d1f7cefd15db3 100644 --- a/deps/v8/src/inspector/v8-debugger.cc +++ b/deps/v8/src/inspector/v8-debugger.cc @@ -226,13 +226,15 @@ void V8Debugger::getCompiledScripts( v8::Local script = scripts.Get(i); if (!script->WasCompiled()) continue; if (script->IsEmbedded()) { - result.push_back(V8DebuggerScript::Create(m_isolate, script, false)); + result.push_back(V8DebuggerScript::Create(m_isolate, script, false, + m_inspector->client())); continue; } int contextId; if (!script->ContextId().To(&contextId)) continue; if (m_inspector->contextGroupId(contextId) != contextGroupId) continue; - result.push_back(V8DebuggerScript::Create(m_isolate, script, false)); + result.push_back(V8DebuggerScript::Create(m_isolate, script, false, + m_inspector->client())); } } @@ -585,13 +587,14 @@ void V8Debugger::ScriptCompiled(v8::Local script, }); } else if (m_ignoreScriptParsedEventsCounter == 0) { v8::Isolate* isolate = m_isolate; + V8InspectorClient* client = m_inspector->client(); m_inspector->forEachSession( m_inspector->contextGroupId(contextId), - [&isolate, &script, &has_compile_error, - &is_live_edited](V8InspectorSessionImpl* session) { + [&isolate, &script, &has_compile_error, &is_live_edited, + &client](V8InspectorSessionImpl* session) { if (!session->debuggerAgent()->enabled()) return; session->debuggerAgent()->didParseSource( - V8DebuggerScript::Create(isolate, script, is_live_edited), + V8DebuggerScript::Create(isolate, script, is_live_edited, client), !has_compile_error); }); } diff --git a/deps/v8/src/inspector/v8-profiler-agent-impl.cc b/deps/v8/src/inspector/v8-profiler-agent-impl.cc index 59a99d79d54c2f..f14815fdc4b031 100644 --- a/deps/v8/src/inspector/v8-profiler-agent-impl.cc +++ b/deps/v8/src/inspector/v8-profiler-agent-impl.cc @@ -7,6 +7,7 @@ #include #include "src/base/atomicops.h" +#include "src/debug/debug-interface.h" #include "src/flags.h" // TODO(jgruber): Remove include and DEPS entry. #include "src/inspector/protocol/Protocol.h" #include "src/inspector/string-util.h" @@ -31,6 +32,15 @@ static const char typeProfileStarted[] = "typeProfileStarted"; namespace { +String16 resourceNameToUrl(V8InspectorImpl* inspector, + v8::Local v8Name) { + String16 name = toProtocolString(v8Name); + if (!inspector) return name; + std::unique_ptr url = + inspector->client()->resourceNameToUrl(toStringView(name)); + return url ? toString16(url->string()) : name; +} + std::unique_ptr> buildInspectorObjectForPositionTicks(const v8::CpuProfileNode* node) { unsigned lineCount = node->GetHitLineCount(); @@ -51,13 +61,14 @@ buildInspectorObjectForPositionTicks(const v8::CpuProfileNode* node) { } std::unique_ptr buildInspectorObjectFor( - v8::Isolate* isolate, const v8::CpuProfileNode* node) { + V8InspectorImpl* inspector, const v8::CpuProfileNode* node) { + v8::Isolate* isolate = inspector->isolate(); v8::HandleScope handleScope(isolate); auto callFrame = protocol::Runtime::CallFrame::create() .setFunctionName(toProtocolString(node->GetFunctionName())) .setScriptId(String16::fromInteger(node->GetScriptId())) - .setUrl(toProtocolString(node->GetScriptResourceName())) + .setUrl(resourceNameToUrl(inspector, node->GetScriptResourceName())) .setLineNumber(node->GetLineNumber() - 1) .setColumnNumber(node->GetColumnNumber() - 1) .build(); @@ -107,18 +118,19 @@ std::unique_ptr> buildInspectorObjectForTimestamps( return array; } -void flattenNodesTree(v8::Isolate* isolate, const v8::CpuProfileNode* node, +void flattenNodesTree(V8InspectorImpl* inspector, + const v8::CpuProfileNode* node, protocol::Array* list) { - list->addItem(buildInspectorObjectFor(isolate, node)); + list->addItem(buildInspectorObjectFor(inspector, node)); const int childrenCount = node->GetChildrenCount(); for (int i = 0; i < childrenCount; i++) - flattenNodesTree(isolate, node->GetChild(i), list); + flattenNodesTree(inspector, node->GetChild(i), list); } std::unique_ptr createCPUProfile( - v8::Isolate* isolate, v8::CpuProfile* v8profile) { + V8InspectorImpl* inspector, v8::CpuProfile* v8profile) { auto nodes = protocol::Array::create(); - flattenNodesTree(isolate, v8profile->GetTopDownRoot(), nodes.get()); + flattenNodesTree(inspector, v8profile->GetTopDownRoot(), nodes.get()); return protocol::Profiler::Profile::create() .setNodes(std::move(nodes)) .setStartTime(static_cast(v8profile->GetStartTime())) @@ -320,7 +332,7 @@ std::unique_ptr createCoverageRange( } Response coverageToProtocol( - v8::Isolate* isolate, const v8::debug::Coverage& coverage, + V8InspectorImpl* inspector, const v8::debug::Coverage& coverage, std::unique_ptr>* out_result) { std::unique_ptr> result = @@ -361,8 +373,10 @@ Response coverageToProtocol( } String16 url; v8::Local name; - if (script->Name().ToLocal(&name) || script->SourceURL().ToLocal(&name)) { + if (script->SourceURL().ToLocal(&name) && name->Length()) { url = toProtocolString(name); + } else if (script->Name().ToLocal(&name) && name->Length()) { + url = resourceNameToUrl(inspector, name); } result->addItem(protocol::Profiler::ScriptCoverage::create() .setScriptId(String16::fromInteger(script->Id())) @@ -384,7 +398,7 @@ Response V8ProfilerAgentImpl::takePreciseCoverage( } v8::HandleScope handle_scope(m_isolate); v8::debug::Coverage coverage = v8::debug::Coverage::CollectPrecise(m_isolate); - return coverageToProtocol(m_isolate, coverage, out_result); + return coverageToProtocol(m_session->inspector(), coverage, out_result); } Response V8ProfilerAgentImpl::getBestEffortCoverage( @@ -393,12 +407,12 @@ Response V8ProfilerAgentImpl::getBestEffortCoverage( v8::HandleScope handle_scope(m_isolate); v8::debug::Coverage coverage = v8::debug::Coverage::CollectBestEffort(m_isolate); - return coverageToProtocol(m_isolate, coverage, out_result); + return coverageToProtocol(m_session->inspector(), coverage, out_result); } namespace { std::unique_ptr> -typeProfileToProtocol(v8::Isolate* isolate, +typeProfileToProtocol(V8InspectorImpl* inspector, const v8::debug::TypeProfile& type_profile) { std::unique_ptr> result = protocol::Array::create(); @@ -426,8 +440,10 @@ typeProfileToProtocol(v8::Isolate* isolate, } String16 url; v8::Local name; - if (script->Name().ToLocal(&name) || script->SourceURL().ToLocal(&name)) { + if (script->SourceURL().ToLocal(&name) && name->Length()) { url = toProtocolString(name); + } else if (script->Name().ToLocal(&name) && name->Length()) { + url = resourceNameToUrl(inspector, name); } result->addItem(protocol::Profiler::ScriptTypeProfile::create() .setScriptId(String16::fromInteger(script->Id())) @@ -462,7 +478,7 @@ Response V8ProfilerAgentImpl::takeTypeProfile( v8::HandleScope handle_scope(m_isolate); v8::debug::TypeProfile type_profile = v8::debug::TypeProfile::Collect(m_isolate); - *out_result = typeProfileToProtocol(m_isolate, type_profile); + *out_result = typeProfileToProtocol(m_session->inspector(), type_profile); return Response::OK(); } @@ -491,7 +507,7 @@ std::unique_ptr V8ProfilerAgentImpl::stopProfiling( m_profiler->StopProfiling(toV8String(m_isolate, title)); std::unique_ptr result; if (profile) { - if (serialize) result = createCPUProfile(m_isolate, profile); + if (serialize) result = createCPUProfile(m_session->inspector(), profile); profile->Delete(); } --m_startedProfilesCount; diff --git a/deps/v8/src/inspector/v8-stack-trace-impl.cc b/deps/v8/src/inspector/v8-stack-trace-impl.cc index 75293c59afee8e..9be0d4fa385f2a 100644 --- a/deps/v8/src/inspector/v8-stack-trace-impl.cc +++ b/deps/v8/src/inspector/v8-stack-trace-impl.cc @@ -7,6 +7,7 @@ #include #include "src/inspector/v8-debugger.h" +#include "src/inspector/v8-inspector-impl.h" #include "src/inspector/wasm-translation.h" namespace v8_inspector { @@ -73,7 +74,10 @@ std::unique_ptr buildInspectorObjectCommon( std::unique_ptr> inspectorFrames = protocol::Array::create(); for (size_t i = 0; i < frames.size(); i++) { - inspectorFrames->addItem(frames[i]->buildInspectorObject()); + V8InspectorClient* client = nullptr; + if (debugger && debugger->inspector()) + client = debugger->inspector()->client(); + inspectorFrames->addItem(frames[i]->buildInspectorObject(client)); } std::unique_ptr stackTrace = protocol::Runtime::StackTrace::create() @@ -117,7 +121,9 @@ StackFrame::StackFrame(v8::Local v8Frame) m_scriptId(String16::fromInteger(v8Frame->GetScriptId())), m_sourceURL(toProtocolString(v8Frame->GetScriptNameOrSourceURL())), m_lineNumber(v8Frame->GetLineNumber() - 1), - m_columnNumber(v8Frame->GetColumn() - 1) { + m_columnNumber(v8Frame->GetColumn() - 1), + m_hasSourceURLComment(v8Frame->GetScriptName() != + v8Frame->GetScriptNameOrSourceURL()) { DCHECK_NE(v8::Message::kNoLineNumberInfo, m_lineNumber + 1); DCHECK_NE(v8::Message::kNoColumnInfo, m_columnNumber + 1); } @@ -137,12 +143,20 @@ int StackFrame::lineNumber() const { return m_lineNumber; } int StackFrame::columnNumber() const { return m_columnNumber; } -std::unique_ptr StackFrame::buildInspectorObject() - const { +std::unique_ptr StackFrame::buildInspectorObject( + V8InspectorClient* client) const { + String16 frameUrl = m_sourceURL; + if (client && !m_hasSourceURLComment && frameUrl.length() > 0) { + std::unique_ptr url = + client->resourceNameToUrl(toStringView(m_sourceURL)); + if (url) { + frameUrl = toString16(url->string()); + } + } return protocol::Runtime::CallFrame::create() .setFunctionName(m_functionName) .setScriptId(m_scriptId) - .setUrl(m_sourceURL) + .setUrl(frameUrl) .setLineNumber(m_lineNumber) .setColumnNumber(m_columnNumber) .build(); diff --git a/deps/v8/src/inspector/v8-stack-trace-impl.h b/deps/v8/src/inspector/v8-stack-trace-impl.h index a8f23c48b67231..019fd469cdd72e 100644 --- a/deps/v8/src/inspector/v8-stack-trace-impl.h +++ b/deps/v8/src/inspector/v8-stack-trace-impl.h @@ -33,7 +33,8 @@ class StackFrame { const String16& sourceURL() const; int lineNumber() const; // 0-based. int columnNumber() const; // 0-based. - std::unique_ptr buildInspectorObject() const; + std::unique_ptr buildInspectorObject( + V8InspectorClient* client) const; bool isEqual(StackFrame* frame) const; private: @@ -42,6 +43,7 @@ class StackFrame { String16 m_sourceURL; int m_lineNumber; // 0-based. int m_columnNumber; // 0-based. + bool m_hasSourceURLComment; }; class V8StackTraceImpl : public V8StackTrace { diff --git a/deps/v8/test/inspector/debugger/resource-name-to-url-expected.txt b/deps/v8/test/inspector/debugger/resource-name-to-url-expected.txt new file mode 100644 index 00000000000000..2e6e589b2517d3 --- /dev/null +++ b/deps/v8/test/inspector/debugger/resource-name-to-url-expected.txt @@ -0,0 +1,122 @@ +Tests V8InspectorClient::resourceNameToUrl. +Check script with url: +{ + method : Debugger.scriptParsed + params : { + endColumn : 16 + endLine : 0 + executionContextId : + hasSourceURL : false + hash : 033b33d191ed51ed823355d865eb871d811403e2 + isLiveEdit : false + isModule : false + length : 16 + scriptId : + sourceMapURL : + startColumn : 0 + startLine : 0 + url : prefix://url + } +} +Check script with sourceURL comment: +{ + method : Debugger.scriptParsed + params : { + endColumn : 37 + endLine : 0 + executionContextId : + hasSourceURL : true + hash : 06c136ce206c5f505f32af524e6ec71b5baa0bbb + isLiveEdit : false + isModule : false + length : 37 + scriptId : + sourceMapURL : + startColumn : 0 + startLine : 0 + url : foo.js + } +} +Check script failed to parse: +{ + method : Debugger.scriptFailedToParse + params : { + endColumn : 15 + endLine : 0 + executionContextId : + hasSourceURL : false + hash : 033b33d191ed51ed1f44cd0465eb871d811403e2 + isModule : false + length : 15 + scriptId : + sourceMapURL : + startColumn : 0 + startLine : 0 + url : prefix://url + } +} +Check script failed to parse with sourceURL comment: +{ + method : Debugger.scriptFailedToParse + params : { + endColumn : 36 + endLine : 0 + executionContextId : + hasSourceURL : true + hash : 23a2885951475580023e2a742563d78876d8f05e + isModule : false + length : 36 + scriptId : + sourceMapURL : + startColumn : 0 + startLine : 0 + url : foo.js + } +} +Test runtime stack trace: +{ + method : Runtime.consoleAPICalled + params : { + args : [ + [0] : { + description : 42 + type : number + value : 42 + } + ] + executionContextId : + stackTrace : { + callFrames : [ + [0] : { + columnNumber : 14 + functionName : foo + lineNumber : 2 + scriptId : + url : prefix://url + } + [1] : { + columnNumber : 0 + functionName : + lineNumber : 0 + scriptId : + url : boo.js + } + [2] : { + columnNumber : 4 + functionName : + lineNumber : 4 + scriptId : + url : prefix://url + } + ] + } + timestamp : + type : log + } +} +Test debugger stack trace: +[ + [0] : prefix://url + [1] : boo.js + [2] : prefix://url +] diff --git a/deps/v8/test/inspector/debugger/resource-name-to-url.js b/deps/v8/test/inspector/debugger/resource-name-to-url.js new file mode 100644 index 00000000000000..620c7a2864b406 --- /dev/null +++ b/deps/v8/test/inspector/debugger/resource-name-to-url.js @@ -0,0 +1,49 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +let {session, contextGroup, Protocol} = InspectorTest.start( + 'Tests V8InspectorClient::resourceNameToUrl.'); + +(async function test(){ + Protocol.Runtime.enable(); + await Protocol.Debugger.enable(); + contextGroup.addScript(`inspector.setResourceNamePrefix('prefix://')`); + await Protocol.Debugger.onceScriptParsed(); + + InspectorTest.log('Check script with url:'); + contextGroup.addScript('function foo(){}', 0, 0, 'url'); + InspectorTest.logMessage(await Protocol.Debugger.onceScriptParsed()); + + InspectorTest.log('Check script with sourceURL comment:'); + contextGroup.addScript('function foo(){} //# sourceURL=foo.js', 0, 0, 'url'); + InspectorTest.logMessage(await Protocol.Debugger.onceScriptParsed()); + + InspectorTest.log('Check script failed to parse:'); + contextGroup.addScript('function foo(){', 0, 0, 'url'); + InspectorTest.logMessage(await Protocol.Debugger.onceScriptFailedToParse()); + + InspectorTest.log('Check script failed to parse with sourceURL comment:'); + contextGroup.addScript('function foo(){ //# sourceURL=foo.js', 0, 0, 'url'); + InspectorTest.logMessage(await Protocol.Debugger.onceScriptFailedToParse()); + + InspectorTest.log('Test runtime stack trace:'); + contextGroup.addScript(` + function foo() { + console.log(42); + } + eval('foo(); //# sourceURL=boo.js'); + `, 0, 0, 'url'); + InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled()); + + InspectorTest.log('Test debugger stack trace:'); + contextGroup.addScript(` + function foo() { + debugger; + } + eval('foo(); //# sourceURL=boo.js'); + `, 0, 0, 'url'); + const {params:{callFrames}} = await Protocol.Debugger.oncePaused(); + InspectorTest.logMessage(callFrames.map(frame => frame.url)); + InspectorTest.completeTest(); +})(); diff --git a/deps/v8/test/inspector/inspector-test.cc b/deps/v8/test/inspector/inspector-test.cc index 668a9463d5b9cf..93a8b1d3f21880 100644 --- a/deps/v8/test/inspector/inspector-test.cc +++ b/deps/v8/test/inspector/inspector-test.cc @@ -712,6 +712,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask { ToV8String(isolate, "setAllowCodeGenerationFromStrings"), v8::FunctionTemplate::New( isolate, &InspectorExtension::SetAllowCodeGenerationFromStrings)); + inspector->Set(ToV8String(isolate, "setResourceNamePrefix"), + v8::FunctionTemplate::New( + isolate, &InspectorExtension::SetResourceNamePrefix)); global->Set(ToV8String(isolate, "inspector"), inspector); } @@ -973,6 +976,18 @@ class InspectorExtension : public IsolateData::SetupGlobalTask { args.GetIsolate()->GetCurrentContext()->AllowCodeGenerationFromStrings( args[0].As()->Value()); } + + static void SetResourceNamePrefix( + const v8::FunctionCallbackInfo& args) { + if (args.Length() != 1 || !args[0]->IsString()) { + fprintf(stderr, "Internal error: setResourceNamePrefix('prefix')."); + Exit(); + } + v8::Isolate* isolate = args.GetIsolate(); + v8::Local context = isolate->GetCurrentContext(); + IsolateData* data = IsolateData::FromContext(context); + data->SetResourceNamePrefix(v8::Local::Cast(args[0])); + } }; bool RunExtraCode(v8::Isolate* isolate, v8::Local context, diff --git a/deps/v8/test/inspector/isolate-data.cc b/deps/v8/test/inspector/isolate-data.cc index 15eee89a61faa2..a669cc41a1f26a 100644 --- a/deps/v8/test/inspector/isolate-data.cc +++ b/deps/v8/test/inspector/isolate-data.cc @@ -423,3 +423,32 @@ void IsolateData::maxAsyncCallStackDepthChanged(int depth) { if (!log_max_async_call_stack_depth_changed_) return; fprintf(stdout, "maxAsyncCallStackDepthChanged: %d\n", depth); } + +void IsolateData::SetResourceNamePrefix(v8::Local prefix) { + resource_name_prefix_.Reset(v8::Isolate::GetCurrent(), prefix); +} + +namespace { +class StringBufferImpl : public v8_inspector::StringBuffer { + public: + StringBufferImpl(v8::Isolate* isolate, v8::Local string) + : data_(ToVector(string)), + view_(data_.start(), data_.length()) {} + const v8_inspector::StringView& string() override { return view_; } + + private: + v8::internal::Vector data_; + v8_inspector::StringView view_; +}; +} // anonymous namespace + +std::unique_ptr IsolateData::resourceNameToUrl( + const v8_inspector::StringView& resourceName) { + if (resource_name_prefix_.IsEmpty()) return nullptr; + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::HandleScope handle_scope(isolate); + v8::Local name = ToString(isolate, resourceName); + v8::Local prefix = resource_name_prefix_.Get(isolate); + v8::Local url = v8::String::Concat(prefix, name); + return std::unique_ptr(new StringBufferImpl(isolate, url)); +} diff --git a/deps/v8/test/inspector/isolate-data.h b/deps/v8/test/inspector/isolate-data.h index 5eb9803a7416fb..d0a263e573827c 100644 --- a/deps/v8/test/inspector/isolate-data.h +++ b/deps/v8/test/inspector/isolate-data.h @@ -76,6 +76,7 @@ class IsolateData : public v8_inspector::V8InspectorClient { void FireContextCreated(v8::Local context, int context_group_id); void FireContextDestroyed(v8::Local context); void FreeContext(v8::Local context); + void SetResourceNamePrefix(v8::Local prefix); private: struct VectorCompare { @@ -114,6 +115,8 @@ class IsolateData : public v8_inspector::V8InspectorClient { v8_inspector::V8StackTrace*) override; bool isInspectableHeapObject(v8::Local) override; void maxAsyncCallStackDepthChanged(int depth) override; + std::unique_ptr resourceNameToUrl( + const v8_inspector::StringView& resourceName) override; // The isolate gets deleted by its {Dispose} method, not by the default // deleter. Therefore we have to define a custom deleter for the unique_ptr to @@ -141,6 +144,7 @@ class IsolateData : public v8_inspector::V8InspectorClient { bool log_console_api_message_calls_ = false; bool log_max_async_call_stack_depth_changed_ = false; v8::Global not_inspectable_private_; + v8::Global resource_name_prefix_; DISALLOW_COPY_AND_ASSIGN(IsolateData); }; From 251677ffad1cc4bca20edfcbd4454467c46aee13 Mon Sep 17 00:00:00 2001 From: Alexey Kozyatinskiy Date: Thu, 30 Aug 2018 14:58:38 -0700 Subject: [PATCH 2/3] src: added URL::FromFilePath method Method returns file URL from native file path. --- src/node_url.cc | 13 +++++++++++++ src/node_url.h | 2 ++ test/cctest/test_url.cc | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/src/node_url.cc b/src/node_url.cc index f1008c917de31f..e9b7eaf7bf9a37 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -2348,6 +2348,19 @@ std::string URL::ToFilePath() const { #endif } +URL URL::FromFilePath(const std::string& file_path) { + URL url("file://"); + std::string escaped_file_path; + for (size_t i = 0; i < file_path.length(); ++i) { + escaped_file_path += file_path[i]; + if (file_path[i] == '%') + escaped_file_path += "25"; + } + URL::Parse(escaped_file_path.c_str(), escaped_file_path.length(), kPathStart, + &url.context_, true, nullptr, false); + return url; +} + // This function works by calling out to a JS function that creates and // returns the JS URL object. Be mindful of the JS<->Native boundary // crossing that is required. diff --git a/src/node_url.h b/src/node_url.h index b2eadf9923a7ea..5e61aee4efbd5a 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -169,6 +169,8 @@ class URL { // Get the path of the file: URL in a format consumable by native file system // APIs. Returns an empty string if something went wrong. std::string ToFilePath() const; + // Get the file URL from native file system path. + static URL FromFilePath(const std::string& file_path); const Local ToObject(Environment* env) const; diff --git a/test/cctest/test_url.cc b/test/cctest/test_url.cc index 088634152a79c1..2e9b06e3a4783a 100644 --- a/test/cctest/test_url.cc +++ b/test/cctest/test_url.cc @@ -109,3 +109,36 @@ TEST_F(URLTest, ToFilePath) { #undef T } + +TEST_F(URLTest, FromFilePath) { + URL file_url; +#ifdef _WIN32 + file_url = URL::FromFilePath("C:\\Program Files\\"); + EXPECT_EQ("file:", file_url.protocol()); + EXPECT_EQ("/C:/Program%20Files/", file_url.path()); + + file_url = URL::FromFilePath("C:\\a\\b\\c"); + EXPECT_EQ("file:", file_url.protocol()); + EXPECT_EQ("/C:/a/b/c", file_url.path()); + + 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()); + EXPECT_EQ("/", file_url.path()); + + file_url = URL::FromFilePath("/a/b/c"); + EXPECT_EQ("file:", file_url.protocol()); + EXPECT_EQ("/a/b/c", file_url.path()); + + file_url = URL::FromFilePath("/a/%%.js"); + EXPECT_EQ("file:", file_url.protocol()); + EXPECT_EQ("/a/%25%25.js", file_url.path()); +#endif +} From a8a63c22edfd5c2bf1c18d068b2d03ad64aa8700 Mon Sep 17 00:00:00 2001 From: Alexey Kozyatinskiy Date: Thu, 30 Aug 2018 14:59:34 -0700 Subject: [PATCH 3/3] inspector: implemented V8InspectorClient::resourceNameToUrl This method is required by inspector to report normalized urls over the protocol. Fixes https://github.com/nodejs/node/issues/22223 --- src/inspector_agent.cc | 32 +++++++++++++++ test/cctest/test_url.cc | 4 -- .../test-inspector-multisession-js.js | 6 ++- test/parallel/test-v8-coverage.js | 2 +- test/sequential/test-inspector-bindings.js | 6 ++- .../test-inspector-break-when-eval.js | 5 ++- .../test-inspector-debug-brk-flag.js | 2 +- test/sequential/test-inspector-exception.js | 3 +- .../test-inspector-resource-name-to-url.js | 40 +++++++++++++++++++ test/sequential/test-inspector.js | 6 +-- 10 files changed, 90 insertions(+), 16 deletions(-) create mode 100644 test/sequential/test-inspector-resource-name-to-url.js diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index da19eefe368efd..2eb9339d3e1641 100644 --- 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" @@ -369,6 +370,25 @@ void NotifyClusterWorkersDebugEnabled(Environment* env) { MakeCallback(env->isolate(), process_object, emit_fn.As(), 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 { @@ -592,6 +612,18 @@ class NodeInspectorClient : public V8InspectorClient { return env_->isolate_data()->platform()->CurrentClockTimeMillis(); } + std::unique_ptr 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 running_nested_loop_ = false; std::unique_ptr client_; diff --git a/test/cctest/test_url.cc b/test/cctest/test_url.cc index 2e9b06e3a4783a..ddef534b57485f 100644 --- 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 index 097b77e2c24231..92879d3ff3a7df 100644 --- 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 index ad5100b42ea8d9..91581a5a073460 100644 --- 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 index bea8b552202087..f5583c1d7bc6d6 100644 --- 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 index 8be5285221d513..e7529f786a9859 100644 --- 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 index 0e5df52560d2b1..d488eea2626584 100644 --- 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 index 8f791740f2feb3..a35115b9f5f15f 100644 --- 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(21, script); + await session.waitForBreakOnLine(21, 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 new file mode 100644 index 00000000000000..41a98ba219b24c --- /dev/null +++ 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.'); + 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 index ba1fce25a04fad..5b8921b4ee5525 100644 --- a/test/sequential/test-inspector.js +++ b/test/sequential/test-inspector.js @@ -69,7 +69,7 @@ async function testBreakpointOnStart(session) { ]; await session.send(commands); - await session.waitForBreakOnLine(0, session.scriptPath()); + await session.waitForBreakOnLine(0, session.scriptURL()); } async function testBreakpoint(session) { @@ -77,7 +77,7 @@ async function testBreakpoint(session) { const commands = [ { 'method': 'Debugger.setBreakpointByUrl', 'params': { 'lineNumber': 5, - 'url': session.scriptPath(), + 'url': session.scriptURL(), 'columnNumber': 0, 'condition': '' } @@ -92,7 +92,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');