Attachment 369776 Details for Bug 197798 – Patch for landing

[patch] Patch for landing

bug-197798-20190513133345.patch (text/plain), 10.77 KB, created by Jer Noble on 2019-05-13 13:33:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jer Noble

Created: 2019-05-13 13:33:46 PDT

Size: 10.77 KB

patch

obsolete

>Subversion Revision: 245158
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 0b1a48cbc7efca67cc27a40e1109535092a1ea95..6fd50e5aba7372c5db766153f24d792444b46382 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-05-10  Jer Noble  <jer.noble@apple.com>
>+
>+        Take out MediaPlayback UI assertion when any WebProcess is playing audible media
>+        https://bugs.webkit.org/show_bug.cgi?id=197798
>+
>+        Reviewed by Chris Dumez.
>+
>+        To keep the system from suspending the UIProcess (and all the other constellation of processes that
>+        are necessary to play media), take a UIProcess assertion with the MediaPlayback reason whenever there
>+        is a WebContent process that is playing audible media.
>+
>+        * Platform/spi/ios/AssertionServicesSPI.h:
>+        * UIProcess/ProcessAssertion.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::updatePlayingMediaDidChange):
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::setWebProcessIsPlayingAudibleMedia):
>+        (WebKit::WebProcessPool::clearWebProcessIsPlayingAudibleMedia):
>+        * UIProcess/WebProcessPool.h:
>+        * UIProcess/WebProcessProxy.cpp:
>+        (WebKit::WebProcessProxy::webPageMediaStateDidChange):
>+        * UIProcess/WebProcessProxy.h:
>+        * UIProcess/ios/ProcessAssertionIOS.mm:
>+        (WebKit::toBKSProcessAssertionReason):
>+        (WebKit::ProcessAssertion::ProcessAssertion):
>+        * WebProcess/WebProcess.h:
>+
> 2019-05-09  Daniel Bates  <dabates@apple.com>
> 
>         [iOS] Lazily request keyboard on first hardware keydown when a non-editable element is focused
>diff --git a/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h b/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h
>index 6ce6f2d95ce36c799500e9740835033659d74d17..a67f41ff95751bbf98341e14449969ff320fb871 100644
>--- a/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h
>+++ b/Source/WebKit/Platform/spi/ios/AssertionServicesSPI.h
>@@ -69,6 +69,7 @@ enum {
> typedef uint32_t BKSProcessAssertionFlags;
> 
> enum {
>+    BKSProcessAssertionReasonMediaPlayback = 1,
>     BKSProcessAssertionReasonFinishTask = 4,
>     BKSProcessAssertionReasonExtension = 13,
>     BKSProcessAssertionReasonFinishTaskUnbounded = 10004,
>diff --git a/Source/WebKit/UIProcess/ProcessAssertion.h b/Source/WebKit/UIProcess/ProcessAssertion.h
>index a01c76db733b9eee6d2be97e762c80e94b583011..771076ac675b713f80a3365832abba02c7f8adab 100644
>--- a/Source/WebKit/UIProcess/ProcessAssertion.h
>+++ b/Source/WebKit/UIProcess/ProcessAssertion.h
>@@ -52,6 +52,7 @@ enum class AssertionReason {
>     Extension,
>     FinishTask,
>     FinishTaskUnbounded,
>+    MediaPlayback,
> };
> 
> class ProcessAssertion : public CanMakeWeakPtr<ProcessAssertion> {
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index a8ab15e7550c4eb095b254b16094edd45f32d570..291e288a7f79a98d348000cda208ff74b350a206 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -1038,6 +1038,7 @@ void WebPageProxy::close()
> #endif
> 
>     stopAllURLSchemeTasks();
>+    updatePlayingMediaDidChange(MediaProducer::IsNotPlaying);
> }
> 
> bool WebPageProxy::tryClose()
>@@ -7053,6 +7054,8 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
>         m_pageLoadState.reset(transaction);
>     }
> 
>+    updatePlayingMediaDidChange(MediaProducer::IsNotPlaying);
>+
>     // FIXME: <rdar://problem/38676604> In case of process swaps, the old process should gracefully suspend instead of terminating.
>     m_process->processTerminated();
> }
>@@ -8073,7 +8076,8 @@ void WebPageProxy::isPlayingMediaDidChange(MediaProducer::MediaStateFlags newSta
>     ASSERT(focusManager);
>     focusManager->updatePlaybackAttributesFromMediaState(this, sourceElementID, newState);
> #endif
>-    updatePlayingMediaDidChange(newState);
>+    if (!m_isClosed)
>+        updatePlayingMediaDidChange(newState);
> }
> 
> void WebPageProxy::updatePlayingMediaDidChange(MediaProducer::MediaStateFlags newState)
>@@ -8111,6 +8115,8 @@ void WebPageProxy::updatePlayingMediaDidChange(MediaProducer::MediaStateFlags ne
> 
>     if ((oldState & MediaProducer::HasAudioOrVideo) != (m_mediaState & MediaProducer::HasAudioOrVideo))
>         videoControlsManagerDidChange();
>+
>+    m_process->webPageMediaStateDidChange(*this);
> }
> 
> void WebPageProxy::videoControlsManagerDidChange()
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 9376da5dab5bd033c2fdee8b20db794bda2bc3f2..9803be6c811e2cc661de6db20ef016ebbc77c811 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -1100,6 +1100,7 @@ void WebProcessPool::processDidFinishLaunching(WebProcessProxy* process)
> void WebProcessPool::disconnectProcess(WebProcessProxy* process)
> {
>     ASSERT(m_processes.contains(process));
>+    ASSERT(!m_processesPlayingAudibleMedia.contains(process.coreProcessIdentifier()));
> 
>     if (m_prewarmedProcess == process) {
>         ASSERT(m_prewarmedProcess->isPrewarmed());
>@@ -2570,4 +2571,40 @@ void WebProcessPool::clearWebProcessHasUploads(ProcessIdentifier processID)
>     
> }
> 
>+void WebProcessPool::setWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier processID)
>+{
>+    auto* process = WebProcessProxy::processForIdentifier(processID);
>+    ASSERT(process);
>+
>+    RELEASE_LOG(ProcessSuspension, "Web process pid %u is now playing audible media", (unsigned)process->processIdentifier());
>+
>+    if (m_processesPlayingAudibleMedia.isEmpty()) {
>+        RELEASE_LOG(ProcessSuspension, "The number of processes playing audible media is now one. Taking UI process assertion.");
>+
>+        ASSERT(!m_uiProcessMediaPlaybackAssertion);
>+        m_uiProcessMediaPlaybackAssertion = std::make_unique<ProcessAssertion>(getCurrentProcessID(), "WebKit Media Playback"_s, AssertionState::Foreground, AssertionReason::MediaPlayback);
>+    }
>+
>+    auto result = m_processesPlayingAudibleMedia.add(processID, nullptr);
>+    ASSERT(result.isNewEntry);
>+    result.iterator->value = std::make_unique<ProcessAssertion>(process->processIdentifier(), "WebKit Media Playback"_s, AssertionState::Foreground, AssertionReason::MediaPlayback);
>+}
>+
>+void WebProcessPool::clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier processID)
>+{
>+    auto result = m_processesPlayingAudibleMedia.take(processID);
>+    ASSERT_UNUSED(result, result);
>+
>+    auto* process = WebProcessProxy::processForIdentifier(processID);
>+    ASSERT(process);
>+    RELEASE_LOG(ProcessSuspension, "Web process pid %u is no longer playing audible media", (unsigned)process->processIdentifier());
>+
>+    if (m_processesPlayingAudibleMedia.isEmpty()) {
>+        RELEASE_LOG(ProcessSuspension, "The number of processes playing audible media now zero. Releasing UI process assertion.");
>+
>+        ASSERT(m_uiProcessMediaPlaybackAssertion);
>+        m_uiProcessMediaPlaybackAssertion = nullptr;
>+    }
>+}
>+
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
>index 49ef97236851c5332a827d368351aae787e56537..165545b1a2f86eaa4b833c93abe80dc90ee52633 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.h
>+++ b/Source/WebKit/UIProcess/WebProcessPool.h
>@@ -505,6 +505,9 @@ public:
>     void setWebProcessHasUploads(WebCore::ProcessIdentifier);
>     void clearWebProcessHasUploads(WebCore::ProcessIdentifier);
> 
>+    void setWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier);
>+    void clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdentifier);
>+
>     void disableDelayedWebProcessLaunch() { m_isDelayedWebProcessLaunchDisabled = true; }
> 
> private:
>@@ -788,6 +791,10 @@ private:
> 
>     HashMap<WebCore::ProcessIdentifier, std::unique_ptr<ProcessAssertion>> m_processesWithUploads;
>     std::unique_ptr<ProcessAssertion> m_uiProcessUploadAssertion;
>+
>+    HashMap<WebCore::ProcessIdentifier, std::unique_ptr<ProcessAssertion>> m_processesPlayingAudibleMedia;
>+    std::unique_ptr<ProcessAssertion> m_uiProcessMediaPlaybackAssertion;
>+
> #if PLATFORM(IOS)
>     // FIXME: Delayed process launch is currently disabled on iOS for performance reasons (rdar://problem/49074131).
>     bool m_isDelayedWebProcessLaunchDisabled { true };
>diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp
>index bfedee5479e07ec1d2d0560c2627e11be7918e69..79c2e5a56cae66b32076688204d4d2059301cd24 100644
>--- a/Source/WebKit/UIProcess/WebProcessProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessProxy.cpp
>@@ -1209,7 +1209,20 @@ void WebProcessProxy::didSetAssertionState(AssertionState state)
>     UNUSED_PARAM(state);
> #endif
> }
>-    
>+
>+void WebProcessProxy::webPageMediaStateDidChange(WebPageProxy&)
>+{
>+    bool newHasAudibleWebPage = WTF::anyOf(m_pageMap.values(), [] (auto& page) { return page->isPlayingAudio(); });
>+    if (m_hasAudibleWebPage == newHasAudibleWebPage)
>+        return;
>+    m_hasAudibleWebPage = newHasAudibleWebPage;
>+
>+    if (m_hasAudibleWebPage)
>+        processPool().setWebProcessIsPlayingAudibleMedia(coreProcessIdentifier());
>+    else
>+        processPool().clearWebProcessIsPlayingAudibleMedia(coreProcessIdentifier());
>+}
>+
> void WebProcessProxy::setIsHoldingLockedFiles(bool isHoldingLockedFiles)
> {
>     if (!isHoldingLockedFiles) {
>diff --git a/Source/WebKit/UIProcess/WebProcessProxy.h b/Source/WebKit/UIProcess/WebProcessProxy.h
>index 1284aea5187c1f884acf255ca030ed93cadeff85..0a47b6c13f843f48414a656e0ffe76b5f9eae7d9 100644
>--- a/Source/WebKit/UIProcess/WebProcessProxy.h
>+++ b/Source/WebKit/UIProcess/WebProcessProxy.h
>@@ -306,6 +306,8 @@ public:
>     void processWasUnexpectedlyUnsuspended(CompletionHandler<void()>&&);
> #endif
> 
>+    void webPageMediaStateDidChange(WebPageProxy&);
>+
> protected:
>     static uint64_t generatePageID();
>     WebProcessProxy(WebProcessPool&, WebsiteDataStore*, IsPrewarmed);
>@@ -485,6 +487,7 @@ private:
>     unsigned m_suspendedPageCount { 0 };
>     bool m_hasCommittedAnyProvisionalLoads { false };
>     bool m_isPrewarmed;
>+    bool m_hasAudibleWebPage { false };
> 
> #if PLATFORM(WATCHOS)
>     ProcessThrottler::BackgroundActivityToken m_backgroundActivityTokenForFullscreenFormControls;
>diff --git a/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm b/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm
>index 8aa2dcf04c346e032443d1f9a20334af2224fcac..dc35acce8d73c455e0f4c7d7d47f48e6421ffa20 100644
>--- a/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm
>@@ -181,6 +181,8 @@ static BKSProcessAssertionReason toBKSProcessAssertionReason(AssertionReason rea
>         return BKSProcessAssertionReasonFinishTask;
>     case AssertionReason::FinishTaskUnbounded:
>         return BKSProcessAssertionReasonFinishTaskUnbounded;
>+    case AssertionReason::MediaPlayback:
>+        return BKSProcessAssertionReasonMediaPlayback;
>     }
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 197798: 369597 | 369604 | 369612 | 369774 | 369776 | 369777