Skip to content

Commit 0f0653d

Browse files
basukebasuke
committed
Reland: Inspector agents should use CheckedRef for InspectorEnvironment references.
https://bugs.webkit.org/show_bug.cgi?id=302416 rdar://165006365 Reviewed by Nobody (OOPS). Inspector agents hold references to their InspectorEnvironment through the AgentContext structure. This changes the raw reference to CheckedRef and WeakRef to provide compile time and runtime lifetime checking. The classes that had InspectorEnvironment reference are changed to store it as WeakRef. When using that, checkedEnvironment() is used to ensure the validity of address during its lifecycle. Because XxxInspectorController classes are the concrete classes of InspectorEnvironment, they are changed to be CheckedPtr-ready and WeakPtr-ready classes. * Source/JavaScriptCore/API/APIUtils.h: (handleExceptionIfNeeded): (setException): * Source/JavaScriptCore/API/JSBase.cpp: (JSEvaluateScriptInternal): (JSCheckScriptSyntax): * Source/JavaScriptCore/API/JSValue.mm: (reportExceptionToInspector): * Source/JavaScriptCore/inspector/InjectedScript.cpp: (Inspector::InjectedScript::execute): (Inspector::InjectedScript::evaluate): (Inspector::InjectedScript::awaitPromise): (Inspector::InjectedScript::callFunctionOn): (Inspector::InjectedScript::evaluateOnCallFrame): (Inspector::InjectedScript::getFunctionDetails): (Inspector::InjectedScript::functionDetails): (Inspector::InjectedScript::getPreview): (Inspector::InjectedScript::getProperties): (Inspector::InjectedScript::getDisplayableProperties): (Inspector::InjectedScript::getInternalProperties): (Inspector::InjectedScript::getCollectionEntries): (Inspector::InjectedScript::saveResult): (Inspector::InjectedScript::wrapCallFrames const): (Inspector::InjectedScript::wrapObject const): (Inspector::InjectedScript::wrapJSONString const): (Inspector::InjectedScript::wrapTable const): (Inspector::InjectedScript::previewValue const): (Inspector::InjectedScript::setEventValue): (Inspector::InjectedScript::clearEventValue): (Inspector::InjectedScript::setExceptionValue): (Inspector::InjectedScript::clearExceptionValue): (Inspector::InjectedScript::findObjectById const): (Inspector::InjectedScript::inspectObject): (Inspector::InjectedScript::releaseObject): (Inspector::InjectedScript::releaseObjectGroup): (Inspector::InjectedScript::createCommandLineAPIObject const): * Source/JavaScriptCore/inspector/InjectedScriptBase.cpp: (Inspector::InjectedScriptBase::hasAccessToInspectedScriptState const): * Source/JavaScriptCore/inspector/InjectedScriptBase.h: (Inspector::InjectedScriptBase::inspectorEnvironment const): (Inspector::InjectedScriptBase::checkedInspectorEnvironment const): * Source/JavaScriptCore/inspector/InjectedScriptManager.cpp: (Inspector::InjectedScriptManager::InjectedScriptManager): (Inspector::InjectedScriptManager::injectedScriptFor): * Source/JavaScriptCore/inspector/InjectedScriptManager.h: (Inspector::InjectedScriptManager::inspectorEnvironment const): (Inspector::InjectedScriptManager::checkedInspectorEnvironment const): * Source/JavaScriptCore/inspector/InjectedScriptModule.cpp: (Inspector::InjectedScriptModule::ensureInjected): * Source/JavaScriptCore/inspector/InspectorAgentBase.h: * Source/JavaScriptCore/inspector/InspectorEnvironment.h: * Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.h: * Source/JavaScriptCore/inspector/agents/InspectorAgent.cpp: (Inspector::InspectorAgent::initialized): * Source/JavaScriptCore/inspector/agents/InspectorAgent.h: * Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp: (Inspector::InspectorAuditAgent::InspectorAuditAgent): * Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp: (Inspector::InspectorDebuggerAgent::InspectorDebuggerAgent): (Inspector::InspectorDebuggerAgent::didPause): (Inspector::InspectorDebuggerAgent::breakpointActionProbe): (Inspector::InspectorDebuggerAgent::didContinue): * Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp: (Inspector::InspectorHeapAgent::enable): (Inspector::InspectorHeapAgent::disable): (Inspector::InspectorHeapAgent::gc): (Inspector::InspectorHeapAgent::snapshot): (Inspector::InspectorHeapAgent::nodeForHeapObjectIdentifier): (Inspector::InspectorHeapAgent::getPreview): (Inspector::InspectorHeapAgent::getRemoteObject): (Inspector::InspectorHeapAgent::willGarbageCollect): (Inspector::InspectorHeapAgent::didGarbageCollect): (Inspector::InspectorHeapAgent::heapSnapshotBuilderIgnoreNode): (Inspector::InspectorHeapAgent::clearHeapSnapshots): * Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.h: * Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp: (Inspector::InspectorRuntimeAgent::InspectorRuntimeAgent): (Inspector::m_vm): * Source/JavaScriptCore/inspector/agents/InspectorScriptProfilerAgent.cpp: (Inspector::InspectorScriptProfilerAgent::willDestroyFrontendAndBackend): (Inspector::InspectorScriptProfilerAgent::startTracking): (Inspector::InspectorScriptProfilerAgent::stopTracking): (Inspector::InspectorScriptProfilerAgent::willEvaluateScript): (Inspector::InspectorScriptProfilerAgent::didEvaluateScript): (Inspector::InspectorScriptProfilerAgent::trackingComplete): (Inspector::InspectorScriptProfilerAgent::stopSamplingWhenDisconnecting): * Source/JavaScriptCore/inspector/agents/InspectorScriptProfilerAgent.h: * Source/JavaScriptCore/runtime/JSGlobalObject.cpp: (JSC::JSGlobalObject::~JSGlobalObject): (JSC::JSGlobalObject::init): (JSC::JSGlobalObject::inspectorController const): (JSC::JSGlobalObject::checkedInspectorController const): * Source/JavaScriptCore/runtime/JSGlobalObject.h: (JSC::JSGlobalObject::inspectorController const): Deleted. * Source/JavaScriptCore/runtime/JSGlobalObjectDebuggable.cpp: (JSC::JSGlobalObjectDebuggable::connect): (JSC::JSGlobalObjectDebuggable::disconnect): (JSC::JSGlobalObjectDebuggable::dispatchMessageFromRemote): * Source/WebCore/inspector/FrameInspectorController.h: * Source/WebCore/inspector/InspectorInstrumentation.cpp: (WebCore::InspectorInstrumentation::frameWindowDiscardedImpl): (WebCore::InspectorInstrumentation::didReceiveResourceResponseImpl): (WebCore::InspectorInstrumentation::didFailLoadingImpl): (WebCore::InspectorInstrumentation::didCommitLoadImpl): (WebCore::InspectorInstrumentation::addMessageToConsoleImpl): (WebCore::InspectorInstrumentation::consoleCountImpl): (WebCore::InspectorInstrumentation::consoleCountResetImpl): (WebCore::InspectorInstrumentation::startConsoleTimingImpl): (WebCore::InspectorInstrumentation::logConsoleTimingImpl): (WebCore::InspectorInstrumentation::stopConsoleTimingImpl): * Source/WebCore/inspector/InspectorWebAgentBase.h: (WebCore::InspectorAgentBase::checkedEnvironment): * Source/WebCore/inspector/InstrumentingAgents.cpp: (WebCore::InstrumentingAgents::developerExtrasEnabled const): * Source/WebCore/inspector/InstrumentingAgents.h: (WebCore::InstrumentingAgents::checkedEnvironment const): (WebCore::InstrumentingAgents::inspectorEnvironment const): Deleted. * Source/WebCore/inspector/PageInspectorController.h: * Source/WebCore/inspector/WorkerInspectorController.h: * Source/WebCore/inspector/agents/InspectorAnimationAgent.cpp: (WebCore::InspectorAnimationAgent::startTracking): (WebCore::InspectorAnimationAgent::stopTracking): (WebCore::InspectorAnimationAgent::willApplyKeyframeEffect): (WebCore::InspectorAnimationAgent::stopTrackingStyleOriginatedAnimation): * Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp: (WebCore::InspectorCPUProfilerAgent::startTracking): (WebCore::InspectorCPUProfilerAgent::stopTracking): (WebCore::InspectorCPUProfilerAgent::collectSample): * Source/WebCore/inspector/agents/InspectorDOMAgent.cpp: (WebCore::InspectorDOMAgent::mediaMetricsTimerFired): * Source/WebCore/inspector/agents/InspectorMemoryAgent.cpp: (WebCore::InspectorMemoryAgent::startTracking): (WebCore::InspectorMemoryAgent::stopTracking): (WebCore::InspectorMemoryAgent::didHandleMemoryPressure): (WebCore::InspectorMemoryAgent::collectSample): * Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp: (WebCore::InspectorNetworkAgent::buildObjectForTiming): (WebCore::InspectorNetworkAgent::timestamp): (WebCore::InspectorNetworkAgent::didFinishLoading): * Source/WebCore/inspector/agents/InspectorPageAgent.cpp: (WebCore::InspectorPageAgent::enable): (WebCore::InspectorPageAgent::timestamp): * Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp: (WebCore::InspectorTimelineAgent::internalStart): (WebCore::InspectorTimelineAgent::internalStop): (WebCore::InspectorTimelineAgent::timestamp): (WebCore::InspectorTimelineAgent::timestampFromMonotonicTime): * Source/WebCore/inspector/agents/page/PageTimelineAgent.cpp: (WebCore::PageTimelineAgent::internalStart): (WebCore::PageTimelineAgent::didCompleteRenderingFrame): * Source/WebKitLegacy/SaferCPPExpectations/UncountedCallArgsCheckerExpectations: * Source/WebKitLegacy/mac/WebInspector/WebNodeHighlightView.mm: (-[WebNodeHighlightView drawRect:]): Canonical link: https://commits.webkit.org/304015@main
1 parent c30e8e0 commit 0f0653d

41 files changed

Lines changed: 206 additions & 135 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Source/JavaScriptCore/API/APIUtils.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ inline ExceptionStatus handleExceptionIfNeeded(JSC::CatchScope& scope, JSContext
4646
*returnedExceptionRef = toRef(globalObject, exception->value());
4747
scope.clearException();
4848
#if ENABLE(REMOTE_INSPECTOR)
49-
globalObject->inspectorController().reportAPIException(globalObject, exception);
49+
globalObject->checkedInspectorController()->reportAPIException(globalObject, exception);
5050
#endif
5151
return ExceptionStatus::DidThrow;
5252
}
@@ -60,7 +60,7 @@ inline void setException(JSContextRef ctx, JSValueRef* returnedExceptionRef, JSC
6060
*returnedExceptionRef = toRef(globalObject, exception);
6161
#if ENABLE(REMOTE_INSPECTOR)
6262
JSC::VM& vm = getVM(globalObject);
63-
globalObject->inspectorController().reportAPIException(globalObject, JSC::Exception::create(vm, exception));
63+
globalObject->checkedInspectorController()->reportAPIException(globalObject, JSC::Exception::create(vm, exception));
6464
#endif
6565
}
6666

‎Source/JavaScriptCore/API/JSBase.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ JSValueRef JSEvaluateScriptInternal(const JSLockHolder&, JSContextRef ctx, JSObj
6161
// Debugger path is currently ignored by inspector.
6262
// NOTE: If we don't have a debugger, this SourceCode will be forever lost to the inspector.
6363
// We could stash it in the inspector in case an inspector is ever opened.
64-
globalObject->inspectorController().reportAPIException(globalObject, evaluationException);
64+
globalObject->checkedInspectorController()->reportAPIException(globalObject, evaluationException);
6565
#endif
6666
return nullptr;
6767
}
@@ -114,7 +114,7 @@ bool JSCheckScriptSyntax(JSContextRef ctx, JSStringRef script, JSStringRef sourc
114114
*exception = toRef(globalObject, syntaxException);
115115
#if ENABLE(REMOTE_INSPECTOR)
116116
Exception* exception = Exception::create(vm, syntaxException);
117-
globalObject->inspectorController().reportAPIException(globalObject, exception);
117+
globalObject->checkedInspectorController()->reportAPIException(globalObject, exception);
118118
#endif
119119
return false;
120120
}

‎Source/JavaScriptCore/API/JSValue.mm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -882,7 +882,7 @@ static void reportExceptionToInspector(JSGlobalContextRef context, JSC::JSValue
882882
JSC::JSGlobalObject* globalObject = toJS(context);
883883
JSC::VM& vm = globalObject->vm();
884884
JSC::Exception* exception = JSC::Exception::create(vm, exceptionValue);
885-
globalObject->inspectorController().reportAPIException(globalObject, exception);
885+
globalObject->checkedInspectorController()->reportAPIException(globalObject, exception);
886886
}
887887
#endif
888888

‎Source/JavaScriptCore/inspector/InjectedScript.cpp‎

Lines changed: 27 additions & 27 deletions
Large diffs are not rendered by default.

‎Source/JavaScriptCore/inspector/InjectedScriptBase.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,9 @@ InjectedScriptBase::~InjectedScriptBase() = default;
126126

127127
bool InjectedScriptBase::hasAccessToInspectedScriptState() const
128128
{
129-
return m_environment && m_environment->canAccessInspectedScriptState(m_globalObject);
129+
if (CheckedPtr environment = m_environment.get())
130+
return environment->canAccessInspectedScriptState(m_globalObject);
131+
return false;
130132
}
131133

132134
JSC::JSObject* InjectedScriptBase::injectedScriptObject() const

‎Source/JavaScriptCore/inspector/InjectedScriptBase.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ class InjectedScriptBase {
6666
InjectedScriptBase(const String& name);
6767
InjectedScriptBase(const String& name, JSC::JSGlobalObject*, JSC::JSObject*, InspectorEnvironment*);
6868

69-
InspectorEnvironment* inspectorEnvironment() const { return m_environment; }
69+
InspectorEnvironment& inspectorEnvironment() const { return *m_environment.get(); }
70+
CheckedRef<InspectorEnvironment> checkedInspectorEnvironment() const { return inspectorEnvironment(); }
7071

7172
bool hasAccessToInspectedScriptState() const;
7273

@@ -83,7 +84,7 @@ class InjectedScriptBase {
8384
String m_name;
8485
JSC::JSGlobalObject* m_globalObject { nullptr };
8586
JSC::Strong<JSC::JSObject> m_injectedScriptObject;
86-
InspectorEnvironment* m_environment { nullptr };
87+
WeakPtr<InspectorEnvironment> m_environment;
8788
};
8889

8990
} // namespace Inspector

‎Source/JavaScriptCore/inspector/InjectedScriptManager.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ using namespace JSC;
4747
WTF_MAKE_TZONE_ALLOCATED_IMPL(InjectedScriptManager);
4848

4949
InjectedScriptManager::InjectedScriptManager(InspectorEnvironment& environment, Ref<InjectedScriptHost>&& injectedScriptHost)
50-
: m_environment(environment)
50+
: m_environment(&environment)
5151
, m_injectedScriptHost(WTFMove(injectedScriptHost))
5252
, m_nextInjectedScriptId(1)
5353
{
@@ -173,7 +173,7 @@ InjectedScript InjectedScriptManager::injectedScriptFor(JSGlobalObject* globalOb
173173
return it1->value;
174174
}
175175

176-
if (!m_environment.canAccessInspectedScriptState(globalObject))
176+
if (!checkedInspectorEnvironment()->canAccessInspectedScriptState(globalObject))
177177
return InjectedScript();
178178

179179
int id = injectedScriptIdFor(globalObject);
@@ -197,7 +197,7 @@ InjectedScript InjectedScriptManager::injectedScriptFor(JSGlobalObject* globalOb
197197
RELEASE_ASSERT_NOT_REACHED();
198198
}
199199

200-
InjectedScript result(globalObject, createResult.value(), &m_environment);
200+
InjectedScript result(globalObject, createResult.value(), m_environment.get());
201201
m_idToInjectedScript.set(id, result);
202202
didCreateInjectedScript(result);
203203
return result;

‎Source/JavaScriptCore/inspector/InjectedScriptManager.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ class InjectedScriptManager {
5959
JS_EXPORT_PRIVATE virtual void discardInjectedScripts();
6060

6161
InjectedScriptHost& injectedScriptHost();
62-
InspectorEnvironment& inspectorEnvironment() const { return m_environment; }
62+
InspectorEnvironment& inspectorEnvironment() const { return *m_environment.get(); }
63+
CheckedRef<InspectorEnvironment> checkedInspectorEnvironment() const { return inspectorEnvironment(); }
6364

6465
JS_EXPORT_PRIVATE InjectedScript injectedScriptFor(JSC::JSGlobalObject*);
6566
JS_EXPORT_PRIVATE InjectedScript injectedScriptForId(int);
@@ -78,7 +79,7 @@ class InjectedScriptManager {
7879
private:
7980
Expected<JSC::JSObject*, NakedPtr<JSC::Exception>> createInjectedScript(JSC::JSGlobalObject*, int id);
8081

81-
InspectorEnvironment& m_environment;
82+
WeakPtr<InspectorEnvironment> m_environment;
8283
const Ref<InjectedScriptHost> m_injectedScriptHost;
8384
int m_nextInjectedScriptId;
8485
};

‎Source/JavaScriptCore/inspector/InjectedScriptModule.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ void InjectedScriptModule::ensureInjected(InjectedScriptManager* injectedScriptM
5959

6060
// FIXME: Make the InjectedScript a module itself.
6161
JSC::JSLockHolder locker(injectedScript.globalObject());
62-
ScriptFunctionCall function(injectedScript.globalObject(), injectedScript.injectedScriptObject(), "hasInjectedModule"_s, injectedScriptManager->inspectorEnvironment().functionCallHandler());
62+
ScriptFunctionCall function(injectedScript.globalObject(), injectedScript.injectedScriptObject(), "hasInjectedModule"_s, injectedScriptManager->checkedInspectorEnvironment()->functionCallHandler());
6363
function.appendArgument(name());
6464
auto hasInjectedModuleResult = injectedScript.callFunctionWithEvalEnabled(function);
6565
ASSERT(hasInjectedModuleResult);
@@ -78,7 +78,7 @@ void InjectedScriptModule::ensureInjected(InjectedScriptManager* injectedScriptM
7878
RELEASE_ASSERT_NOT_REACHED();
7979
}
8080
if (!hasInjectedModuleResult.value().isBoolean() || !hasInjectedModuleResult.value().asBoolean()) {
81-
ScriptFunctionCall function(injectedScript.globalObject(), injectedScript.injectedScriptObject(), "injectModule"_s, injectedScriptManager->inspectorEnvironment().functionCallHandler());
81+
ScriptFunctionCall function(injectedScript.globalObject(), injectedScript.injectedScriptObject(), "injectModule"_s, injectedScriptManager->checkedInspectorEnvironment()->functionCallHandler());
8282
function.appendArgument(name());
8383
function.appendArgument(JSC::JSValue(injectModuleFunction(injectedScript.globalObject())));
8484
function.appendArgument(host(injectedScriptManager, injectedScript.globalObject()));

‎Source/JavaScriptCore/inspector/InspectorAgentBase.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#pragma once
2828

29+
#include <JavaScriptCore/InspectorEnvironment.h>
2930
#include <JavaScriptCore/InspectorFrontendRouter.h>
3031
#include <wtf/CheckedRef.h>
3132
#include <wtf/TZoneMalloc.h>
@@ -39,10 +40,9 @@ namespace Inspector {
3940

4041
class BackendDispatcher;
4142
class InjectedScriptManager;
42-
class InspectorEnvironment;
4343

4444
struct AgentContext {
45-
InspectorEnvironment& environment;
45+
CheckedRef<InspectorEnvironment> environment;
4646
InjectedScriptManager& injectedScriptManager;
4747
CheckedRef<FrontendRouter> frontendRouter;
4848
BackendDispatcher& backendDispatcher;

0 commit comments

Comments
 (0)