[inspector] cleanup old failed to parse anonymous scripts
We already cleanup these scripts on frontend side. It is crucial to cleanup them on backend side as well, since some web applications use following logic: get some data from network, add this data to buffer, try to parse buffer using JSON.parse. On each unsuccessfull JSON.parse we get another scriptFailedToParse event. Frontend logic of discarding scripts: https://goo.gl/FDtaWK Some idea of smarter logic here: track what script ids are reported using protocol and cleanup only script ids which reported not only as part of scriptFailedToParse event. R=alph@chromium.org Bug: chromium:810812 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ifd67764c232e4abc7dc6e8e69a651bf9ac0e381b Reviewed-on: https://chromium-review.googlesource.com/919834 Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Reviewed-by: Alexei Filippov <alph@chromium.org> Cr-Commit-Position: refs/heads/master@{#51337}
This commit is contained in:
parent
667173aab8
commit
6db8a9c079
@ -57,6 +57,8 @@ static const char kDebuggerNotPaused[] =
|
||||
static const size_t kBreakpointHintMaxLength = 128;
|
||||
static const intptr_t kBreakpointHintMaxSearchOffset = 80 * 10;
|
||||
|
||||
static const int kMaxScriptFailedToParseScripts = 1000;
|
||||
|
||||
namespace {
|
||||
|
||||
void TranslateLocation(protocol::Debugger::Location* location,
|
||||
@ -1416,7 +1418,13 @@ void V8DebuggerAgentImpl::didParseSource(
|
||||
static_cast<int>(scriptRef->source().length()), std::move(stackTrace));
|
||||
}
|
||||
|
||||
if (!success) return;
|
||||
if (!success) {
|
||||
if (scriptURL.isEmpty()) {
|
||||
m_failedToParseAnonymousScriptIds.push_back(scriptId);
|
||||
cleanupOldFailedToParseAnonymousScriptsIfNeeded();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
std::vector<protocol::DictionaryValue*> potentialBreakpoints;
|
||||
if (!scriptURL.isEmpty()) {
|
||||
@ -1618,4 +1626,18 @@ void V8DebuggerAgentImpl::reset() {
|
||||
m_breakpointIdToDebuggerBreakpointIds.clear();
|
||||
}
|
||||
|
||||
void V8DebuggerAgentImpl::cleanupOldFailedToParseAnonymousScriptsIfNeeded() {
|
||||
if (m_failedToParseAnonymousScriptIds.size() <=
|
||||
kMaxScriptFailedToParseScripts)
|
||||
return;
|
||||
static_assert(kMaxScriptFailedToParseScripts > 100,
|
||||
"kMaxScriptFailedToParseScripts should be greater then 100");
|
||||
while (m_failedToParseAnonymousScriptIds.size() >
|
||||
kMaxScriptFailedToParseScripts - 100 + 1) {
|
||||
String16 scriptId = m_failedToParseAnonymousScriptIds.front();
|
||||
m_failedToParseAnonymousScriptIds.pop_front();
|
||||
m_scripts.erase(scriptId);
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace v8_inspector
|
||||
|
@ -5,6 +5,7 @@
|
||||
#ifndef V8_INSPECTOR_V8_DEBUGGER_AGENT_IMPL_H_
|
||||
#define V8_INSPECTOR_V8_DEBUGGER_AGENT_IMPL_H_
|
||||
|
||||
#include <deque>
|
||||
#include <vector>
|
||||
|
||||
#include "src/base/macros.h"
|
||||
@ -192,6 +193,9 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
|
||||
BreakpointIdToDebuggerBreakpointIdsMap m_breakpointIdToDebuggerBreakpointIds;
|
||||
DebuggerBreakpointIdToBreakpointIdMap m_debuggerBreakpointIdToBreakpointId;
|
||||
|
||||
std::deque<String16> m_failedToParseAnonymousScriptIds;
|
||||
void cleanupOldFailedToParseAnonymousScriptsIfNeeded();
|
||||
|
||||
using BreakReason =
|
||||
std::pair<String16, std::unique_ptr<protocol::DictionaryValue>>;
|
||||
std::vector<BreakReason> m_breakReason;
|
||||
|
@ -0,0 +1,17 @@
|
||||
Checks that inspector collects old faied to parse anonymous scripts.
|
||||
Generate 1000 scriptFailedToParse events
|
||||
error:0
|
||||
success:1000
|
||||
Generate three scriptFailedToParse event for non anonymous script
|
||||
error:0
|
||||
success:1003
|
||||
Generate one more scriptFailedToParse event for anonymous script
|
||||
error:100
|
||||
success:904
|
||||
Check that latest script is still available
|
||||
{
|
||||
id : <messageId>
|
||||
result : {
|
||||
scriptSource : }
|
||||
}
|
||||
}
|
@ -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(
|
||||
'Checks that inspector collects old faied to parse anonymous scripts.');
|
||||
|
||||
(async function main() {
|
||||
Protocol.Debugger.enable();
|
||||
const scriptIds = [];
|
||||
Protocol.Debugger.onScriptFailedToParse(
|
||||
message => scriptIds.push(message.params.scriptId));
|
||||
InspectorTest.log('Generate 1000 scriptFailedToParse events');
|
||||
await Protocol.Runtime.evaluate({
|
||||
expression: `for (var i = 0; i < 1000; ++i) {
|
||||
try { JSON.parse('}'); } catch(e) {}
|
||||
}`
|
||||
});
|
||||
await dumpScriptIdsStats(scriptIds);
|
||||
InspectorTest.log(
|
||||
'Generate three scriptFailedToParse event for non anonymous script');
|
||||
for (var i = 0; i < 3; ++i) {
|
||||
await Protocol.Runtime.evaluate({expression: '}//# sourceURL=foo.js'});
|
||||
}
|
||||
await dumpScriptIdsStats(scriptIds);
|
||||
InspectorTest.log(
|
||||
'Generate one more scriptFailedToParse event for anonymous script');
|
||||
await Protocol.Runtime.evaluate(
|
||||
{expression: `try {JSON.parse('}');} catch(e){}`});
|
||||
await dumpScriptIdsStats(scriptIds);
|
||||
InspectorTest.log('Check that latest script is still available');
|
||||
InspectorTest.logMessage(await Protocol.Debugger.getScriptSource(
|
||||
{scriptId: scriptIds[scriptIds.length - 1]}));
|
||||
InspectorTest.completeTest();
|
||||
})();
|
||||
|
||||
async function dumpScriptIdsStats(scriptIds) {
|
||||
let errors = 0;
|
||||
let success = 0;
|
||||
for (let scriptId of scriptIds) {
|
||||
const result =
|
||||
await Protocol.Debugger.getScriptSource({scriptId: scriptId});
|
||||
if (result.error)
|
||||
++errors;
|
||||
else
|
||||
++success;
|
||||
}
|
||||
InspectorTest.log(`error:${errors}\nsuccess:${success}`);
|
||||
}
|
Loading…
Reference in New Issue
Block a user