Skip to content

Commit

Permalink
[DevTools] Reduce V8DebuggerAgent API surface.
Browse files Browse the repository at this point in the history
Instrumentation methods go to V8Debugger.

BUG=580337

Review URL: https://codereview.chromium.org/1889533002

Cr-Commit-Position: refs/heads/master@{#389251}
  • Loading branch information
dgozman authored and Commit bot committed Apr 22, 2016
1 parent 8cdb0fb commit ac39500
Show file tree
Hide file tree
Showing 16 changed files with 59 additions and 79 deletions.
10 changes: 5 additions & 5 deletions third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
#include "core/dom/ExecutionContext.h"
#include "core/fetch/CachedMetadata.h"
#include "core/fetch/ScriptResource.h"
#include "core/inspector/InspectorInstrumentation.h"
#include "core/inspector/InspectorTraceEvents.h"
#include "core/inspector/ThreadDebugger.h"
#include "platform/Histogram.h"
#include "platform/ScriptForbiddenScope.h"
#include "platform/TraceEvent.h"
Expand Down Expand Up @@ -404,9 +404,9 @@ v8::MaybeLocal<v8::Value> V8ScriptRunner::runCompiledScript(v8::Isolate* isolate
return v8::MaybeLocal<v8::Value>();
}
v8::MicrotasksScope microtasksScope(isolate, v8::MicrotasksScope::kRunMicrotasks);
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willExecuteScript(context, script->GetUnboundScript()->GetId());
ThreadDebugger::willExecuteScript(isolate, script->GetUnboundScript()->GetId());
result = script->Run(isolate->GetCurrentContext());
InspectorInstrumentation::didExecuteScript(cookie);
ThreadDebugger::didExecuteScript(isolate);
}

crashIfIsolateIsDead(isolate);
Expand Down Expand Up @@ -452,10 +452,10 @@ v8::MaybeLocal<v8::Value> V8ScriptRunner::callFunction(v8::Local<v8::Function> f
return v8::MaybeLocal<v8::Value>();
}
v8::MicrotasksScope microtasksScope(isolate, v8::MicrotasksScope::kRunMicrotasks);
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willExecuteScript(context, function->ScriptId());
ThreadDebugger::willExecuteScript(isolate, function->ScriptId());
v8::MaybeLocal<v8::Value> result = function->Call(isolate->GetCurrentContext(), receiver, argc, args);
crashIfIsolateIsDead(isolate);
InspectorInstrumentation::didExecuteScript(cookie);
ThreadDebugger::didExecuteScript(isolate);
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ void InspectorConsoleAgent::addMessageToConsole(ConsoleMessage* consoleMessage)
sendConsoleMessageToFrontend(consoleMessage, true);
if (consoleMessage->type() != AssertMessageType)
return;
if (!m_debuggerAgent || !m_debuggerAgent->enabled())
if (!m_debuggerAgent)
return;
m_debuggerAgent->breakProgramOnException(protocol::Debugger::Paused::ReasonEnum::Assert, nullptr);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ void InspectorDOMDebuggerAgent::descriptionForDOMEvent(Node* target, int breakpo

bool InspectorDOMDebuggerAgent::hasBreakpoint(Node* node, int type)
{
if (!m_domAgent->enabled() || !m_debuggerAgent->enabled())
if (!m_domAgent->enabled())
return false;
uint32_t rootBit = 1 << type;
uint32_t derivedBit = rootBit << domBreakpointDerivedTypeShift;
Expand Down Expand Up @@ -504,8 +504,6 @@ void InspectorDOMDebuggerAgent::pauseOnNativeEventIfNeeded(PassOwnPtr<protocol::
{
if (!eventData)
return;
if (!m_debuggerAgent->enabled())
return;
if (synchronous)
m_debuggerAgent->breakProgram(protocol::Debugger::Paused::ReasonEnum::EventListener, eventData);
else
Expand Down Expand Up @@ -541,12 +539,12 @@ void InspectorDOMDebuggerAgent::didFireWebGLError(const String& errorName)
return;
if (!errorName.isEmpty())
eventData->setString(webglErrorNameProperty, errorName);
pauseOnNativeEventIfNeeded(eventData.release(), m_debuggerAgent->canBreakProgram());
pauseOnNativeEventIfNeeded(eventData.release(), false);
}

void InspectorDOMDebuggerAgent::didFireWebGLWarning()
{
pauseOnNativeEventIfNeeded(preparePauseOnNativeEventData(webglWarningFiredEventName, 0), m_debuggerAgent->canBreakProgram());
pauseOnNativeEventIfNeeded(preparePauseOnNativeEventData(webglWarningFiredEventName, 0), false);
}

void InspectorDOMDebuggerAgent::didFireWebGLErrorOrWarning(const String& message)
Expand Down Expand Up @@ -598,8 +596,6 @@ void InspectorDOMDebuggerAgent::willSendXMLHttpRequest(const String& url)

if (breakpointURL.isNull())
return;
if (!m_debuggerAgent->enabled())
return;

OwnPtr<protocol::DictionaryValue> eventData = protocol::DictionaryValue::create();
eventData->setString("breakpointURL", breakpointURL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,28 +251,13 @@ void InspectorDebuggerAgent::setBlackboxedRanges(
m_v8DebuggerAgent->setBlackboxedRanges(errorString, inScriptId, inPositions);
}

bool InspectorDebuggerAgent::isPaused()
{
return m_v8DebuggerAgent->isPaused();
}

void InspectorDebuggerAgent::scriptExecutionBlockedByCSP(const String& directiveText)
{
OwnPtr<protocol::DictionaryValue> directive = protocol::DictionaryValue::create();
directive->setString("directiveText", directiveText);
m_v8DebuggerAgent->breakProgramOnException(protocol::Debugger::Paused::ReasonEnum::CSPViolation, directive.release());
}

void InspectorDebuggerAgent::willExecuteScript(int scriptId)
{
m_v8DebuggerAgent->willExecuteScript(scriptId);
}

void InspectorDebuggerAgent::didExecuteScript()
{
m_v8DebuggerAgent->didExecuteScript();
}

void InspectorDebuggerAgent::asyncTaskScheduled(const String& taskName, void* task)
{
m_v8DebuggerAgent->asyncTaskScheduled(taskName, task, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,7 @@ class CORE_EXPORT InspectorDebuggerAgent
void setBlackboxedRanges(ErrorString*, const String16& scriptId, PassOwnPtr<protocol::Array<protocol::Debugger::ScriptPosition>> positions) override;

// Called by InspectorInstrumentation.
bool isPaused();
void scriptExecutionBlockedByCSP(const String& directiveText);
void willExecuteScript(int scriptId);
void didExecuteScript();

// Async stack implementation.
void asyncTaskScheduled(const String& taskName, void* task);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "core/inspector/InspectorProfilerAgent.h"
#include "core/inspector/InspectorResourceAgent.h"
#include "core/inspector/InspectorSession.h"
#include "core/inspector/MainThreadDebugger.h"
#include "core/inspector/WorkerInspectorController.h"
#include "core/page/Page.h"
#include "core/workers/MainThreadWorkletGlobalScope.h"
Expand Down Expand Up @@ -157,16 +158,9 @@ InspectorInstrumentationCookie::~InspectorInstrumentationCookie()

namespace InspectorInstrumentation {

bool isDebuggerPaused(LocalFrame* frame)
bool isDebuggerPaused(LocalFrame*)
{
InstrumentingSessions* instrumentingSessions = instrumentingSessionsFor(frame);
if (!instrumentingSessions || instrumentingSessions->isEmpty())
return false;
for (InspectorSession* session : *instrumentingSessions) {
if (InspectorDebuggerAgent* debuggerAgent = session->instrumentingAgents()->inspectorDebuggerAgent())
return debuggerAgent->isPaused();
}
return false;
return MainThreadDebugger::instance()->debugger()->isPaused();
}

void didReceiveResourceResponseButCanceled(LocalFrame* frame, DocumentLoader* loader, unsigned long identifier, const ResourceResponse& r)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ class XMLHttpRequest;
[DOMDebugger]
void didFireWebGLErrorOrWarning(Element*, const String& message);

[Debugger]
InspectorInstrumentationCookie willExecuteScript(ExecutionContext*, int scriptId);

[Debugger]
void didExecuteScript(const InspectorInstrumentationCookie&);

[Page]
void didUpdateLayout(LocalFrame*);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ int MainThreadDebugger::contextGroupId(LocalFrame* frame)
MainThreadDebugger* MainThreadDebugger::instance()
{
ASSERT(isMainThread());
v8::Isolate* isolate = V8PerIsolateData::mainThreadIsolate();
V8PerIsolateData* data = V8PerIsolateData::from(isolate);
V8PerIsolateData* data = V8PerIsolateData::from(V8PerIsolateData::mainThreadIsolate());
ASSERT(data->threadDebugger() && !data->threadDebugger()->isWorker());
return static_cast<MainThreadDebugger*>(data->threadDebugger());
}

Expand Down
14 changes: 14 additions & 0 deletions third_party/WebKit/Source/core/inspector/ThreadDebugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ ThreadDebugger::~ThreadDebugger()
{
}

void ThreadDebugger::willExecuteScript(v8::Isolate* isolate, int scriptId)
{
V8PerIsolateData* data = V8PerIsolateData::from(isolate);
if (data && data->threadDebugger())
data->threadDebugger()->debugger()->willExecuteScript(isolate->GetCurrentContext(), scriptId);
}

void ThreadDebugger::didExecuteScript(v8::Isolate* isolate)
{
V8PerIsolateData* data = V8PerIsolateData::from(isolate);
if (data && data->threadDebugger())
data->threadDebugger()->debugger()->didExecuteScript(isolate->GetCurrentContext());
}

void ThreadDebugger::eventListeners(v8::Local<v8::Value> value, V8EventListenerInfoList& result)
{
InspectorDOMDebuggerAgent::eventListenersInfoForTarget(m_isolate, value, result);
Expand Down
3 changes: 3 additions & 0 deletions third_party/WebKit/Source/core/inspector/ThreadDebugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ class CORE_EXPORT ThreadDebugger : public V8DebuggerClient {
explicit ThreadDebugger(v8::Isolate*);
~ThreadDebugger() override;

static void willExecuteScript(v8::Isolate*, int scriptId);
static void didExecuteScript(v8::Isolate*);

// V8DebuggerClient implementation.
void eventListeners(v8::Local<v8::Value>, V8EventListenerInfoList&) override;
String16 valueSubtype(v8::Local<v8::Value>) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,6 @@ void V8DebuggerAgentImpl::setSkipAllPauses(ErrorString*, bool skipped)
m_state->setBoolean(DebuggerAgentState::skipAllPauses, m_skipAllPauses);
}

bool V8DebuggerAgentImpl::isPaused()
{
return debugger().isPaused();
}

static PassOwnPtr<protocol::DictionaryValue> buildObjectForBreakpointCookie(const String16& url, int lineNumber, int columnNumber, const String16& condition, bool isRegex)
{
OwnPtr<protocol::DictionaryValue> breakpointObject = protocol::DictionaryValue::create();
Expand Down Expand Up @@ -759,8 +754,7 @@ void V8DebuggerAgentImpl::getCollectionEntries(ErrorString* errorString, const S

void V8DebuggerAgentImpl::schedulePauseOnNextStatement(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data)
{
ASSERT(enabled());
if (m_scheduledDebuggerStep == StepInto || m_javaScriptPauseScheduled || isPaused() || !debugger().breakpointsActivated())
if (!enabled() || m_scheduledDebuggerStep == StepInto || m_javaScriptPauseScheduled || debugger().isPaused() || !debugger().breakpointsActivated())
return;
m_breakReason = breakReason;
m_breakAuxData = data;
Expand All @@ -772,7 +766,7 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatement(const String16& breakReas
void V8DebuggerAgentImpl::schedulePauseOnNextStatementIfSteppingInto()
{
ASSERT(enabled());
if (m_scheduledDebuggerStep != StepInto || m_javaScriptPauseScheduled || isPaused())
if (m_scheduledDebuggerStep != StepInto || m_javaScriptPauseScheduled || debugger().isPaused())
return;
clearBreakDetails();
m_pausingOnNativeEvent = false;
Expand All @@ -783,7 +777,7 @@ void V8DebuggerAgentImpl::schedulePauseOnNextStatementIfSteppingInto()

void V8DebuggerAgentImpl::cancelPauseOnNextStatement()
{
if (m_javaScriptPauseScheduled || isPaused())
if (m_javaScriptPauseScheduled || debugger().isPaused())
return;
clearBreakDetails();
m_pausingOnNativeEvent = false;
Expand Down Expand Up @@ -814,7 +808,7 @@ void V8DebuggerAgentImpl::pause(ErrorString* errorString)
{
if (!checkEnabled(errorString))
return;
if (m_javaScriptPauseScheduled || isPaused())
if (m_javaScriptPauseScheduled || debugger().isPaused())
return;
clearBreakDetails();
m_javaScriptPauseScheduled = true;
Expand Down Expand Up @@ -1107,7 +1101,7 @@ void V8DebuggerAgentImpl::didExecuteScript()

void V8DebuggerAgentImpl::changeJavaScriptRecursionLevel(int step)
{
if (m_javaScriptPauseScheduled && !m_skipAllPauses && !isPaused()) {
if (m_javaScriptPauseScheduled && !m_skipAllPauses && !debugger().isPaused()) {
// Do not ever loose user's pause request until we have actually paused.
debugger().setPauseOnNextStatement(true);
}
Expand Down Expand Up @@ -1357,15 +1351,9 @@ void V8DebuggerAgentImpl::didContinue()
m_frontend->resumed();
}

bool V8DebuggerAgentImpl::canBreakProgram()
{
return debugger().canBreakProgram();
}

void V8DebuggerAgentImpl::breakProgram(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data)
{
ASSERT(enabled());
if (m_skipAllPauses || !m_pausedContext.IsEmpty() || isCurrentCallStackEmptyOrBlackboxed() || !debugger().breakpointsActivated())
if (!enabled() || m_skipAllPauses || !m_pausedContext.IsEmpty() || isCurrentCallStackEmptyOrBlackboxed() || !debugger().breakpointsActivated())
return;
m_breakReason = breakReason;
m_breakAuxData = data;
Expand All @@ -1377,7 +1365,7 @@ void V8DebuggerAgentImpl::breakProgram(const String16& breakReason, PassOwnPtr<p

void V8DebuggerAgentImpl::breakProgramOnException(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data)
{
if (m_debugger->getPauseOnExceptionsState() == V8DebuggerImpl::DontPauseOnExceptions)
if (!enabled() || m_debugger->getPauseOnExceptionsState() == V8DebuggerImpl::DontPauseOnExceptions)
return;
breakProgram(breakReason, data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ class V8DebuggerAgentImpl : public V8DebuggerAgent {
void restore() override;
void disable(ErrorString*) override;

bool isPaused() override;

// Part of the protocol.
void enable(ErrorString*) override;
void setBreakpointsActive(ErrorString*, bool active) override;
Expand Down Expand Up @@ -136,13 +134,10 @@ class V8DebuggerAgentImpl : public V8DebuggerAgent {

void schedulePauseOnNextStatement(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) override;
void cancelPauseOnNextStatement() override;
bool canBreakProgram() override;
void breakProgram(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) override;
void breakProgramOnException(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) override;
void willExecuteScript(int scriptId) override;
void didExecuteScript() override;

bool enabled() override;
bool enabled();
V8DebuggerImpl& debugger() override { return *m_debugger; }

void setBreakpointAt(const String16& scriptId, int lineNumber, int columnNumber, BreakpointSource, const String16& condition = String16());
Expand All @@ -163,6 +158,8 @@ class V8DebuggerAgentImpl : public V8DebuggerAgent {
void didParseSource(const V8DebuggerParsedScript&);
bool v8AsyncTaskEventsEnabled() const;
void didReceiveV8AsyncTaskEvent(v8::Local<v8::Context>, const String16& eventType, const String16& eventName, int id);
void willExecuteScript(int scriptId);
void didExecuteScript();

v8::Isolate* isolate() { return m_isolate; }
int maxAsyncCallChainDepth() { return m_maxAsyncCallStackDepth; }
Expand Down
12 changes: 12 additions & 0 deletions third_party/WebKit/Source/platform/v8_inspector/V8DebuggerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,18 @@ void V8DebuggerImpl::resetContextGroup(int contextGroupId)
m_contexts.remove(contextGroupId);
}

void V8DebuggerImpl::willExecuteScript(v8::Local<v8::Context> context, int scriptId)
{
if (V8DebuggerAgentImpl* agent = findEnabledDebuggerAgent(context))
agent->willExecuteScript(scriptId);
}

void V8DebuggerImpl::didExecuteScript(v8::Local<v8::Context> context)
{
if (V8DebuggerAgentImpl* agent = findEnabledDebuggerAgent(context))
agent->didExecuteScript();
}

PassOwnPtr<V8StackTrace> V8DebuggerImpl::captureStackTrace(size_t maxStackSize)
{
V8DebuggerAgentImpl* agent = findEnabledDebuggerAgent(m_isolate->GetCurrentContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class V8DebuggerImpl : public V8Debugger {
void debuggerAgentEnabled();
void debuggerAgentDisabled();

bool isPaused();
bool isPaused() override;
v8::Local<v8::Context> pausedContext() { return m_pausedContext; }

v8::MaybeLocal<v8::Value> functionScopes(v8::Local<v8::Function>);
Expand All @@ -110,6 +110,8 @@ class V8DebuggerImpl : public V8Debugger {
void contextCreated(const V8ContextInfo&) override;
void contextDestroyed(v8::Local<v8::Context>) override;
void resetContextGroup(int contextGroupId) override;
void willExecuteScript(v8::Local<v8::Context>, int scriptId) override;
void didExecuteScript(v8::Local<v8::Context>) override;
PassOwnPtr<V8StackTrace> createStackTrace(v8::Local<v8::StackTrace>, size_t maxStackSize) override;
PassOwnPtr<V8StackTrace> captureStackTrace(size_t maxStackSize) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ class PLATFORM_EXPORT V8Debugger {
virtual void contextDestroyed(v8::Local<v8::Context>) = 0;
// TODO(dgozman): remove this one.
virtual void resetContextGroup(int contextGroupId) = 0;
virtual void willExecuteScript(v8::Local<v8::Context>, int scriptId) = 0;
virtual void didExecuteScript(v8::Local<v8::Context>) = 0;

virtual PassOwnPtr<V8InspectorSession> connect(int contextGroupId) = 0;
virtual bool isPaused() = 0;

static v8::Local<v8::Symbol> scopeExtensionSymbol(v8::Isolate*);
static bool isCommandLineAPIMethod(const String16& name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,9 @@ class PLATFORM_EXPORT V8DebuggerAgent : public protocol::Backend::Debugger, publ
// API for the embedder to report native activities.
virtual void schedulePauseOnNextStatement(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) = 0;
virtual void cancelPauseOnNextStatement() = 0;
virtual bool canBreakProgram() = 0;
virtual void breakProgram(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) = 0;
virtual void breakProgramOnException(const String16& breakReason, PassOwnPtr<protocol::DictionaryValue> data) = 0;
virtual void willExecuteScript(int scriptId) = 0;
virtual void didExecuteScript() = 0;

virtual bool isPaused() = 0;
virtual bool enabled() = 0;
virtual V8Debugger& debugger() = 0;

// Async call stacks implementation.
Expand Down

0 comments on commit ac39500

Please sign in to comment.