Skip to content

Commit 5d6df46

Browse files
pvollanpvollan
committed
Minor refactoring of assertion code, v2
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
1 parent b2ec6cb commit 5d6df46

14 files changed

Lines changed: 105 additions & 133 deletions

‎Source/WebKit/Platform/cocoa/XPCUtilities.mm‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ void terminateWithReason(xpc_connection_t connection, ReasonCode, const char*)
3232
{
3333
// This could use ReasonSPI.h, but currently does not as the SPI is blocked by the sandbox.
3434
// See https://bugs.webkit.org/show_bug.cgi?id=224499 rdar://76396241
35+
if (!connection)
36+
return;
3537
ALLOW_DEPRECATED_DECLARATIONS_BEGIN
3638
xpc_connection_kill(connection, SIGKILL);
3739
ALLOW_DEPRECATED_DECLARATIONS_END

‎Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ void AuxiliaryProcessProxy::didFinishLaunching(ProcessLauncher*, IPC::Connection
320320

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

326326
RefPtr connection = IPC::Connection::createServerConnection(connectionIdentifier);

‎Source/WebKit/UIProcess/Cocoa/AuxiliaryProcessProxyCocoa.mm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
// Deploy a watchdog in the UI process, since the child process may be suspended.
7171
// If 30s is insufficient for any outstanding activity to complete cleanly, then it will be killed.
7272
ASSERT(m_connection && m_connection->xpcConnection());
73-
XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(m_connection->xpcConnection(), 30_s);
73+
XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(*this, 30_s);
7474
#endif
7575
}
7676

‎Source/WebKit/UIProcess/Cocoa/ProcessAssertionCocoa.mm‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -364,31 +364,36 @@ - (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)er
364364
, m_pid(pid)
365365
, m_reason(reason)
366366
{
367-
NSString *runningBoardAssertionName = runningBoardNameForAssertionType(assertionType);
367+
init(environmentIdentifier);
368+
}
369+
370+
void ProcessAssertion::init(const String& environmentIdentifier)
371+
{
372+
NSString *runningBoardAssertionName = runningBoardNameForAssertionType(m_assertionType);
368373
ASSERT(runningBoardAssertionName);
369-
if (pid <= 0) {
370-
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);
374+
if (m_pid <= 0) {
375+
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);
371376
m_wasInvalidated = true;
372377
return;
373378
}
374379

375380
RBSTarget *target = nil;
376381
if (environmentIdentifier.isEmpty())
377-
target = [RBSTarget targetWithPid:pid];
382+
target = [RBSTarget targetWithPid:m_pid];
378383
else
379-
target = [RBSTarget targetWithPid:pid environmentIdentifier:environmentIdentifier];
384+
target = [RBSTarget targetWithPid:m_pid environmentIdentifier:environmentIdentifier];
380385

381-
RBSDomainAttribute *domainAttribute = [RBSDomainAttribute attributeWithDomain:runningBoardDomainForAssertionType(assertionType) name:runningBoardAssertionName];
382-
m_rbsAssertion = adoptNS([[RBSAssertion alloc] initWithExplanation:reason target:target attributes:@[domainAttribute]]);
386+
RBSDomainAttribute *domainAttribute = [RBSDomainAttribute attributeWithDomain:runningBoardDomainForAssertionType(m_assertionType) name:runningBoardAssertionName];
387+
m_rbsAssertion = adoptNS([[RBSAssertion alloc] initWithExplanation:m_reason target:target attributes:@[domainAttribute]]);
383388

384389
m_delegate = adoptNS([[WKRBSAssertionDelegate alloc] init]);
385390
[m_rbsAssertion addObserver:m_delegate.get()];
386391
m_delegate.get().invalidationCallback = ^{
387-
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion: RBS %{public}@ assertion for process with PID=%d was invalidated", this, runningBoardAssertionName, pid);
392+
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion: RBS %{public}@ assertion for process with PID=%d was invalidated", this, runningBoardAssertionName, m_pid);
388393
processAssertionWasInvalidated();
389394
};
390395
m_delegate.get().prepareForInvalidationCallback = ^{
391-
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() RBS %{public}@ assertion for process with PID=%d will be invalidated", this, runningBoardAssertionName, pid);
396+
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() RBS %{public}@ assertion for process with PID=%d will be invalidated", this, runningBoardAssertionName, m_pid);
392397
processAssertionWillBeInvalidated();
393398
};
394399
}
@@ -408,7 +413,7 @@ - (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)er
408413
if (!m_grant)
409414
RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion() Failed to grant capability with error %@", this, error);
410415
#else
411-
ProcessAssertion(m_pid, reason, assertionType, process.environmentIdentifier());
416+
init(process.environmentIdentifier());
412417
#endif
413418
}
414419

@@ -497,8 +502,8 @@ - (void)assertion:(RBSAssertion *)assertion didInvalidateWithError:(NSError *)er
497502
return !m_wasInvalidated;
498503
}
499504

500-
ProcessAndUIAssertion::ProcessAndUIAssertion(pid_t pid, const String& reason, ProcessAssertionType assertionType, const String& environmentIdentifier)
501-
: ProcessAssertion(pid, reason, assertionType, environmentIdentifier)
505+
ProcessAndUIAssertion::ProcessAndUIAssertion(AuxiliaryProcessProxy& process, const String& reason, ProcessAssertionType assertionType)
506+
: ProcessAssertion(process, reason, assertionType)
502507
{
503508
#if PLATFORM(IOS_FAMILY)
504509
updateRunInBackgroundCount();

‎Source/WebKit/UIProcess/Cocoa/XPCConnectionTerminationWatchdog.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
namespace WebKit {
3838

39+
class AuxiliaryProcessProxy;
3940
class ProcessAndUIAssertion;
4041

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

4950
private:
50-
XPCConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t>&&, Seconds interval);
51+
XPCConnectionTerminationWatchdog(AuxiliaryProcessProxy&, Seconds interval);
5152
void watchdogTimerFired();
5253

53-
OSObjectPtr<xpc_connection_t> m_xpcConnection;
54+
WeakPtr<AuxiliaryProcessProxy> m_process;
5455
RunLoop::Timer m_watchdogTimer;
5556
Ref<ProcessAndUIAssertion> m_assertion;
5657
};

‎Source/WebKit/UIProcess/Cocoa/XPCConnectionTerminationWatchdog.mm‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,29 @@
2626
#import "config.h"
2727
#import "XPCConnectionTerminationWatchdog.h"
2828

29+
#import "AuxiliaryProcessProxy.h"
2930
#import "ProcessAssertion.h"
3031
#import "XPCUtilities.h"
3132

3233
namespace WebKit {
3334

34-
void XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t> xpcConnection, Seconds interval)
35+
void XPCConnectionTerminationWatchdog::startConnectionTerminationWatchdog(AuxiliaryProcessProxy& process, Seconds interval)
3536
{
36-
new XPCConnectionTerminationWatchdog(WTFMove(xpcConnection), interval);
37+
new XPCConnectionTerminationWatchdog(process, interval);
3738
}
3839

39-
XPCConnectionTerminationWatchdog::XPCConnectionTerminationWatchdog(OSObjectPtr<xpc_connection_t>&& xpcConnection, Seconds interval)
40-
: m_xpcConnection(WTFMove(xpcConnection))
40+
XPCConnectionTerminationWatchdog::XPCConnectionTerminationWatchdog(AuxiliaryProcessProxy& process, Seconds interval)
41+
: m_process(process)
4142
, m_watchdogTimer(RunLoop::main(), this, &XPCConnectionTerminationWatchdog::watchdogTimerFired)
42-
, m_assertion(ProcessAndUIAssertion::create(xpc_connection_get_pid(m_xpcConnection.get()), "XPCConnectionTerminationWatchdog"_s, ProcessAssertionType::Background))
43+
, m_assertion(ProcessAndUIAssertion::create(process, "XPCConnectionTerminationWatchdog"_s, ProcessAssertionType::Background))
4344
{
4445
m_watchdogTimer.startOneShot(interval);
4546
}
4647

4748
void XPCConnectionTerminationWatchdog::watchdogTimerFired()
4849
{
49-
terminateWithReason(m_xpcConnection.get(), ReasonCode::WatchdogTimerFired, "XPCConnectionTerminationWatchdog::watchdogTimerFired");
50+
if (m_process && m_process->connection())
51+
terminateWithReason(m_process->connection()->xpcConnection(), ReasonCode::WatchdogTimerFired, "XPCConnectionTerminationWatchdog::watchdogTimerFired");
5052
delete this;
5153
}
5254

‎Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -542,12 +542,7 @@ void GPUProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Connect
542542
}
543543

544544
#if USE(RUNNINGBOARD)
545-
#if USE(EXTENSIONKIT_ASSERTIONS)
546-
m_throttler.didConnectToProcess(extensionProcess());
547-
#else
548-
if (xpc_connection_t connection = this->connection()->xpcConnection())
549-
m_throttler.didConnectToProcess(xpc_connection_get_pid(connection));
550-
#endif
545+
m_throttler.didConnectToProcess(*this);
551546
#endif
552547

553548
#if PLATFORM(COCOA)

‎Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -563,12 +563,7 @@ void NetworkProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Con
563563
}
564564

565565
#if USE(RUNNINGBOARD)
566-
#if USE(EXTENSIONKIT_ASSERTIONS)
567-
m_throttler.didConnectToProcess(extensionProcess());
568-
#else
569-
if (xpc_connection_t connection = this->connection()->xpcConnection())
570-
m_throttler.didConnectToProcess(xpc_connection_get_pid(connection));
571-
#endif
566+
m_throttler.didConnectToProcess(*this);
572567
#endif
573568
}
574569

‎Source/WebKit/UIProcess/ProcessAssertion.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ void ProcessAssertion::acquireSync()
116116
{
117117
}
118118

119-
ProcessAndUIAssertion::ProcessAndUIAssertion(ProcessID pid, const String& reason, ProcessAssertionType assertionType, const String& environmentIdentifier)
120-
: ProcessAssertion(pid, reason, assertionType, environmentIdentifier)
119+
ProcessAndUIAssertion::ProcessAndUIAssertion(AuxiliaryProcessProxy& process, const String& reason, ProcessAssertionType assertionType)
120+
: ProcessAssertion(process, reason, assertionType)
121121
{
122122
}
123123

‎Source/WebKit/UIProcess/ProcessAssertion.h‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ OBJC_CLASS WKRBSAssertionDelegate;
4444

4545
#if USE(EXTENSIONKIT_ASSERTIONS)
4646
OBJC_CLASS _SEExtensionProcess;
47-
OBJC_CLASS _SEGrant;
47+
OBJC_PROTOCOL(_SEGrant);
4848
#endif
4949

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

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

91+
void init(const String& environmentIdentifier);
92+
9293
void aquireAssertion(Mode, CompletionHandler<void()>&&);
9394

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

118119
class ProcessAndUIAssertion final : public ProcessAssertion {
119120
public:
120-
static Ref<ProcessAndUIAssertion> create(ProcessID pid, const String& reason, ProcessAssertionType type, Mode mode = Mode::Async, const String& environmentIdentifier = emptyString(), CompletionHandler<void()>&& acquisisionHandler = nullptr)
121+
static Ref<ProcessAndUIAssertion> create(AuxiliaryProcessProxy& process, const String& reason, ProcessAssertionType type, Mode mode = Mode::Async, CompletionHandler<void()>&& acquisisionHandler = nullptr)
121122
{
122-
auto assertion = adoptRef(*new ProcessAndUIAssertion(pid, reason, type, environmentIdentifier));
123-
if (mode == Mode::Async)
124-
assertion->acquireAsync(WTFMove(acquisisionHandler));
125-
else {
126-
assertion->acquireSync();
127-
if (acquisisionHandler)
128-
acquisisionHandler();
129-
}
123+
auto assertion = adoptRef(*new ProcessAndUIAssertion(process, reason, type));
124+
assertion->aquireAssertion(mode, WTFMove(acquisisionHandler));
130125
return assertion;
131126
}
132127
~ProcessAndUIAssertion();
@@ -139,7 +134,7 @@ class ProcessAndUIAssertion final : public ProcessAssertion {
139134
#endif
140135

141136
private:
142-
ProcessAndUIAssertion(ProcessID, const String& reason, ProcessAssertionType, const String& environmentIdentifier);
137+
ProcessAndUIAssertion(AuxiliaryProcessProxy&, const String& reason, ProcessAssertionType);
143138

144139
#if PLATFORM(IOS_FAMILY)
145140
void processAssertionWasInvalidated() final;

0 commit comments

Comments
 (0)