Skip to content

Commit

Permalink
[DevTools] Do not instrument context creation through InspectorRuntim…
Browse files Browse the repository at this point in the history
…eAgent.

Instrumentation is done through V8Debugger instead.
This leaves only a tiny bit of logic into InspectorRuntimeAgent, which can be
easily removed now.

BUG=580337

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

Cr-Commit-Position: refs/heads/master@{#380728}
  • Loading branch information
dgozman authored and Commit bot committed Mar 11, 2016
1 parent 15e94e2 commit a8b8995
Show file tree
Hide file tree
Showing 44 changed files with 293 additions and 211 deletions.
5 changes: 2 additions & 3 deletions third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ void WindowProxy::disposeContext(GlobalDetachmentBehavior behavior)
LocalFrame* frame = toLocalFrame(m_frame);
// The embedder could run arbitrary code in response to the willReleaseScriptContext callback, so all disposing should happen after it returns.
frame->loader().client()->willReleaseScriptContext(context, m_world->worldId());
InspectorInstrumentation::willReleaseScriptContext(frame, m_scriptState.get());
MainThreadDebugger::contextWillBeDestroyed(m_scriptState.get());
}

m_document.clear();
Expand Down Expand Up @@ -269,8 +269,7 @@ bool WindowProxy::initialize()
}
if (m_frame->isLocalFrame()) {
LocalFrame* frame = toLocalFrame(m_frame);
MainThreadDebugger::initializeContext(context, frame, m_world->worldId());
InspectorInstrumentation::didCreateScriptContext(frame, m_scriptState.get(), origin, m_world->worldId());
MainThreadDebugger::contextCreated(m_scriptState.get(), frame, origin);
frame->loader().client()->didCreateScriptContext(context, m_world->extensionGroup(), m_world->worldId());
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ namespace DebuggerAgentState {
static const char debuggerEnabled[] = "debuggerEnabled";
}

InspectorDebuggerAgent::InspectorDebuggerAgent(V8RuntimeAgent* runtimeAgent, V8Debugger* debugger, int contextGroupId)
InspectorDebuggerAgent::InspectorDebuggerAgent(V8RuntimeAgent* runtimeAgent)
: InspectorBaseAgent<InspectorDebuggerAgent, protocol::Frontend::Debugger>("Debugger")
, m_v8DebuggerAgent(V8DebuggerAgent::create(runtimeAgent, contextGroupId))
, m_debugger(debugger)
, m_v8DebuggerAgent(V8DebuggerAgent::create(runtimeAgent))
{
}

Expand Down Expand Up @@ -360,6 +359,11 @@ void InspectorDebuggerAgent::restore()
setTrackingAsyncCalls(m_v8DebuggerAgent->trackingAsyncCalls());
}

void InspectorDebuggerAgent::discardAgent()
{
m_v8DebuggerAgent.clear();
}

void InspectorDebuggerAgent::setTrackingAsyncCalls(bool tracking)
{
m_asyncCallTracker->asyncCallTrackingStateChanged(tracking);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,22 +93,21 @@ class CORE_EXPORT InspectorDebuggerAgent
void setFrontend(protocol::Frontend*) override;
void clearFrontend() override;
void restore() override;
void discardAgent() override;

V8DebuggerAgent* v8Agent() const { return m_v8DebuggerAgent.get(); }

virtual void muteConsole() = 0;
virtual void unmuteConsole() = 0;

V8Debugger* debugger() { return m_debugger; }
protected:
InspectorDebuggerAgent(V8RuntimeAgent*, V8Debugger*, int contextGroupId);
explicit InspectorDebuggerAgent(V8RuntimeAgent*);

OwnPtr<V8DebuggerAgent> m_v8DebuggerAgent;
OwnPtrWillBeMember<AsyncCallTracker> m_asyncCallTracker;

private:
void setTrackingAsyncCalls(bool);
V8Debugger* m_debugger;
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ void InspectorHeapProfilerAgent::restore()
startUpdateStatsTimer();
}

void InspectorHeapProfilerAgent::discardAgent()
{
m_v8HeapProfilerAgent.clear();
}

// Protocol implementation.
void InspectorHeapProfilerAgent::collectGarbage(ErrorString* error)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class CORE_EXPORT InspectorHeapProfilerAgent final : public InspectorBaseAgent<I
void setFrontend(protocol::Frontend*) override;
void clearFrontend() override;
void restore() override;
void discardAgent() override;

void enable(ErrorString*) override;
void disable(ErrorString*) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,6 @@ interface InspectorInstrumentation {
[DOMDebugger, Inline=FastReturn]
void willEvaluateScript(ExecutionContext*);

[PageRuntime]
void didCreateScriptContext([Keep] LocalFrame*, ScriptState*, SecurityOrigin*, int worldId);

[PageRuntime, Inline=FastReturn]
void willReleaseScriptContext([Keep] LocalFrame*, ScriptState*);

[AsyncCallTracker, DOMDebugger, Inline=FastReturn]
InspectorInstrumentationCookie willFireTimer([Keep] ExecutionContext*, int timerId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ void InspectorPageAgent::searchContentAfterResourcesContentLoaded(const String&
}

OwnPtr<protocol::Array<protocol::Debugger::SearchMatch>> results;
results = V8ContentSearchUtil::searchInTextByLines(m_debuggerAgent->debugger(), content, query, caseSensitive, isRegex);
results = V8ContentSearchUtil::searchInTextByLines(&m_debuggerAgent->v8Agent()->debugger(), content, query, caseSensitive, isRegex);
callback->sendSuccess(results.release());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ void InspectorProfilerAgent::restore()
enable(&errorString);
}

void InspectorProfilerAgent::discardAgent()
{
m_v8ProfilerAgent.clear();
}

// Protocol implementation.
void InspectorProfilerAgent::consoleProfile(ExecutionContext* context, const String16& title)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class CORE_EXPORT InspectorProfilerAgent final : public InspectorBaseAgent<Inspe
void setFrontend(protocol::Frontend*) override;
void clearFrontend() override;
void restore() override;
void discardAgent() override;

void consoleProfile(ExecutionContext*, const String16& title);
void consoleProfileEnd(const String16& title);
Expand Down
21 changes: 7 additions & 14 deletions third_party/WebKit/Source/core/inspector/InspectorRuntimeAgent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ namespace InspectorRuntimeAgentState {
static const char runtimeEnabled[] = "runtimeEnabled";
};

InspectorRuntimeAgent::InspectorRuntimeAgent(V8Debugger* debugger, Client* client)
InspectorRuntimeAgent::InspectorRuntimeAgent(V8Debugger* debugger, Client* client, int contextGroupId)
: InspectorBaseAgent<InspectorRuntimeAgent, protocol::Frontend::Runtime>("Runtime")
, m_enabled(false)
, m_v8RuntimeAgent(V8RuntimeAgent::create(debugger, this))
, m_v8RuntimeAgent(V8RuntimeAgent::create(debugger, contextGroupId))
, m_client(client)
{
}
Expand Down Expand Up @@ -83,6 +83,11 @@ void InspectorRuntimeAgent::restore()
enable(&errorString);
}

void InspectorRuntimeAgent::discardAgent()
{
m_v8RuntimeAgent.clear();
}

void InspectorRuntimeAgent::evaluate(ErrorString* errorString,
const String16& expression,
const Maybe<String16>& objectGroup,
Expand Down Expand Up @@ -205,16 +210,4 @@ void InspectorRuntimeAgent::disable(ErrorString* errorString)
m_v8RuntimeAgent->disable(errorString);
}

void InspectorRuntimeAgent::reportExecutionContextCreated(ScriptState* scriptState, const String16& type, const String16& origin, const String16& humanReadableName, const String16& frameId)
{
v8::HandleScope handles(scriptState->isolate());
m_v8RuntimeAgent->reportExecutionContextCreated(scriptState->context(), type, origin, humanReadableName, frameId);
}

void InspectorRuntimeAgent::reportExecutionContextDestroyed(ScriptState* scriptState)
{
v8::HandleScope handles(scriptState->isolate());
m_v8RuntimeAgent->reportExecutionContextDestroyed(scriptState->context());
}

} // namespace blink
12 changes: 3 additions & 9 deletions third_party/WebKit/Source/core/inspector/InspectorRuntimeAgent.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ using protocol::Maybe;

class CORE_EXPORT InspectorRuntimeAgent
: public InspectorBaseAgent<InspectorRuntimeAgent, protocol::Frontend::Runtime>
, public protocol::Dispatcher::RuntimeCommandHandler
, public V8RuntimeAgent::Client {
, public protocol::Dispatcher::RuntimeCommandHandler {
WTF_MAKE_NONCOPYABLE(InspectorRuntimeAgent);
public:
class Client {
Expand All @@ -65,14 +64,12 @@ class CORE_EXPORT InspectorRuntimeAgent

~InspectorRuntimeAgent() override;

// V8RuntimeAgent::Client.
void reportExecutionContexts() override { }

// InspectorBaseAgent overrides.
void setState(protocol::DictionaryValue*) override;
void setFrontend(protocol::Frontend*) override;
void clearFrontend() override;
void restore() override;
void discardAgent() override;

// Part of the protocol.
void evaluate(ErrorString*, const String16& expression, const Maybe<String16>& objectGroup, const Maybe<bool>& includeCommandLineAPI, const Maybe<bool>& doNotPauseOnExceptionsAndMuteConsole, const Maybe<int>& contextId, const Maybe<bool>& returnByValue, const Maybe<bool>& generatePreview, OwnPtr<protocol::Runtime::RemoteObject>* result, Maybe<bool>* wasThrown, Maybe<protocol::Runtime::ExceptionDetails>*) override;
Expand All @@ -93,12 +90,9 @@ class CORE_EXPORT InspectorRuntimeAgent
V8RuntimeAgent* v8Agent() { return m_v8RuntimeAgent.get(); }

protected:
InspectorRuntimeAgent(V8Debugger*, Client*);
InspectorRuntimeAgent(V8Debugger*, Client*, int contextGroupId);
virtual ScriptState* defaultScriptState() = 0;

void reportExecutionContextCreated(ScriptState*, const String16& type, const String16& origin, const String16& humanReadableName, const String16& frameId);
void reportExecutionContextDestroyed(ScriptState*);

bool m_enabled;
OwnPtr<V8RuntimeAgent> m_v8RuntimeAgent;
Client* m_client;
Expand Down
59 changes: 56 additions & 3 deletions third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@

#include "bindings/core/v8/BindingSecurity.h"
#include "bindings/core/v8/DOMWrapperWorld.h"
#include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/V8Window.h"
#include "core/frame/FrameConsole.h"
#include "core/frame/LocalDOMWindow.h"
#include "core/frame/LocalFrame.h"
#include "core/frame/UseCounter.h"
#include "core/inspector/IdentifiersFactory.h"
#include "core/inspector/InspectedFrames.h"
#include "core/inspector/InspectorTaskRunner.h"
#include "platform/UserGestureIndicator.h"
#include "platform/v8_inspector/public/V8Debugger.h"
Expand Down Expand Up @@ -83,10 +86,22 @@ Mutex& MainThreadDebugger::creationMutex()
return mutex;
}

void MainThreadDebugger::initializeContext(v8::Local<v8::Context> context, LocalFrame* frame, int worldId)
void MainThreadDebugger::contextCreated(ScriptState* scriptState, LocalFrame* frame, SecurityOrigin* origin)
{
String type = worldId == MainWorldId ? "page" : "injected";
V8Debugger::setContextDebugData(context, type, contextGroupId(frame));
ASSERT(isMainThread());
v8::HandleScope handles(scriptState->isolate());
DOMWrapperWorld& world = scriptState->world();
V8Debugger::setContextDebugData(scriptState->context(), world.isMainWorld() ? "page" : "injected", contextGroupId(frame));
if (s_instance)
s_instance->debugger()->contextCreated(V8ContextInfo(scriptState->context(), world.isMainWorld() ? "" : "Extension", origin ? origin->toRawString() : "", world.isIsolatedWorld() ? world.isolatedWorldHumanReadableName() : "", IdentifiersFactory::frameId(frame)));
}

void MainThreadDebugger::contextWillBeDestroyed(ScriptState* scriptState)
{
if (s_instance) {
v8::HandleScope handles(scriptState->isolate());
s_instance->debugger()->contextDestroyed(scriptState->context());
}
}

int MainThreadDebugger::contextGroupId(LocalFrame* frame)
Expand Down Expand Up @@ -147,4 +162,42 @@ bool MainThreadDebugger::callingContextCanAccessContext(v8::Local<v8::Context> c
return window && BindingSecurity::shouldAllowAccessTo(m_isolate, toLocalDOMWindow(toDOMWindow(calling)), window, DoNotReportSecurityError);
}

void MainThreadDebugger::contextsToReport(int contextGroupId, V8ContextInfoVector& contexts)
{
LocalFrame* root = WeakIdentifierMap<LocalFrame>::lookup(contextGroupId);
if (!root)
return;

// Only report existing contexts if the page did commit load, otherwise we may
// unintentionally initialize contexts in the frames which may trigger some listeners
// that are expected to be triggered only after the load is committed, see http://crbug.com/131623
if (!root->loader().stateMachine()->committedFirstRealDocumentLoad())
return;

OwnPtrWillBeRawPtr<InspectedFrames> inspectedFrames = InspectedFrames::create(root);

Vector<std::pair<ScriptState*, SecurityOrigin*>> isolatedContexts;
for (LocalFrame* frame : *inspectedFrames) {
if (!frame->script().canExecuteScripts(NotAboutToExecuteScript))
continue;
String frameId = IdentifiersFactory::frameId(frame);

// Ensure execution context is created.
// If initializeMainWorld returns true, then is registered by didCreateScriptContext
if (!frame->script().initializeMainWorld()) {
ScriptState* scriptState = ScriptState::forMainWorld(frame);
contexts.append(V8ContextInfo(scriptState->context(), "", "", "", frameId));
}
frame->script().collectIsolatedContexts(isolatedContexts);
if (isolatedContexts.isEmpty())
continue;
for (const auto& pair : isolatedContexts) {
String originString = pair.second ? pair.second->toRawString() : "";
ScriptState* scriptState = pair.first;
contexts.append(V8ContextInfo(scriptState->context(), "Extension", originString, scriptState->world().isolatedWorldHumanReadableName(), frameId));
}
isolatedContexts.clear();
}
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#ifndef MainThreadDebugger_h
#define MainThreadDebugger_h

#include "bindings/core/v8/ScriptState.h"
#include "core/CoreExport.h"
#include "core/inspector/InspectorTaskRunner.h"
#include "core/inspector/ThreadDebugger.h"
Expand All @@ -45,6 +46,7 @@ namespace blink {

class LocalFrame;
class V8Debugger;
class SecurityOrigin;

class CORE_EXPORT MainThreadDebugger final : public ThreadDebugger {
WTF_MAKE_NONCOPYABLE(MainThreadDebugger);
Expand All @@ -64,7 +66,8 @@ class CORE_EXPORT MainThreadDebugger final : public ThreadDebugger {

~MainThreadDebugger() override;

static void initializeContext(v8::Local<v8::Context>, LocalFrame*, int worldId);
static void contextCreated(ScriptState*, LocalFrame*, SecurityOrigin*);
static void contextWillBeDestroyed(ScriptState*);
static int contextGroupId(LocalFrame*);

static MainThreadDebugger* instance();
Expand All @@ -82,6 +85,7 @@ class CORE_EXPORT MainThreadDebugger final : public ThreadDebugger {
void muteWarningsAndDeprecations() override;
void unmuteWarningsAndDeprecations() override;
bool callingContextCanAccessContext(v8::Local<v8::Context> calling, v8::Local<v8::Context> target) override;
void contextsToReport(int contextGroupId, V8ContextInfoVector&) override;

static WTF::Mutex& creationMutex();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,20 @@
#include "core/inspector/InspectorInstrumentation.h"
#include "core/inspector/InspectorTraceEvents.h"
#include "core/inspector/InstrumentingAgents.h"
#include "core/inspector/MainThreadDebugger.h"
#include "core/loader/DocumentLoader.h"
#include "core/page/Page.h"

using blink::protocol::Runtime::RemoteObject;

namespace blink {

PassOwnPtrWillBeRawPtr<PageDebuggerAgent> PageDebuggerAgent::create(MainThreadDebugger* mainThreadDebugger, InspectedFrames* inspectedFrames, V8RuntimeAgent* runtimeAgent)
PassOwnPtrWillBeRawPtr<PageDebuggerAgent> PageDebuggerAgent::create(InspectedFrames* inspectedFrames, V8RuntimeAgent* runtimeAgent)
{
return adoptPtrWillBeNoop(new PageDebuggerAgent(mainThreadDebugger, inspectedFrames, runtimeAgent));
return adoptPtrWillBeNoop(new PageDebuggerAgent(inspectedFrames, runtimeAgent));
}

PageDebuggerAgent::PageDebuggerAgent(MainThreadDebugger* mainThreadDebugger, InspectedFrames* inspectedFrames, V8RuntimeAgent* runtimeAgent)
: InspectorDebuggerAgent(runtimeAgent, mainThreadDebugger->debugger(), mainThreadDebugger->contextGroupId(inspectedFrames->root()))
PageDebuggerAgent::PageDebuggerAgent(InspectedFrames* inspectedFrames, V8RuntimeAgent* runtimeAgent)
: InspectorDebuggerAgent(runtimeAgent)
, m_inspectedFrames(inspectedFrames)
{
}
Expand Down
5 changes: 2 additions & 3 deletions third_party/WebKit/Source/core/inspector/PageDebuggerAgent.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,13 @@ namespace blink {

class DocumentLoader;
class InspectedFrames;
class MainThreadDebugger;

class CORE_EXPORT PageDebuggerAgent final
: public InspectorDebuggerAgent {
WTF_MAKE_NONCOPYABLE(PageDebuggerAgent);
USING_FAST_MALLOC_WILL_BE_REMOVED(PageDebuggerAgent);
public:
static PassOwnPtrWillBeRawPtr<PageDebuggerAgent> create(MainThreadDebugger*, InspectedFrames*, V8RuntimeAgent*);
static PassOwnPtrWillBeRawPtr<PageDebuggerAgent> create(InspectedFrames*, V8RuntimeAgent*);
~PageDebuggerAgent() override;
DECLARE_VIRTUAL_TRACE();

Expand All @@ -61,7 +60,7 @@ class CORE_EXPORT PageDebuggerAgent final
void didClearDocumentOfWindowObject(LocalFrame*);

private:
PageDebuggerAgent(MainThreadDebugger*, InspectedFrames*, V8RuntimeAgent*);
PageDebuggerAgent(InspectedFrames*, V8RuntimeAgent*);
void muteConsole() override;
void unmuteConsole() override;

Expand Down
Loading

0 comments on commit a8b8995

Please sign in to comment.