Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(453)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/inspector/v8-debugger-agent-impl.cc

Issue 2633803002: [inspector] implemented blackboxing inside v8 (Closed)
Patch Set: one more test Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/inspector/v8-debugger-agent-impl.h ('K') | « src/inspector/v8-debugger-agent-impl.h ('k') | src/inspector/v8-debugger-script.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/inspector/v8-debugger-agent-impl.cc
diff --git a/src/inspector/v8-debugger-agent-impl.cc b/src/inspector/v8-debugger-agent-impl.cc
index 09c7b5842be7cc5b9ce57929f8603e6f46c30f3e..17111bff5ff8284627563ab97a049752e9486ea7 100644
--- a/src/inspector/v8-debugger-agent-impl.cc
+++ b/src/inspector/v8-debugger-agent-impl.cc
@@ -54,7 +54,6 @@ static const char skipAllPauses[] = "skipAllPauses";
} // namespace DebuggerAgentState
-static const int kMaxSkipStepFrameCount = 128;
static const char kBacktraceObjectGroup[] = "backtrace";
static const char kDebuggerNotEnabled[] = "Debugger agent is not enabled";
static const char kDebuggerNotPaused[] =
@@ -134,13 +133,8 @@ V8DebuggerAgentImpl::V8DebuggerAgentImpl(
m_isolate(m_inspector->isolate()),
m_breakReason(protocol::Debugger::Paused::ReasonEnum::Other),
m_scheduledDebuggerStep(NoStep),
- m_skipNextDebuggerStepOut(false),
m_javaScriptPauseScheduled(false),
- m_steppingFromFramework(false),
- m_pausingOnNativeEvent(false),
- m_skippedStepFrameCount(0),
m_recursionLevelForStepOut(0),
- m_recursionLevelForStepFrame(0),
m_skipAllPauses(false) {
clearBreakDetails();
}
@@ -190,21 +184,19 @@ Response V8DebuggerAgentImpl::disable() {
m_pausedContext.Reset();
JavaScriptCallFrames emptyCallFrames;
m_pausedCallFrames.swap(emptyCallFrames);
- m_scripts.clear();
+ m_blackboxPattern = nullptr;
m_blackboxedPositions.clear();
+ for (const auto& it : m_scripts) {
+ it.second->blackboxStateChanged();
+ }
+ m_scripts.clear();
m_breakpointIdToDebuggerBreakpointIds.clear();
m_debugger->setAsyncCallStackDepth(this, 0);
m_continueToLocationBreakpointId = String16();
clearBreakDetails();
m_scheduledDebuggerStep = NoStep;
- m_skipNextDebuggerStepOut = false;
m_javaScriptPauseScheduled = false;
- m_steppingFromFramework = false;
- m_pausingOnNativeEvent = false;
- m_skippedStepFrameCount = 0;
- m_recursionLevelForStepFrame = 0;
m_skipAllPauses = false;
- m_blackboxPattern = nullptr;
m_state->remove(DebuggerAgentState::blackboxPattern);
m_enabled = false;
m_state->setBoolean(DebuggerAgentState::debuggerEnabled, false);
@@ -434,28 +426,10 @@ Response V8DebuggerAgentImpl::continueToLocation(
return resume();
}
-bool V8DebuggerAgentImpl::isCurrentCallStackEmptyOrBlackboxed() {
- DCHECK(enabled());
- JavaScriptCallFrames callFrames = m_debugger->currentCallFrames();
- for (size_t index = 0; index < callFrames.size(); ++index) {
- if (!isCallFrameWithUnknownScriptOrBlackboxed(callFrames[index].get()))
- return false;
- }
- return true;
-}
-
-bool V8DebuggerAgentImpl::isTopPausedCallFrameBlackboxed() {
- DCHECK(enabled());
- JavaScriptCallFrame* frame =
- m_pausedCallFrames.size() ? m_pausedCallFrames[0].get() : nullptr;
- return isCallFrameWithUnknownScriptOrBlackboxed(frame);
-}
-
-bool V8DebuggerAgentImpl::isCallFrameWithUnknownScriptOrBlackboxed(
- JavaScriptCallFrame* frame) {
- if (!frame) return true;
- ScriptsMap::iterator it =
- m_scripts.find(String16::fromInteger(frame->sourceID()));
+bool V8DebuggerAgentImpl::isBlackboxed(const String16& scriptId,
+ const v8::debug::Location& start,
+ const v8::debug::Location& end) {
+ ScriptsMap::iterator it = m_scripts.find(scriptId);
if (it == m_scripts.end()) {
// Unknown scripts are blackboxed.
return true;
@@ -466,48 +440,25 @@ bool V8DebuggerAgentImpl::isCallFrameWithUnknownScriptOrBlackboxed(
m_blackboxPattern->match(scriptSourceURL) != -1)
return true;
}
- auto itBlackboxedPositions =
- m_blackboxedPositions.find(String16::fromInteger(frame->sourceID()));
+ auto itBlackboxedPositions = m_blackboxedPositions.find(scriptId);
if (itBlackboxedPositions == m_blackboxedPositions.end()) return false;
const std::vector<std::pair<int, int>>& ranges =
itBlackboxedPositions->second;
- auto itRange = std::lower_bound(
+ auto itStartRange = std::lower_bound(
ranges.begin(), ranges.end(),
- std::make_pair(frame->line(), frame->column()), positionComparator);
+ std::make_pair(start.GetLineNumber(), start.GetColumnNumber()),
+ positionComparator);
+ auto itEndRange = std::lower_bound(
+ ranges.begin(), ranges.end(),
+ std::make_pair(end.GetLineNumber(), end.GetColumnNumber()),
+ positionComparator);
// Ranges array contains positions in script where blackbox state is changed.
// [(0,0) ... ranges[0]) isn't blackboxed, [ranges[0] ... ranges[1]) is
// blackboxed...
- return std::distance(ranges.begin(), itRange) % 2;
-}
-
-V8DebuggerAgentImpl::SkipPauseRequest
-V8DebuggerAgentImpl::shouldSkipExceptionPause(
- JavaScriptCallFrame* topCallFrame) {
- if (m_steppingFromFramework) return RequestNoSkip;
- if (isCallFrameWithUnknownScriptOrBlackboxed(topCallFrame))
- return RequestContinue;
- return RequestNoSkip;
-}
-
-V8DebuggerAgentImpl::SkipPauseRequest V8DebuggerAgentImpl::shouldSkipStepPause(
- JavaScriptCallFrame* topCallFrame) {
- if (m_steppingFromFramework) return RequestNoSkip;
-
- if (m_skipNextDebuggerStepOut) {
- m_skipNextDebuggerStepOut = false;
- if (m_scheduledDebuggerStep == StepOut) return RequestStepOut;
- }
-
- if (!isCallFrameWithUnknownScriptOrBlackboxed(topCallFrame))
- return RequestNoSkip;
-
- if (m_skippedStepFrameCount >= kMaxSkipStepFrameCount) return RequestStepOut;
-
- if (!m_skippedStepFrameCount) m_recursionLevelForStepFrame = 1;
-
- ++m_skippedStepFrameCount;
- return RequestStepFrame;
+ return std::distance(ranges.begin(), itStartRange) % 2 ||
dgozman 2017/01/19 21:49:14 Let's be safe instead and blackbox only if script
kozy 2017/01/20 02:32:37 Done.
+ std::distance(ranges.begin(), itEndRange) % 2 ||
+ std::distance(itStartRange, itEndRange) > 0;
}
std::unique_ptr<protocol::Debugger::Location>
@@ -641,8 +592,6 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatement(
return;
m_breakReason = breakReason;
m_breakAuxData = std::move(data);
- m_pausingOnNativeEvent = true;
- m_skipNextDebuggerStepOut = false;
m_debugger->setPauseOnNextStatement(true);
}
@@ -652,16 +601,12 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatementIfSteppingInto() {
m_debugger->isPaused())
return;
clearBreakDetails();
- m_pausingOnNativeEvent = false;
- m_skippedStepFrameCount = 0;
- m_recursionLevelForStepFrame = 0;
m_debugger->setPauseOnNextStatement(true);
}
void V8DebuggerAgentImpl::cancelPauseOnNextStatement() {
if (m_javaScriptPauseScheduled || m_debugger->isPaused()) return;
clearBreakDetails();
- m_pausingOnNativeEvent = false;
m_debugger->setPauseOnNextStatement(false);
}
@@ -672,8 +617,6 @@ Response V8DebuggerAgentImpl::pause() {
clearBreakDetails();
m_javaScriptPauseScheduled = true;
m_scheduledDebuggerStep = NoStep;
- m_skippedStepFrameCount = 0;
- m_steppingFromFramework = false;
m_debugger->setPauseOnNextStatement(true);
return Response::OK();
}
@@ -681,7 +624,6 @@ Response V8DebuggerAgentImpl::pause() {
Response V8DebuggerAgentImpl::resume() {
if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
m_scheduledDebuggerStep = NoStep;
- m_steppingFromFramework = false;
m_session->releaseObjectGroup(kBacktraceObjectGroup);
m_debugger->continueProgram();
return Response::OK();
@@ -694,7 +636,6 @@ Response V8DebuggerAgentImpl::stepOver() {
!m_pausedCallFrames.empty() ? m_pausedCallFrames[0].get() : nullptr;
if (frame && frame->isAtReturn()) return stepInto();
m_scheduledDebuggerStep = StepOver;
- m_steppingFromFramework = isTopPausedCallFrameBlackboxed();
m_session->releaseObjectGroup(kBacktraceObjectGroup);
m_debugger->stepOverStatement();
return Response::OK();
@@ -703,7 +644,6 @@ Response V8DebuggerAgentImpl::stepOver() {
Response V8DebuggerAgentImpl::stepInto() {
if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
m_scheduledDebuggerStep = StepInto;
- m_steppingFromFramework = isTopPausedCallFrameBlackboxed();
m_session->releaseObjectGroup(kBacktraceObjectGroup);
m_debugger->stepIntoStatement();
return Response::OK();
@@ -712,9 +652,7 @@ Response V8DebuggerAgentImpl::stepInto() {
Response V8DebuggerAgentImpl::stepOut() {
if (m_pausedContext.IsEmpty()) return Response::Error(kDebuggerNotPaused);
m_scheduledDebuggerStep = StepOut;
- m_skipNextDebuggerStepOut = false;
m_recursionLevelForStepOut = 1;
- m_steppingFromFramework = isTopPausedCallFrameBlackboxed();
m_session->releaseObjectGroup(kBacktraceObjectGroup);
m_debugger->stepOutOfFunction();
return Response::OK();
@@ -811,6 +749,9 @@ Response V8DebuggerAgentImpl::setBlackboxPatterns(
std::unique_ptr<protocol::Array<String16>> patterns) {
if (!patterns->length()) {
m_blackboxPattern = nullptr;
+ for (const auto& it : m_scripts) {
+ it.second->blackboxStateChanged();
+ }
m_state->remove(DebuggerAgentState::blackboxPattern);
return Response::OK();
}
@@ -836,6 +777,9 @@ Response V8DebuggerAgentImpl::setBlackboxPattern(const String16& pattern) {
if (!regex->isValid())
return Response::Error("Pattern parser error: " + regex->errorMessage());
m_blackboxPattern = std::move(regex);
+ for (const auto& it : m_scripts) {
+ it.second->blackboxStateChanged();
+ }
return Response::OK();
}
@@ -843,11 +787,13 @@ Response V8DebuggerAgentImpl::setBlackboxedRanges(
const String16& scriptId,
std::unique_ptr<protocol::Array<protocol::Debugger::ScriptPosition>>
inPositions) {
- if (m_scripts.find(scriptId) == m_scripts.end())
+ auto it = m_scripts.find(scriptId);
+ if (it == m_scripts.end())
return Response::Error("No script with passed id.");
if (!inPositions->length()) {
m_blackboxedPositions.erase(scriptId);
+ it->second->blackboxStateChanged();
return Response::OK();
}
@@ -873,6 +819,7 @@ Response V8DebuggerAgentImpl::setBlackboxedRanges(
}
m_blackboxedPositions[scriptId] = positions;
+ it->second->blackboxStateChanged();
return Response::OK();
}
@@ -901,27 +848,6 @@ void V8DebuggerAgentImpl::changeJavaScriptRecursionLevel(int step) {
// switch stepping to step into a next JS task, as if we exited to a
// blackboxed framework.
m_scheduledDebuggerStep = StepInto;
- m_skipNextDebuggerStepOut = false;
- }
- }
- if (m_recursionLevelForStepFrame) {
- m_recursionLevelForStepFrame += step;
- if (!m_recursionLevelForStepFrame) {
- // We have walked through a blackboxed framework and got back to where we
- // started.
- // If there was no stepping scheduled, we should cancel the stepping
- // explicitly,
- // since there may be a scheduled StepFrame left.
- // Otherwise, if we were stepping in/over, the StepFrame will stop at the
- // right location,
- // whereas if we were stepping out, we should continue doing so after
- // debugger pauses
- // from the old StepFrame.
- m_skippedStepFrameCount = 0;
- if (m_scheduledDebuggerStep == NoStep)
- m_debugger->clearStepping();
- else if (m_scheduledDebuggerStep == StepOut)
- m_skipNextDebuggerStepOut = true;
}
}
}
@@ -1072,6 +998,7 @@ void V8DebuggerAgentImpl::didParseSource(
ScriptsMap::iterator scriptIterator = m_scripts.find(scriptId);
DCHECK(scriptIterator != m_scripts.end());
V8DebuggerScript* scriptRef = scriptIterator->second.get();
+ scriptRef->blackboxStateChanged();
Maybe<String16> sourceMapURLParam = scriptRef->sourceMappingURL();
Maybe<protocol::DictionaryValue> executionContextAuxDataParam(
@@ -1121,35 +1048,16 @@ void V8DebuggerAgentImpl::didParseSource(
}
}
-V8DebuggerAgentImpl::SkipPauseRequest V8DebuggerAgentImpl::didPause(
- v8::Local<v8::Context> context, v8::Local<v8::Value> exception,
- const std::vector<String16>& hitBreakpoints, bool isPromiseRejection,
- bool isUncaught, bool isOOMBreak) {
+bool V8DebuggerAgentImpl::didPause(v8::Local<v8::Context> context,
+ v8::Local<v8::Value> exception,
+ const std::vector<String16>& hitBreakpoints,
+ bool isPromiseRejection, bool isUncaught,
+ bool isOOMBreak) {
JavaScriptCallFrames callFrames = m_debugger->currentCallFrames(1);
JavaScriptCallFrame* topCallFrame =
!callFrames.empty() ? callFrames.begin()->get() : nullptr;
-
- V8DebuggerAgentImpl::SkipPauseRequest result;
- if (isOOMBreak)
- result = RequestNoSkip;
- else if (m_skipAllPauses)
- result = RequestContinue;
- else if (!hitBreakpoints.empty())
- result = RequestNoSkip; // Don't skip explicit breakpoints even if set in
- // frameworks.
- else if (!exception.IsEmpty())
- result = shouldSkipExceptionPause(topCallFrame);
- else if (m_scheduledDebuggerStep != NoStep || m_javaScriptPauseScheduled ||
- m_pausingOnNativeEvent)
- result = shouldSkipStepPause(topCallFrame);
- else
- result = RequestNoSkip;
-
- m_skipNextDebuggerStepOut = false;
- if (result != RequestNoSkip) return result;
// Skip pauses inside V8 internal scripts and on syntax errors.
- if (!topCallFrame) return RequestContinue;
-
+ if (!isOOMBreak && (!topCallFrame || m_skipAllPauses)) return false;
dgozman 2017/01/19 21:49:14 - Let's check for m_skipAllPauses before retrievin
kozy 2017/01/20 02:32:37 first done, second in follow up.
DCHECK(m_pausedContext.IsEmpty());
JavaScriptCallFrames frames = m_debugger->currentCallFrames();
m_pausedCallFrames.swap(frames);
@@ -1205,16 +1113,12 @@ V8DebuggerAgentImpl::SkipPauseRequest V8DebuggerAgentImpl::didPause(
currentAsyncStackTrace());
m_scheduledDebuggerStep = NoStep;
m_javaScriptPauseScheduled = false;
- m_steppingFromFramework = false;
- m_pausingOnNativeEvent = false;
- m_skippedStepFrameCount = 0;
- m_recursionLevelForStepFrame = 0;
if (!m_continueToLocationBreakpointId.isEmpty()) {
m_debugger->removeBreakpoint(m_continueToLocationBreakpointId);
m_continueToLocationBreakpointId = "";
}
- return result;
+ return true;
}
void V8DebuggerAgentImpl::didContinue() {
@@ -1229,14 +1133,12 @@ void V8DebuggerAgentImpl::breakProgram(
const String16& breakReason,
std::unique_ptr<protocol::DictionaryValue> data) {
if (!enabled() || m_skipAllPauses || !m_pausedContext.IsEmpty() ||
- isCurrentCallStackEmptyOrBlackboxed() ||
!m_debugger->breakpointsActivated())
return;
+ if (!m_debugger->hasUserFrameOnStack()) return;
m_breakReason = breakReason;
m_breakAuxData = std::move(data);
m_scheduledDebuggerStep = NoStep;
- m_steppingFromFramework = false;
- m_pausingOnNativeEvent = false;
m_debugger->breakProgram();
}
@@ -1246,7 +1148,8 @@ void V8DebuggerAgentImpl::breakProgramOnException(
if (!enabled() ||
m_debugger->getPauseOnExceptionsState() == v8::debug::NoBreakOnException)
return;
- breakProgram(breakReason, std::move(data));
+ if (m_debugger->hasUserFrameOnStack())
dgozman 2017/01/19 21:49:14 You already check for that in breakProgram.
kozy 2017/01/20 02:32:37 Done.
+ breakProgram(breakReason, std::move(data));
}
void V8DebuggerAgentImpl::clearBreakDetails() {
@@ -1274,8 +1177,11 @@ void V8DebuggerAgentImpl::removeBreakpointAt(const String16& scriptId,
void V8DebuggerAgentImpl::reset() {
if (!enabled()) return;
m_scheduledDebuggerStep = NoStep;
- m_scripts.clear();
m_blackboxedPositions.clear();
+ for (const auto& it : m_scripts) {
+ it.second->blackboxStateChanged();
dgozman 2017/01/19 21:49:14 I don't really like that we are talking to v8::deb
kozy 2017/01/20 02:32:37 Acknowledged.
+ }
+ m_scripts.clear();
m_breakpointIdToDebuggerBreakpointIds.clear();
}
« src/inspector/v8-debugger-agent-impl.h ('K') | « src/inspector/v8-debugger-agent-impl.h ('k') | src/inspector/v8-debugger-script.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698