Commit 1b3b808a authored by Alexey Kozyatinskiy's avatar Alexey Kozyatinskiy Committed by Commit Bot

inspector: find magic comment using V8 scanner

Inspector tries to provide sourceURL and sourceMappingURL for scripts
with parser errors. Without this CL we convert source of each script
to inspector string and search for magic comment there. Some web sites
use pattern when they get some data from network and constantly try to
parse this data as JSON, in this case we do a lot of useless work.

So we can parse magic comments on V8 side only for compilation errors
(excluding parse JSON errors), to do it we can reuse scanner by running
it on each potential comment.

R=alph@chromium.org,verwaest@chromium.org,yangguo@chromium.org

Bug: chromium:873865,v8:7731
Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I77c270fd0e95cd7b2c9ee4b7f72ef344bc1fa104
Reviewed-on: https://chromium-review.googlesource.com/1182446Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Reviewed-by: 's avatarAlexei Filippov <alph@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55280}
parent 7fb6109b
...@@ -32,6 +32,8 @@ ...@@ -32,6 +32,8 @@
#include "src/objects/debug-objects-inl.h" #include "src/objects/debug-objects-inl.h"
#include "src/objects/js-generator-inl.h" #include "src/objects/js-generator-inl.h"
#include "src/objects/js-promise-inl.h" #include "src/objects/js-promise-inl.h"
#include "src/parsing/scanner-character-streams.h"
#include "src/parsing/scanner.h"
#include "src/snapshot/natives.h" #include "src/snapshot/natives.h"
#include "src/snapshot/snapshot.h" #include "src/snapshot/snapshot.h"
#include "src/wasm/wasm-objects-inl.h" #include "src/wasm/wasm-objects-inl.h"
...@@ -1845,15 +1847,59 @@ bool Debug::SetScriptSource(Handle<Script> script, Handle<String> source, ...@@ -1845,15 +1847,59 @@ bool Debug::SetScriptSource(Handle<Script> script, Handle<String> source,
return result->status == debug::LiveEditResult::OK; return result->status == debug::LiveEditResult::OK;
} }
void Debug::OnParseJSONError(Handle<Script> script) {
ProcessCompileEvent(CompileStatus::JSON_PARSE_ERROR, script);
}
void Debug::OnCompileError(Handle<Script> script) { void Debug::OnCompileError(Handle<Script> script) {
ProcessCompileEvent(true, script); ProcessCompileEvent(CompileStatus::COMPILE_ERROR, script);
} }
void Debug::OnAfterCompile(Handle<Script> script) { void Debug::OnAfterCompile(Handle<Script> script) {
ProcessCompileEvent(false, script); ProcessCompileEvent(CompileStatus::OK, script);
}
namespace {
void EnsureMagicComments(Isolate* isolate, Handle<Script> script) {
Handle<Object> source_value(script->source(), isolate);
DCHECK(source_value->IsString());
Handle<String> comment_start =
isolate->factory()->NewStringFromStaticChars("//");
Handle<String> source(Handle<String>::cast(source_value));
std::unique_ptr<Utf16CharacterStream> scanner_stream(
ScannerStream::For(isolate, source));
Scanner scanner(isolate->unicode_cache(), scanner_stream.get(), false);
scanner.Initialize();
bool has_source_url = script->source_url()->IsString();
bool has_source_mapping_url = script->source_mapping_url()->IsString();
int prev_start = source->length();
while (prev_start > 0 && (!has_source_mapping_url || !has_source_url)) {
int start = Smi::ToInt(
String::LastIndexOf(isolate, source, comment_start,
handle(Smi::FromInt(prev_start), isolate)));
if (start == -1) return;
scanner.SeekForward(start);
scanner.Next();
Handle<String> source_url = scanner.SourceUrl(isolate);
if (!has_source_url && !source_url.is_null()) {
script->set_source_url(*source_url);
has_source_url = true;
}
Handle<String> source_mapping_url = scanner.SourceMappingUrl(isolate);
if (!has_source_mapping_url && !source_mapping_url.is_null()) {
script->set_source_mapping_url(*source_mapping_url);
has_source_mapping_url = true;
}
prev_start = start - 1;
}
} }
} // namespace
void Debug::ProcessCompileEvent(bool has_compile_error, Handle<Script> script) { void Debug::ProcessCompileEvent(CompileStatus status, Handle<Script> script) {
// TODO(kozyatinskiy): teach devtools to work with liveedit scripts better // TODO(kozyatinskiy): teach devtools to work with liveedit scripts better
// first and then remove this fast return. // first and then remove this fast return.
if (running_live_edit_) return; if (running_live_edit_) return;
...@@ -1865,13 +1911,19 @@ void Debug::ProcessCompileEvent(bool has_compile_error, Handle<Script> script) { ...@@ -1865,13 +1911,19 @@ void Debug::ProcessCompileEvent(bool has_compile_error, Handle<Script> script) {
return; return;
} }
if (!debug_delegate_) return; if (!debug_delegate_) return;
if (status == CompileStatus::COMPILE_ERROR) {
EnsureMagicComments(isolate_, script);
}
SuppressDebug while_processing(this); SuppressDebug while_processing(this);
DebugScope debug_scope(this); DebugScope debug_scope(this);
HandleScope scope(isolate_); HandleScope scope(isolate_);
DisableBreak no_recursive_break(this); DisableBreak no_recursive_break(this);
AllowJavascriptExecution allow_script(isolate_); AllowJavascriptExecution allow_script(isolate_);
debug_delegate_->ScriptCompiled(ToApiHandle<debug::Script>(script), debug_delegate_->ScriptCompiled(ToApiHandle<debug::Script>(script),
running_live_edit_, has_compile_error); running_live_edit_,
status != CompileStatus::OK);
} }
int Debug::CurrentFrameCount() { int Debug::CurrentFrameCount() {
......
...@@ -222,6 +222,7 @@ class Debug { ...@@ -222,6 +222,7 @@ class Debug {
void OnThrow(Handle<Object> exception); void OnThrow(Handle<Object> exception);
void OnPromiseReject(Handle<Object> promise, Handle<Object> value); void OnPromiseReject(Handle<Object> promise, Handle<Object> value);
void OnCompileError(Handle<Script> script); void OnCompileError(Handle<Script> script);
void OnParseJSONError(Handle<Script> script);
void OnAfterCompile(Handle<Script> script); void OnAfterCompile(Handle<Script> script);
void HandleDebugBreak(IgnoreBreakMode ignore_break_mode); void HandleDebugBreak(IgnoreBreakMode ignore_break_mode);
...@@ -408,7 +409,8 @@ class Debug { ...@@ -408,7 +409,8 @@ class Debug {
void OnException(Handle<Object> exception, Handle<Object> promise); void OnException(Handle<Object> exception, Handle<Object> promise);
void ProcessCompileEvent(bool has_compile_error, Handle<Script> script); enum CompileStatus { OK, COMPILE_ERROR, JSON_PARSE_ERROR };
void ProcessCompileEvent(CompileStatus status, Handle<Script> script);
// Find the closest source position for a break point for a given position. // Find the closest source position for a break point for a given position.
int FindBreakablePosition(Handle<DebugInfo> debug_info, int source_position); int FindBreakablePosition(Handle<DebugInfo> debug_info, int source_position);
......
...@@ -13,57 +13,6 @@ namespace v8_inspector { ...@@ -13,57 +13,6 @@ namespace v8_inspector {
namespace { namespace {
String16 findMagicComment(const String16& content, const String16& name,
bool multiline) {
DCHECK_EQ(String16::kNotFound, name.find("="));
size_t length = content.length();
size_t nameLength = name.length();
size_t pos = length;
size_t equalSignPos = 0;
size_t closingCommentPos = 0;
while (true) {
pos = content.reverseFind(name, pos);
if (pos == String16::kNotFound) return String16();
// Check for a /\/[\/*][@#][ \t]/ regexp (length of 4) before found name.
if (pos < 4) return String16();
pos -= 4;
if (content[pos] != '/') continue;
if ((content[pos + 1] != '/' || multiline) &&
(content[pos + 1] != '*' || !multiline))
continue;
if (content[pos + 2] != '#' && content[pos + 2] != '@') continue;
if (content[pos + 3] != ' ' && content[pos + 3] != '\t') continue;
equalSignPos = pos + 4 + nameLength;
if (equalSignPos < length && content[equalSignPos] != '=') continue;
if (multiline) {
closingCommentPos = content.find("*/", equalSignPos + 1);
if (closingCommentPos == String16::kNotFound) return String16();
}
break;
}
DCHECK(equalSignPos);
DCHECK(!multiline || closingCommentPos);
size_t urlPos = equalSignPos + 1;
String16 match = multiline
? content.substring(urlPos, closingCommentPos - urlPos)
: content.substring(urlPos);
size_t newLine = match.find("\n");
if (newLine != String16::kNotFound) match = match.substring(0, newLine);
match = match.stripWhiteSpace();
for (size_t i = 0; i < match.length(); ++i) {
UChar c = match[i];
if (c == '"' || c == '\'' || c == ' ' || c == '\t') return "";
}
return match;
}
String16 createSearchRegexSource(const String16& text) { String16 createSearchRegexSource(const String16& text) {
String16Builder result; String16Builder result;
...@@ -153,13 +102,4 @@ searchInTextByLinesImpl(V8InspectorSession* session, const String16& text, ...@@ -153,13 +102,4 @@ searchInTextByLinesImpl(V8InspectorSession* session, const String16& text,
result.push_back(buildObjectForSearchMatch(match.first, match.second)); result.push_back(buildObjectForSearchMatch(match.first, match.second));
return result; return result;
} }
String16 findSourceURL(const String16& content, bool multiline) {
return findMagicComment(content, "sourceURL", multiline);
}
String16 findSourceMapURL(const String16& content, bool multiline) {
return findMagicComment(content, "sourceMappingURL", multiline);
}
} // namespace v8_inspector } // namespace v8_inspector
...@@ -12,8 +12,6 @@ namespace v8_inspector { ...@@ -12,8 +12,6 @@ namespace v8_inspector {
class V8InspectorSession; class V8InspectorSession;
String16 findSourceURL(const String16& content, bool multiline);
String16 findSourceMapURL(const String16& content, bool multiline);
std::vector<std::unique_ptr<protocol::Debugger::SearchMatch>> std::vector<std::unique_ptr<protocol::Debugger::SearchMatch>>
searchInTextByLinesImpl(V8InspectorSession*, const String16& text, searchInTextByLinesImpl(V8InspectorSession*, const String16& text,
const String16& query, bool caseSensitive, const String16& query, bool caseSensitive,
......
...@@ -1378,12 +1378,6 @@ bool V8DebuggerAgentImpl::isPaused() const { ...@@ -1378,12 +1378,6 @@ bool V8DebuggerAgentImpl::isPaused() const {
void V8DebuggerAgentImpl::didParseSource( void V8DebuggerAgentImpl::didParseSource(
std::unique_ptr<V8DebuggerScript> script, bool success) { std::unique_ptr<V8DebuggerScript> script, bool success) {
v8::HandleScope handles(m_isolate); v8::HandleScope handles(m_isolate);
if (!success) {
DCHECK(!script->isSourceLoadedLazily());
String16 scriptSource = script->source();
script->setSourceURL(findSourceURL(scriptSource, false));
script->setSourceMappingURL(findSourceMapURL(scriptSource, false));
}
int contextId = script->executionContextId(); int contextId = script->executionContextId();
int contextGroupId = m_inspector->contextGroupId(contextId); int contextGroupId = m_inspector->contextGroupId(contextId);
......
...@@ -201,7 +201,7 @@ MaybeHandle<Object> JsonParser<seq_one_byte>::ParseJson() { ...@@ -201,7 +201,7 @@ MaybeHandle<Object> JsonParser<seq_one_byte>::ParseJson() {
} }
// We should sent compile error event because we compile JSON object in // We should sent compile error event because we compile JSON object in
// separated source file. // separated source file.
isolate()->debug()->OnCompileError(script); isolate()->debug()->OnParseJSONError(script);
MessageLocation location(script, position_, position_ + 1); MessageLocation location(script, position_, position_ + 1);
Handle<Object> error = factory->NewSyntaxError(message, arg1, arg2); Handle<Object> error = factory->NewSyntaxError(message, arg1, arg2);
return isolate()->template Throw<Object>(error, &location); return isolate()->template Throw<Object>(error, &location);
......
...@@ -674,6 +674,24 @@ scriptFailedToParse ...@@ -674,6 +674,24 @@ scriptFailedToParse
startLine : 0 startLine : 0
url : failed.js url : failed.js
} }
scriptFailedToParse
{
scriptSource : {a:2://# sourceURL=failed.js<nl>//# sourceMappingURL=failed-map
}
{
endColumn : 31
endLine : 1
executionContextId : <executionContextId>
hasSourceURL : true
hash : 370d368557b75a8e281e4a84625a9e0263647da6
isModule : false
length : 60
scriptId : <scriptId>
sourceMapURL : failed-map
startColumn : 0
startLine : 0
url : failed.js
}
scriptParsed scriptParsed
{ {
scriptSource : function foo16(){}<nl> scriptSource : function foo16(){}<nl>
......
...@@ -35,6 +35,7 @@ function addScripts() { ...@@ -35,6 +35,7 @@ function addScripts() {
.then(() => addScript("function foo15(){}; eval(\"function foo14(){}//# sourceURL=eval.js\")//# sourceURL=eval-wrapper.js")) .then(() => addScript("function foo15(){}; eval(\"function foo14(){}//# sourceURL=eval.js\")//# sourceURL=eval-wrapper.js"))
// sourceURL and sourceMappingURL works even for script with syntax error // sourceURL and sourceMappingURL works even for script with syntax error
.then(() => addScript("}//# sourceURL=failed.js\n//# sourceMappingURL=failed-map")) .then(() => addScript("}//# sourceURL=failed.js\n//# sourceMappingURL=failed-map"))
.then(() => addScript("{a:2://# sourceURL=failed.js\n//# sourceMappingURL=failed-map"))
// empty lines at end // empty lines at end
.then(() => addScript("function foo16(){}\n")) .then(() => addScript("function foo16(){}\n"))
.then(() => addScript("function foo17(){}\n\n")) .then(() => addScript("function foo17(){}\n\n"))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment