- Revision
- 260373
- Author
- [email protected]
- Date
- 2020-04-20 10:48:42 -0700 (Mon, 20 Apr 2020)
Log Message
[iOS] Refactor WebKit media playback process assertion logic to minimize chances of leaking them
https://bugs.webkit.org/show_bug.cgi?id=210670
Reviewed by Geoffrey Garen.
Refactor WebKit media playback process assertion logic to minimize chances of leaking them. In particular,
the following changes were made:
1. Instead of the WebProcessPool having a HashMap of media playback process assertions for
the WebProcess, we now store the assertion on the WebProcessProxy object itself. This is
less likely to get out of sync and leak.
2. Add a RefCounter to the WebProcessPool to count WebProcesses that have audible media.
Whenever a WebProcess starts or stops playing audible media, it merely grabs a RefCounter
token or releases it. The WebProcessPool relies on this counter to decide whether or not
to take a media playback assertion on behalf of the UIProcess. Since this is token-based
and the token is stored on the WebProcessProxy object, it makes it less likely to leak
the assertion.
3. The WebProcessProxy object now has a AudibleMediaActivity data structure wrapping both
its media playback assertion and its WebProcessWithAudibleMediaToken that it got from
the WebProcessPool. When the WebProcess shuts down (normally or due to crash/termination),
we make sure to clear this data structure.
4. Make sure that the WebProcessProxy updates its AudibleMediaActivity whenever a page is
removed from the WebProcess.
* UIProcess/WebPageProxy.cpp:
* UIProcess/WebProcessPool.cpp:
(WebKit::m_webProcessWithAudibleMediaCounter):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::webProcessWithAudibleMediaToken const):
(WebKit::WebProcessPool::updateAudibleMediaAssertions):
* UIProcess/WebProcessPool.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::updateAudibleMediaAssertions):
* UIProcess/WebProcessProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (260372 => 260373)
--- trunk/Source/WebKit/ChangeLog 2020-04-20 17:15:28 UTC (rev 260372)
+++ trunk/Source/WebKit/ChangeLog 2020-04-20 17:48:42 UTC (rev 260373)
@@ -1,3 +1,41 @@
+2020-04-20 Chris Dumez <[email protected]>
+
+ [iOS] Refactor WebKit media playback process assertion logic to minimize chances of leaking them
+ https://bugs.webkit.org/show_bug.cgi?id=210670
+
+ Reviewed by Geoffrey Garen.
+
+ Refactor WebKit media playback process assertion logic to minimize chances of leaking them. In particular,
+ the following changes were made:
+ 1. Instead of the WebProcessPool having a HashMap of media playback process assertions for
+ the WebProcess, we now store the assertion on the WebProcessProxy object itself. This is
+ less likely to get out of sync and leak.
+ 2. Add a RefCounter to the WebProcessPool to count WebProcesses that have audible media.
+ Whenever a WebProcess starts or stops playing audible media, it merely grabs a RefCounter
+ token or releases it. The WebProcessPool relies on this counter to decide whether or not
+ to take a media playback assertion on behalf of the UIProcess. Since this is token-based
+ and the token is stored on the WebProcessProxy object, it makes it less likely to leak
+ the assertion.
+ 3. The WebProcessProxy object now has a AudibleMediaActivity data structure wrapping both
+ its media playback assertion and its WebProcessWithAudibleMediaToken that it got from
+ the WebProcessPool. When the WebProcess shuts down (normally or due to crash/termination),
+ we make sure to clear this data structure.
+ 4. Make sure that the WebProcessProxy updates its AudibleMediaActivity whenever a page is
+ removed from the WebProcess.
+
+ * UIProcess/WebPageProxy.cpp:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::m_webProcessWithAudibleMediaCounter):
+ (WebKit::WebProcessPool::disconnectProcess):
+ (WebKit::WebProcessPool::webProcessWithAudibleMediaToken const):
+ (WebKit::WebProcessPool::updateAudibleMediaAssertions):
+ * UIProcess/WebProcessPool.h:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::shutDown):
+ (WebKit::WebProcessProxy::removeWebPage):
+ (WebKit::WebProcessProxy::updateAudibleMediaAssertions):
+ * UIProcess/WebProcessProxy.h:
+
2020-04-20 Kate Cheney <[email protected]>
<rdar://problem/62059046>
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (260372 => 260373)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-04-20 17:15:28 UTC (rev 260372)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-04-20 17:48:42 UTC (rev 260373)
@@ -8929,7 +8929,7 @@
if ((oldState & MediaProducer::HasAudioOrVideo) != (m_mediaState & MediaProducer::HasAudioOrVideo))
videoControlsManagerDidChange();
- m_process->webPageMediaStateDidChange(*this);
+ m_process->updateAudibleMediaAssertions();
}
void WebPageProxy::videoControlsManagerDidChange()
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (260372 => 260373)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2020-04-20 17:15:28 UTC (rev 260372)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2020-04-20 17:48:42 UTC (rev 260373)
@@ -260,6 +260,7 @@
, m_backgroundWebProcessCounter([this](RefCounterEvent) { updateProcessAssertions(); })
, m_backForwardCache(makeUniqueRef<WebBackForwardCache>(*this))
, m_webProcessCache(makeUniqueRef<WebProcessCache>(*this))
+ , m_webProcessWithAudibleMediaCounter([this](RefCounterEvent) { updateAudibleMediaAssertions(); })
{
static std::once_flag onceFlag;
std::call_once(onceFlag, [] {
@@ -1182,7 +1183,6 @@
void WebProcessPool::disconnectProcess(WebProcessProxy* process)
{
ASSERT(m_processes.contains(process));
- ASSERT(!m_processesPlayingAudibleMedia.contains(process->coreProcessIdentifier()));
if (m_prewarmedProcess == process) {
ASSERT(m_prewarmedProcess->isPrewarmed());
@@ -2350,48 +2350,31 @@
}
#endif
-void WebProcessPool::setWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier processID)
+WebProcessWithAudibleMediaToken WebProcessPool::webProcessWithAudibleMediaToken() const
{
- auto* process = WebProcessProxy::processForIdentifier(processID);
- ASSERT(process);
+ return m_webProcessWithAudibleMediaCounter.count();
+}
- WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "setWebProcessIsPlayingAudibleMedia: Web process is now playing audible media (process=%p, PID=%i)", process, process->processIdentifier());
+void WebProcessPool::updateAudibleMediaAssertions()
+{
+ if (!m_webProcessWithAudibleMediaCounter.value()) {
+ WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "updateAudibleMediaAssertions: The number of processes playing audible media now zero. Releasing UI process assertion.");
+ m_audibleMediaActivity = WTF::nullopt;
+ return;
+ }
- if (m_processesPlayingAudibleMedia.isEmpty()) {
- WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "setWebProcessIsPlayingAudibleMedia: The number of processes playing audible media is now one. Taking UI process assertion.");
+ if (m_audibleMediaActivity)
+ return;
- ASSERT(!m_uiProcessMediaPlaybackAssertion);
- m_uiProcessMediaPlaybackAssertion = makeUnique<ProcessAssertion>(getCurrentProcessID(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback);
+ WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "updateAudibleMediaAssertions: The number of processes playing audible media is now greater than zero. Taking UI process assertion.");
+ m_audibleMediaActivity = AudibleMediaActivity {
+ makeUniqueRef<ProcessAssertion>(getCurrentProcessID(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback)
#if ENABLE(GPU_PROCESS)
- if (GPUProcessProxy::singletonIfCreated())
- m_gpuProcessMediaPlaybackAssertion = makeUnique<ProcessAssertion>(GPUProcessProxy::singleton().processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback);
+ , GPUProcessProxy::singletonIfCreated() ? makeUnique<ProcessAssertion>(GPUProcessProxy::singleton().processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback) : nullptr
#endif
- }
-
- auto result = m_processesPlayingAudibleMedia.add(processID, nullptr);
- ASSERT(result.isNewEntry);
- result.iterator->value = makeUnique<ProcessAssertion>(process->processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback);
+ };
}
-void WebProcessPool::clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier processID)
-{
- auto result = m_processesPlayingAudibleMedia.take(processID);
- ASSERT_UNUSED(result, result);
-
- auto* process = WebProcessProxy::processForIdentifier(processID);
- ASSERT_UNUSED(process, process);
-
- WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "clearWebProcessIsPlayingAudibleMedia: Web process is no longer playing audible media (process=%p, PID=%i)", process, process->processIdentifier());
-
- if (m_processesPlayingAudibleMedia.isEmpty()) {
- WEBPROCESSPOOL_RELEASE_LOG(ProcessSuspension, "clearWebProcessIsPlayingAudibleMedia: The number of processes playing audible media now zero. Releasing UI process assertion.");
-
- ASSERT(m_uiProcessMediaPlaybackAssertion);
- m_uiProcessMediaPlaybackAssertion = nullptr;
- m_gpuProcessMediaPlaybackAssertion = nullptr;
- }
-}
-
void WebProcessPool::setUseSeparateServiceWorkerProcess(bool useSeparateServiceWorkerProcess)
{
if (m_useSeparateServiceWorkerProcess == useSeparateServiceWorkerProcess)
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (260372 => 260373)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2020-04-20 17:15:28 UTC (rev 260372)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2020-04-20 17:48:42 UTC (rev 260373)
@@ -517,8 +517,7 @@
const Function<void(UserMessage&&, CompletionHandler<void(UserMessage&&)>&&)>& userMessageHandler() const { return m_userMessageHandler; }
#endif
- void setWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier);
- void clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier);
+ WebProcessWithAudibleMediaToken webProcessWithAudibleMediaToken() const;
void disableDelayedWebProcessLaunch() { m_isDelayedWebProcessLaunchDisabled = true; }
@@ -562,6 +561,7 @@
#endif
void updateProcessAssertions();
+ void updateAudibleMediaAssertions();
// IPC::MessageReceiver.
// Implemented in generated WebProcessPoolMessageReceiver.cpp
@@ -787,10 +787,16 @@
Function<void(UserMessage&&, CompletionHandler<void(UserMessage&&)>&&)> m_userMessageHandler;
#endif
- HashMap<WebCore::ProcessIdentifier, std::unique_ptr<ProcessAssertion>> m_processesPlayingAudibleMedia;
- std::unique_ptr<ProcessAssertion> m_uiProcessMediaPlaybackAssertion;
- std::unique_ptr<ProcessAssertion> m_gpuProcessMediaPlaybackAssertion;
+ WebProcessWithAudibleMediaCounter m_webProcessWithAudibleMediaCounter;
+ struct AudibleMediaActivity {
+ UniqueRef<ProcessAssertion> uiProcessMediaPlaybackAssertion;
+#if ENABLE(GPU_PROCESS)
+ std::unique_ptr<ProcessAssertion> gpuProcessMediaPlaybackAssertion;
+#endif
+ };
+ Optional<AudibleMediaActivity> m_audibleMediaActivity;
+
#if PLATFORM(IOS)
// FIXME: Delayed process launch is currently disabled on iOS for performance reasons (rdar://problem/49074131).
bool m_isDelayedWebProcessLaunchDisabled { true };
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (260372 => 260373)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2020-04-20 17:15:28 UTC (rev 260372)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2020-04-20 17:48:42 UTC (rev 260373)
@@ -414,6 +414,7 @@
m_responsivenessTimer.invalidate();
m_backgroundResponsivenessTimer.invalidate();
m_activityForHoldingLockedFiles = nullptr;
+ m_audibleMediaActivity = WTF::nullopt;
for (auto& frame : copyToVector(m_frameMap.values()))
frame->webProcessWillShutDown();
@@ -513,7 +514,7 @@
removeVisitedLinkStoreUser(webPage.visitedLinkStore(), webPage.identifier());
updateRegistrationWithDataStore();
-
+ updateAudibleMediaAssertions();
updateBackgroundResponsivenessTimer();
maybeShutDown();
@@ -1382,17 +1383,24 @@
ASSERT(!m_backgroundToken || !m_foregroundToken);
}
-void WebProcessProxy::webPageMediaStateDidChange(WebPageProxy&)
+void WebProcessProxy::updateAudibleMediaAssertions()
{
bool newHasAudibleWebPage = WTF::anyOf(m_pageMap.values(), [] (auto& page) { return page->isPlayingAudio(); });
- if (m_hasAudibleWebPage == newHasAudibleWebPage)
+
+ bool hasAudibleMediaActivity = !!m_audibleMediaActivity;
+ if (hasAudibleMediaActivity == newHasAudibleWebPage)
return;
- m_hasAudibleWebPage = newHasAudibleWebPage;
- if (m_hasAudibleWebPage)
- processPool().setWebProcessIsPlayingAudibleMedia(coreProcessIdentifier());
- else
- processPool().clearWebProcessIsPlayingAudibleMedia(coreProcessIdentifier());
+ if (newHasAudibleWebPage) {
+ RELEASE_LOG(ProcessSuspension, "%p - Taking MediaPlayback assertion for WebProcess with PID %d", this, processIdentifier());
+ m_audibleMediaActivity = AudibleMediaActivity {
+ makeUniqueRef<ProcessAssertion>(processIdentifier(), "WebKit Media Playback"_s, ProcessAssertionType::MediaPlayback),
+ processPool().webProcessWithAudibleMediaToken()
+ };
+ } else {
+ RELEASE_LOG(ProcessSuspension, "%p - Releasing MediaPlayback assertion for WebProcess with PID %d", this, processIdentifier());
+ m_audibleMediaActivity = WTF::nullopt;
+ }
}
void WebProcessProxy::setIsHoldingLockedFiles(bool isHoldingLockedFiles)
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (260372 => 260373)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2020-04-20 17:15:28 UTC (rev 260372)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2020-04-20 17:48:42 UTC (rev 260373)
@@ -106,6 +106,9 @@
enum BackgroundWebProcessCounterType { };
typedef RefCounter<BackgroundWebProcessCounterType> BackgroundWebProcessCounter;
typedef BackgroundWebProcessCounter::Token BackgroundWebProcessToken;
+enum WebProcessWithAudibleMediaCounterType { };
+using WebProcessWithAudibleMediaCounter = RefCounter<WebProcessWithAudibleMediaCounterType>;
+using WebProcessWithAudibleMediaToken = WebProcessWithAudibleMediaCounter::Token;
class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer::Client, public ThreadSafeRefCounted<WebProcessProxy>, public CanMakeWeakPtr<WebProcessProxy>, private ProcessThrottlerClient {
public:
@@ -345,7 +348,7 @@
#endif
#endif
- void webPageMediaStateDidChange(WebPageProxy&);
+ void updateAudibleMediaAssertions();
void ref() final { ThreadSafeRefCounted::ref(); }
void deref() final { ThreadSafeRefCounted::deref(); }
@@ -568,7 +571,6 @@
unsigned m_shutdownPreventingScopeCount { 0 };
bool m_hasCommittedAnyProvisionalLoads { false };
bool m_isPrewarmed;
- bool m_hasAudibleWebPage { false };
#if ENABLE(ATTACHMENT_ELEMENT) && PLATFORM(IOS_FAMILY)
bool m_hasIssuedAttachmentElementRelatedSandboxExtensions { false };
#endif
@@ -593,6 +595,12 @@
Optional<ServiceWorkerInformation> m_serviceWorkerInformation;
HashMap<WebCore::SleepDisablerIdentifier, std::unique_ptr<WebCore::SleepDisabler>> m_sleepDisablers;
+
+ struct AudibleMediaActivity {
+ UniqueRef<ProcessAssertion> assertion;
+ WebProcessWithAudibleMediaToken token;
+ };
+ Optional<AudibleMediaActivity> m_audibleMediaActivity;
};
} // namespace WebKit
