Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: e02e121abf243cc5c461af6def0d30df24eadf81
https://github.com/WebKit/WebKit/commit/e02e121abf243cc5c461af6def0d30df24eadf81
Author: Qianlang Chen <qianlangc...@apple.com>
Date: 2025-03-18 (Tue, 18 Mar 2025)
Changed paths:
M Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp
M Source/JavaScriptCore/inspector/remote/RemoteConnectionToTarget.cpp
M Source/JavaScriptCore/inspector/remote/RemoteInspectionTarget.cpp
M Source/JavaScriptCore/inspector/remote/RemoteInspectionTarget.h
M Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
M Source/JavaScriptCore/inspector/remote/RemoteInspector.h
M
Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm
M Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
M Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
M Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
M Source/WTF/wtf/PlatformEnableCocoa.h
M Source/WebCore/inspector/InspectorController.cpp
M Source/WebCore/inspector/WorkerInspectorController.cpp
M Source/WebCore/inspector/WorkerInspectorController.h
M Source/WebCore/workers/service/context/SWContextManager.cpp
M Source/WebCore/workers/service/context/SWContextManager.h
M Source/WebCore/workers/service/context/ServiceWorkerDebuggable.cpp
M Source/WebCore/workers/service/context/ServiceWorkerDebuggable.h
M Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp
M Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.h
M Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp
M Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h
M Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm
M Source/WebKit/UIProcess/WebProcessProxy.h
M Source/WebKit/UIProcess/WebProcessProxy.messages.in
M Source/WebKit/WebProcess/Inspector/ServiceWorkerDebuggableProxy.cpp
M Source/WebKit/WebProcess/Inspector/ServiceWorkerDebuggableProxy.h
M Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp
M Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h
M Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in
Log Message:
-----------
Web Inspector: The new ServiceWorkerDebuggableProxy needs to adopt auto
inspection
rdar://145795990
https://bugs.webkit.org/show_bug.cgi?id=288776
Reviewed by BJ Burg.
This work adds support to automatically inspect or pause the
ServiceWorkerDebuggableProxy, alongside minor refactoring of existing
code around auto-inspection.
The squashed commit includes three patches:
(Patch 1 of 3) Make pausing for automatic inspection a responsibility
of the RemoteInspectionTarget rather than RemoteInspector.
To summarize how automatic inspection generally works:
1. The web content marks its debuggable as inspectable.
2. The debuggable blocks its thread before starting code execution.
3. The auto-launched frontend, after initializing, notifies the backend.
4. The backend unblocks the debuggable to resume its code execution.
As the newly introduced ServiceWorkerDebuggableProxy lives in the UI
process but controls a different thread, during step 2, it's the service
worker thread that would need to be blocked, while the UI process
remains free to handle incoming messages from the remote inspector host.
This patch builds the ground work for supporting pausing in a different
thread by moving the handling of pausing or unpausing into the
debuggable instead of implicitly in RemoteInspector. With this change:
- From the RemoteInspector's perspective, it only needs to manage the
set of debuggables waiting for a reply of auto-inspection policy.
Notably, it now tells the debuggable to unpause when auto-inspection
is rejected, rather than relying on the call to
pauseWaitingForAutomaticInspection
to return and assuming that it was a blocking operation.
- From the RemoteInspectionTarget's perspective, it should decide
what the pausing behavior is, defaulting to blocking the current
thread with a busy loop. The pause should then persist until an
explicit call of unpauseForResolvedAutomaticInspection.
With this patch, it'll be easier to let ServiceWorkerDebuggableProxy
later define a custom pausing behavior that controls a different thread.
I tested on both Mac and iOS, with an app that spawns a JSContext, that
this change preserves normal and automatic inspection with no noticeable
difference from before.
* Source/JavaScriptCore/inspector/remote/RemoteInspectionTarget.h:
* Source/JavaScriptCore/inspector/remote/RemoteInspectionTarget.cpp:
(Inspector::RemoteInspectionTarget::pauseWaitingForAutomaticInspection):
(Inspector::RemoteInspectionTarget::unpauseForResolvedAutomaticInspection):
(Inspector::RemoteInspectionTarget::unpauseForInitializedInspector): Deleted.
- Use m_isPausedWaitingForAutomaticInspection to explicitly handle
the period of pausing.
- Rename unpauseForResolvedAutomaticInspection to indicate that this
should be called in both the frontend-initialized case and, as of
now, the rejected case as well.
* Source/JavaScriptCore/inspector/remote/RemoteConnectionToTarget.cpp:
* Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:
(Inspector::RemoteConnectionToTarget::setup):
- Make sure to call setupCompleted when we've determined the answer
of isAutomaticInspection to clean up that hash set entry, since
it's no longer the debuggable's job to do this upon
frontend-initialized.
* Source/JavaScriptCore/inspector/remote/RemoteInspector.h:
* Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp:
* Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:
- Rename m_automaticInspectionCandidates since "pausing" is no longer
this object's responsibility, and this hash set is only here to
know which incoming inspection requests were automatic.
(Inspector::RemoteInspector::waitingForAutomaticInspection): Deleted.
- No longer used. Debuggables should not rely on this information
to remain paused.
(Inspector::RemoteInspector::setupFailed):
(Inspector::RemoteInspector::setupCompleted):
(Inspector::RemoteInspector::setPendingMainThreadInitialization):
(Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
(Inspector::RemoteInspector::sendAutomaticInspectionCandidateMessage):
(Inspector::RemoteInspector::stopInternal):
(Inspector::RemoteInspector::setupXPCConnectionIfNeeded):
(Inspector::RemoteInspector::xpcConnectionFailed):
(Inspector::RemoteInspector::receivedSetupMessage):
(Inspector::RemoteInspector::receivedAutomaticInspectionConfigurationMessage):
(Inspector::RemoteInspector::receivedAutomaticInspectionRejectMessage):
* Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp:
(Inspector::RemoteInspector::stopInternal):
* Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp:
(Inspector::RemoteInspector::stopInternal):
* Source/WebCore/inspector/InspectorController.cpp:
(WebCore::InspectorController::frontendInitialized):
* Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp:
(Inspector::JSGlobalObjectInspectorController::frontendInitialized):
* Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp:
(WebCore::ServiceWorkerInspectorProxy::connectToWorker):
- Adapt to renamed things.
---
(Patch 2 of 3) Move setup of ServiceWorkerDebuggable auto-inspection
into WebSWContextManagerConnection.
Implementing automatic inspection of the ServiceWorkerDebuggable
required marking the debuggable as inspectable and then handling when
to unpause. That work was originally done inside ServiceWorkerThreadProxy.
As we plan to implement the same function for ServiceWorkerDebuggableProxy,
which is created and largely controlled in WebSWContextManagerConnection,
it might be more readable and logical to have both implementations in
that class instead.
* Source/WebCore/workers/service/context/SWContextManager.cpp:
(WebCore::SWContextManager::registerServiceWorkerThreadForInstall):
(WebCore::SWContextManager::stopRunningDebuggerTasksOnServiceWorker):
* Source/WebCore/workers/service/context/SWContextManager.h:
(WebCore::SWContextManager::registerServiceWorkerThreadForInstall):
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:
(WebCore::ServiceWorkerThreadProxy::ServiceWorkerThreadProxy):
(WebCore::ServiceWorkerThreadProxy::threadStartedRunningDebuggerTasks): Deleted.
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h:
* Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:
(WebKit::WebSWContextManagerConnection::installServiceWorker):
- Move the work of handling the debuggable (steps 2 and 4) into
WebSWContextManagerConnection
where more implementation is anticipated.
- Lay down a basic code structure for handling auto-inspection for
when either the Debuggable or the DebuggableProxy is used.
* Source/WTF/wtf/PlatformEnableCocoa.h:
- Add back the feature flag here as my work should end up providing
support for both ServiceWorkerDebuggable and ServiceWorkerDebuggableProxy.
---
(Patch 3 of 3) Add support for auto-inspecting ServiceWorkerDebuggableProxy.
This patch adds support to automatically inspect or pause a
ServiceWorkerDebuggableProxy in a similar fashion to the same features
for JSGlobalObjectInspectorController and ServiceWorkerDebuggable.
The ServiceWorkerDebuggableProxy is unique in that, in step 2, it should
keep the UI process unblocked to handle incoming remote inspector
messages including the auto-inspection policy, while controlling when
to unblock the actual service worker thread. This is done by two IPC
messages:
- The async callback of CreateServiceWorkerDebuggable, which can
indicate to unpause because there's no eligible remote inspector
at all.
- The new message UnpauseServiceWorkerForRejectedAutomaticInspection can
request unpause when the policy is received and it's a rejection.
These messages are from the UI process into the web process so they
should be relatively safe. If neither of the above two cases are met,
that means auto-inspection was accepted, and the web process will listen
to the frontendInitialized event to unblock the service worker thread.
I tested on Mac and iOS with a inspector host having Develop menu
on or off, or without support of auto-inspecting service workers and
didn't experience problems or hangs like what my last attempt introduced.
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.messages.in:
* Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm:
(WebKit::WebProcessProxy::createServiceWorkerDebuggable):
* Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h:
* Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in:
* Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:
(WebKit::WebSWContextManagerConnection::installServiceWorker):
(WebKit::WebSWContextManagerConnection::unpauseServiceWorkerForRejectedAutomaticInspection):
- Step 1 and 2, creating and blocking the thread as needed.
(WebKit::WebSWContextManagerConnection::connectToInspector):
* Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.h:
* Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp:
(WebCore::ServiceWorkerInspectorProxy::connectToWorker):
* Source/WebKit/WebProcess/Inspector/ServiceWorkerDebuggableProxy.h:
* Source/WebKit/WebProcess/Inspector/ServiceWorkerDebuggableProxy.cpp:
(WebKit::ServiceWorkerDebuggableProxy::connect):
- Pass the isAutomaticInspection and immediatelyPause flags.
(WebKit::ServiceWorkerDebuggableProxy::pauseWaitingForAutomaticInspection):
(WebKit::ServiceWorkerDebuggableProxy::unpauseForResolvedAutomaticInspection):
- Customize pausing behavior for ServiceWorkerDebuggableProxy.
- The m_isPausedWaitingForAutomaticInspection is repurposed to
represent if pausing was requested, and it's never marked to false
again because that'd need a message from the web process upon
unpausing which is not worth the effort for now.
(WebKit::ServiceWorkerDebuggableProxy::disconnect):
(WebKit::ServiceWorkerDebuggableProxy::dispatchMessageFromRemote):
- Following pauseWaitingForAutomaticInspection, make logging more
descriptive.
* Source/WebCore/inspector/WorkerInspectorController.cpp:
(WebCore::WorkerInspectorController::frontendInitialized):
- In step 4, unblock the paused service worker thread.
(WebCore::WorkerInspectorController::connectFrontend):
(WebCore::WorkerInspectorController::disconnectFrontend):
* Source/WebCore/inspector/WorkerInspectorController.h:
* Source/WebCore/workers/service/context/ServiceWorkerDebuggable.cpp:
(WebCore::ServiceWorkerDebuggable::connect):
- The unpausing logic turned out to work for both the
ServiceWorkerDebuggableProxy
and the old debuggable, so we can remove needing the
m_frontendInitializedCallback.
* Source/WebCore/workers/service/context/ServiceWorkerDebuggable.h:
- Don't support auto-inspecting the old debuggable when the new proxy
is used.
Canonical link: https://commits.webkit.org/292318@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
