Skip to content

Commit

Permalink
Minor refactoring of assertion code, v2
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263797
rdar://117600174

Reviewed by Brent Fulgham.

This patch contains some minor refactoring of assertions code:

1) Let ProcessAndUIAssertion create method take an AuxiliaryProcessProxy reference, instead of a PID.
2) Let XPCConnectionTerminationWatchdog hold on to a weak AuxiliaryProcessProxy pointer, instead of an XPC connection.
3) Let ProcessThrottler::didConnectToProcess take an AuxiliaryProcessProxy reference, instead of a PID.

* Source/WebKit/Platform/cocoa/XPCUtilities.mm:
(WebKit::terminateWithReason):
* Source/WebKit/Platform/spi/Cocoa/ExtensionKitSPI.h:
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/Cocoa/AuxiliaryProcessProxyCocoa.mm:
(WebKit::AuxiliaryProcessProxy::platformStartConnectionTerminationWatchdog):
(WebKit::AuxiliaryProcessProxy::extensionProcess const):
(WebKit::AuxiliaryProcessProxy::extensionProcess): Deleted.
* Source/WebKit/UIProcess/Cocoa/ProcessAssertionCocoa.mm:
(WebKit::ProcessAndUIAssertion::ProcessAndUIAssertion):
* Source/WebKit/UIProcess/Cocoa/XPCConnectionTerminationWatchdog.h:
* Source/WebKit/UIProcess/Cocoa/XPCConnectionTerminationWatchdog.mm:
(WebKit::XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog):
(WebKit::XPCConnectionTerminationWatchdog::XPCConnectionTerminationWatchdog):
(WebKit::XPCConnectionTerminationWatchdog::watchdogTimerFired):
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/ProcessAssertion.h:
* Source/WebKit/UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::setThrottleState):
(WebKit::ProcessThrottler::updateThrottleStateIfNeeded):
(WebKit::ProcessThrottler::didConnectToProcess):
(WebKit::ProcessThrottler::didDisconnectFromProcess):
(WebKit::ProcessThrottler::sendPrepareToSuspendIPC):
(WebKit::ProcessThrottler::setShouldTakeNearSuspendedAssertion):
(WebKit::ProcessThrottler::isSuspended const):
(WebKit::ProcessThrottlerActivity::ProcessThrottlerActivity):
(WebKit::ProcessThrottlerActivity::invalidate):
* Source/WebKit/UIProcess/ProcessThrottler.h:
(WebKit::ProcessThrottler::isSuspended const): Deleted.
(WebKit::ProcessThrottlerActivity::ProcessThrottlerActivity): Deleted.
(WebKit::ProcessThrottlerActivity::invalidate): Deleted.
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::updateAudibleMediaAssertions):
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::updateAudibleMediaAssertions):

Canonical link: https://commits.webkit.org/270212@main
  • Loading branch information
pvollan committed Nov 3, 2023
1 parent b2ec6cb commit 5d6df46
Show file tree
Hide file tree
Showing 14 changed files with 105 additions and 133 deletions.
2 changes: 2 additions & 0 deletions Source/WebKit/Platform/cocoa/XPCUtilities.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ void terminateWithReason(xpc_connection_t connection, ReasonCode, const char*)
{
// This could use ReasonSPI.h, but currently does not as the SPI is blocked by the sandbox.
// See https://bugs.webkit.org/show_bug.cgi?id=224499 rdar://76396241
if (!connection)
return;
ALLOW_DEPRECATED_DECLARATIONS_BEGIN
xpc_connection_kill(connection, SIGKILL);
ALLOW_DEPRECATED_DECLARATIONS_END
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ void AuxiliaryProcessProxy::didFinishLaunching(ProcessLauncher*, IPC::Connection

#if PLATFORM(MAC) && USE(RUNNINGBOARD)
m_lifetimeActivity = throttler().foregroundActivity("Lifetime Activity"_s).moveToUniquePtr();
m_boostedJetsamAssertion = ProcessAssertion::create(xpc_connection_get_pid(connectionIdentifier.xpcConnection.get()), "Jetsam Boost"_s, ProcessAssertionType::BoostedJetsam);
m_boostedJetsamAssertion = ProcessAssertion::create(*this, "Jetsam Boost"_s, ProcessAssertionType::BoostedJetsam);
#endif

RefPtr connection = IPC::Connection::createServerConnection(connectionIdentifier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
// Deploy a watchdog in the UI process, since the child process may be suspended.
// If 30s is insufficient for any outstanding activity to complete cleanly, then it will be killed.
ASSERT(m_connection && m_connection->xpcConnection());
XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(m_connection->xpcConnection(), 30_s);
XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(*this, 30_s);
#endif
}

Expand Down
29 changes: 17 additions & 12 deletions Source/WebKit/UIProcess/Cocoa/ProcessAssertionCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -364,31 +364,36 @@ - (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)er
, m_pid(pid)
, m_reason(reason)
{
NSString *runningBoardAssertionName = runningBoardNameForAssertionType(assertionType);
init(environmentIdentifier);
}

void ProcessAssertion::init(const String& environmentIdentifier)
{
NSString *runningBoardAssertionName = runningBoardNameForAssertionType(m_assertionType);
ASSERT(runningBoardAssertionName);
if (pid <= 0) {
RELEASE_LOG_ERROR(ProcessSuspension, "%p - ProcessAssertion: Failed to acquire RBS %{public}@ assertion '%{public}s' for process because PID %d is invalid", this, runningBoardAssertionName, reason.utf8().data(), pid);
if (m_pid <= 0) {
RELEASE_LOG_ERROR(ProcessSuspension, "%p - ProcessAssertion: Failed to acquire RBS %{public}@ assertion '%{public}s' for process because PID %d is invalid", this, runningBoardAssertionName, m_reason.utf8().data(), m_pid);
m_wasInvalidated = true;
return;
}

RBSTarget *target = nil;
if (environmentIdentifier.isEmpty())
target = [RBSTarget targetWithPid:pid];
target = [RBSTarget targetWithPid:m_pid];
else
target = [RBSTarget targetWithPid:pid environmentIdentifier:environmentIdentifier];
target = [RBSTarget targetWithPid:m_pid environmentIdentifier:environmentIdentifier];

RBSDomainAttribute *domainAttribute = [RBSDomainAttribute attributeWithDomain:runningBoardDomainForAssertionType(assertionType) name:runningBoardAssertionName];
m_rbsAssertion = adoptNS([[RBSAssertion alloc] initWithExplanation:reason target:target attributes:@[domainAttribute]]);
RBSDomainAttribute *domainAttribute = [RBSDomainAttribute attributeWithDomain:runningBoardDomainForAssertionType(m_assertionType) name:runningBoardAssertionName];
m_rbsAssertion = adoptNS([[RBSAssertion alloc] initWithExplanation:m_reason target:target attributes:@[domainAttribute]]);

m_delegate = adoptNS([[WKRBSAssertionDelegate alloc] init]);
[m_rbsAssertion addObserver:m_delegate.get()];
m_delegate.get().invalidationCallback = ^{
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion: RBS %{public}@ assertion for process with PID=%d was invalidated", this, runningBoardAssertionName, pid);
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion: RBS %{public}@ assertion for process with PID=%d was invalidated", this, runningBoardAssertionName, m_pid);
processAssertionWasInvalidated();
};
m_delegate.get().prepareForInvalidationCallback = ^{
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() RBS %{public}@ assertion for process with PID=%d will be invalidated", this, runningBoardAssertionName, pid);
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() RBS %{public}@ assertion for process with PID=%d will be invalidated", this, runningBoardAssertionName, m_pid);
processAssertionWillBeInvalidated();
};
}
Expand All @@ -408,7 +413,7 @@ - (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)er
if (!m_grant)
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() Failed to grant capability with error %@", this, error);
#else
ProcessAssertion(m_pid, reason, assertionType, process.environmentIdentifier());
init(process.environmentIdentifier());
#endif
}

Expand Down Expand Up @@ -497,8 +502,8 @@ - (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)er
return !m_wasInvalidated;
}

ProcessAndUIAssertion::ProcessAndUIAssertion(pid_t pid, const String& reason, ProcessAssertionType assertionType, const String& environmentIdentifier)
: ProcessAssertion(pid, reason, assertionType, environmentIdentifier)
ProcessAndUIAssertion::ProcessAndUIAssertion(AuxiliaryProcessProxy& process, const String& reason, ProcessAssertionType assertionType)
: ProcessAssertion(process, reason, assertionType)
{
#if PLATFORM(IOS_FAMILY)
updateRunInBackgroundCount();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

namespace WebKit {

class AuxiliaryProcessProxy;
class ProcessAndUIAssertion;

// ConnectionTerminationWatchdog does two things:
Expand All @@ -44,13 +45,13 @@ class ProcessAndUIAssertion;
// to ensure it has a chance to terminate cleanly.
class XPCConnectionTerminationWatchdog {
public:
static void startConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t>, Seconds interval);
static void startConnectionTerminationWatchdog(AuxiliaryProcessProxy&, Seconds interval);

private:
XPCConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t>&&, Seconds interval);
XPCConnectionTerminationWatchdog(AuxiliaryProcessProxy&, Seconds interval);
void watchdogTimerFired();

OSObjectPtr<xpc_connection_t> m_xpcConnection;
WeakPtr<AuxiliaryProcessProxy> m_process;
RunLoop::Timer m_watchdogTimer;
Ref<ProcessAndUIAssertion> m_assertion;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,29 @@
#import "config.h"
#import "XPCConnectionTerminationWatchdog.h"

#import "AuxiliaryProcessProxy.h"
#import "ProcessAssertion.h"
#import "XPCUtilities.h"

namespace WebKit {

void XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t> xpcConnection, Seconds interval)
void XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(AuxiliaryProcessProxy& process, Seconds interval)
{
new XPCConnectionTerminationWatchdog(WTFMove(xpcConnection), interval);
new XPCConnectionTerminationWatchdog(process, interval);
}

XPCConnectionTerminationWatchdog::XPCConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t>&& xpcConnection, Seconds interval)
: m_xpcConnection(WTFMove(xpcConnection))
XPCConnectionTerminationWatchdog::XPCConnectionTerminationWatchdog(AuxiliaryProcessProxy& process, Seconds interval)
: m_process(process)
, m_watchdogTimer(RunLoop::main(), this, &XPCConnectionTerminationWatchdog::watchdogTimerFired)
, m_assertion(ProcessAndUIAssertion::create(xpc_connection_get_pid(m_xpcConnection.get()), "XPCConnectionTerminationWatchdog"_s, ProcessAssertionType::Background))
, m_assertion(ProcessAndUIAssertion::create(process, "XPCConnectionTerminationWatchdog"_s, ProcessAssertionType::Background))
{
m_watchdogTimer.startOneShot(interval);
}

void XPCConnectionTerminationWatchdog::watchdogTimerFired()
{
terminateWithReason(m_xpcConnection.get(), ReasonCode::WatchdogTimerFired, "XPCConnectionTerminationWatchdog::watchdogTimerFired");
if (m_process && m_process->connection())
terminateWithReason(m_process->connection()->xpcConnection(), ReasonCode::WatchdogTimerFired, "XPCConnectionTerminationWatchdog::watchdogTimerFired");
delete this;
}

Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,12 +542,7 @@ void GPUProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Connect
}

#if USE(RUNNINGBOARD)
#if USE(EXTENSIONKIT_ASSERTIONS)
m_throttler.didConnectToProcess(extensionProcess());
#else
if (xpc_connection_t connection = this->connection()->xpcConnection())
m_throttler.didConnectToProcess(xpc_connection_get_pid(connection));
#endif
m_throttler.didConnectToProcess(*this);
#endif

#if PLATFORM(COCOA)
Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -563,12 +563,7 @@ void NetworkProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Con
}

#if USE(RUNNINGBOARD)
#if USE(EXTENSIONKIT_ASSERTIONS)
m_throttler.didConnectToProcess(extensionProcess());
#else
if (xpc_connection_t connection = this->connection()->xpcConnection())
m_throttler.didConnectToProcess(xpc_connection_get_pid(connection));
#endif
m_throttler.didConnectToProcess(*this);
#endif
}

Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/ProcessAssertion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ void ProcessAssertion::acquireSync()
{
}

ProcessAndUIAssertion::ProcessAndUIAssertion(ProcessID pid, const String& reason, ProcessAssertionType assertionType, const String& environmentIdentifier)
: ProcessAssertion(pid, reason, assertionType, environmentIdentifier)
ProcessAndUIAssertion::ProcessAndUIAssertion(AuxiliaryProcessProxy& process, const String& reason, ProcessAssertionType assertionType)
: ProcessAssertion(process, reason, assertionType)
{
}

Expand Down
21 changes: 8 additions & 13 deletions Source/WebKit/UIProcess/ProcessAssertion.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ OBJC_CLASS WKRBSAssertionDelegate;

#if USE(EXTENSIONKIT_ASSERTIONS)
OBJC_CLASS _SEExtensionProcess;
OBJC_CLASS _SEGrant;
OBJC_PROTOCOL(_SEGrant);
#endif

namespace WebKit {
Expand All @@ -69,9 +69,8 @@ class ProcessAssertion : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<
enum class Mode : bool { Sync, Async };
#if USE(EXTENSIONKIT_ASSERTIONS)
static Ref<ProcessAssertion> create(RetainPtr<_SEExtensionProcess>, const String& reason, ProcessAssertionType, Mode = Mode::Async, const String& environmentIdentifier = emptyString(), CompletionHandler<void()>&& acquisisionHandler = nullptr);
#else
static Ref<ProcessAssertion> create(ProcessID, const String& reason, ProcessAssertionType, Mode = Mode::Async, const String& environmentIdentifier = emptyString(), CompletionHandler<void()>&& acquisisionHandler = nullptr);
#endif
static Ref<ProcessAssertion> create(ProcessID, const String& reason, ProcessAssertionType, Mode = Mode::Async, const String& environmentIdentifier = emptyString(), CompletionHandler<void()>&& acquisisionHandler = nullptr);
static Ref<ProcessAssertion> create(AuxiliaryProcessProxy&, const String& reason, ProcessAssertionType, Mode = Mode::Async, CompletionHandler<void()>&& acquisisionHandler = nullptr);

static double remainingRunTimeInSeconds(ProcessID);
Expand All @@ -89,6 +88,8 @@ class ProcessAssertion : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<
ProcessAssertion(ProcessID, const String& reason, ProcessAssertionType, const String& environmentIdentifier);
ProcessAssertion(AuxiliaryProcessProxy&, const String& reason, ProcessAssertionType);

void init(const String& environmentIdentifier);

void aquireAssertion(Mode, CompletionHandler<void()>&&);

void acquireAsync(CompletionHandler<void()>&&);
Expand Down Expand Up @@ -117,16 +118,10 @@ class ProcessAssertion : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<

class ProcessAndUIAssertion final : public ProcessAssertion {
public:
static Ref<ProcessAndUIAssertion> create(ProcessID pid, const String& reason, ProcessAssertionType type, Mode mode = Mode::Async, const String& environmentIdentifier = emptyString(), CompletionHandler<void()>&& acquisisionHandler = nullptr)
static Ref<ProcessAndUIAssertion> create(AuxiliaryProcessProxy& process, const String& reason, ProcessAssertionType type, Mode mode = Mode::Async, CompletionHandler<void()>&& acquisisionHandler = nullptr)
{
auto assertion = adoptRef(*new ProcessAndUIAssertion(pid, reason, type, environmentIdentifier));
if (mode == Mode::Async)
assertion->acquireAsync(WTFMove(acquisisionHandler));
else {
assertion->acquireSync();
if (acquisisionHandler)
acquisisionHandler();
}
auto assertion = adoptRef(*new ProcessAndUIAssertion(process, reason, type));
assertion->aquireAssertion(mode, WTFMove(acquisisionHandler));
return assertion;
}
~ProcessAndUIAssertion();
Expand All @@ -139,7 +134,7 @@ class ProcessAndUIAssertion final : public ProcessAssertion {
#endif

private:
ProcessAndUIAssertion(ProcessID, const String& reason, ProcessAssertionType, const String& environmentIdentifier);
ProcessAndUIAssertion(AuxiliaryProcessProxy&, const String& reason, ProcessAssertionType);

#if PLATFORM(IOS_FAMILY)
void processAssertionWasInvalidated() final;
Expand Down
Loading

0 comments on commit 5d6df46

Please sign in to comment.