Title: [294937] trunk/Source/WebKit
Revision
294937
Author
j...@apple.com
Date
2022-05-27 07:47:04 -0700 (Fri, 27 May 2022)

Log Message

Simplify MediaPlayerPrivateRemote::RequestResource API
https://bugs.webkit.org/show_bug.cgi?id=240999
<rdar://94012261>

Reviewed by Eric Carlson.

When the GPU Process' MediaPlayerProxy needs to allocate a new MediaResource it sends
a message to the content process via the MediaPlayerPrivateRemote::RequestResource
IPC call with a RemoteMediaResourceIdentifier which will then respond to indicate that
the MediaResource is now "ready".

There's two scenarios possible here:
Either the creation of the media resource in the content process is successful or it's not.

If it's successful the content process will start sending data starting by a call to the
GPU's RemoteMediaResourceManager responseReceived
Or it will fail and call RemoteMediaResourceManager::LoadFailed.

The RemoteMediaResourceManager will only accept incoming data once the MediaResource's
ready status is true, and if you're only working on the main thread, it will always be,
as the Content Process' RequestResource response will always be received before either
a LoadFailed or ResponseReceived call.
Under these conditions, the ready status is totally redundant. Testing that the
RemoteMediaResource exists in the MediaResourceManager's map is sufficient.

However, if we want to parallelise networking operations so that ResponseReceived and
LoadFailed will be called on a secondary thread as introduced in bug 235353;
the "ready" flag becomes problematic as the response from a RequestResource is handled
on the main thread.
The multi-threaded nature of the work means that the RequestResource response could be
received only after LoadFailed or ResponseReceived message; and if that's the case
those two messages will be dropped.

This can be seen with the intermittent failures occurring with media/video-src-blob-replay.html test.

So we remove this concept of RemoteMediaResource::ready as at best it serves no purpose.

* Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:
(WebKit::RemoteMediaPlayerProxy::requestResource):
* Source/WebKit/GPUProcess/media/RemoteMediaResource.h:
(WebKit::RemoteMediaResource::ready const): Deleted.
(WebKit::RemoteMediaResource::setReady): Deleted.
* Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp:
(WebKit::RemoteMediaResourceManager::responseReceived):
(WebKit::RemoteMediaResourceManager::redirectReceived):
(WebKit::RemoteMediaResourceManager::dataSent):
(WebKit::RemoteMediaResourceManager::dataReceived):
(WebKit::RemoteMediaResourceManager::accessControlCheckFailed):
(WebKit::RemoteMediaResourceManager::loadFailed):
(WebKit::RemoteMediaResourceManager::loadFinished):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::requestResource):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in:

Canonical link: https://commits.webkit.org/251048@main

Modified Paths

Diff

Modified: trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp (294936 => 294937)


--- trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp	2022-05-27 13:17:15 UTC (rev 294936)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp	2022-05-27 14:47:04 UTC (rev 294937)
@@ -302,9 +302,7 @@
     auto remoteMediaResource = RemoteMediaResource::create(remoteMediaResourceManager, *this, remoteMediaResourceIdentifier);
     remoteMediaResourceManager.addMediaResource(remoteMediaResourceIdentifier, remoteMediaResource);
 
-    m_webProcessConnection->sendWithAsyncReply(Messages::MediaPlayerPrivateRemote::RequestResource(remoteMediaResourceIdentifier, request, options), [remoteMediaResource]() {
-        remoteMediaResource->setReady(true);
-    }, m_id);
+    m_webProcessConnection->send(Messages::MediaPlayerPrivateRemote::RequestResource(remoteMediaResourceIdentifier, request, options), m_id);
 
     return remoteMediaResource;
 }

Modified: trunk/Source/WebKit/GPUProcess/media/RemoteMediaResource.h (294936 => 294937)


--- trunk/Source/WebKit/GPUProcess/media/RemoteMediaResource.h	2022-05-27 13:17:15 UTC (rev 294936)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteMediaResource.h	2022-05-27 14:47:04 UTC (rev 294937)
@@ -48,9 +48,6 @@
     static Ref<RemoteMediaResource> create(RemoteMediaResourceManager&, RemoteMediaPlayerProxy&, RemoteMediaResourceIdentifier);
     ~RemoteMediaResource();
 
-    bool ready() const { return m_ready; }
-    void setReady(bool ready) { m_ready = ready; }
-
     // PlatformMediaResource
     void stop() final;
     bool didPassAccessControlCheck() const final;
@@ -70,7 +67,6 @@
     WeakPtr<RemoteMediaPlayerProxy> m_remoteMediaPlayerProxy;
     RemoteMediaResourceIdentifier m_id;
     bool m_didPassAccessControlCheck { false };
-    bool m_ready { false };
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp (294936 => 294937)


--- trunk/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp	2022-05-27 13:17:15 UTC (rev 294936)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp	2022-05-27 14:47:04 UTC (rev 294937)
@@ -61,7 +61,7 @@
 void RemoteMediaResourceManager::responseReceived(RemoteMediaResourceIdentifier identifier, const ResourceResponse& response, bool didPassAccessControlCheck, CompletionHandler<void(ShouldContinuePolicyCheck)>&& completionHandler)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready()) {
+    if (!resource) {
         completionHandler(ShouldContinuePolicyCheck::No);
         return;
     }
@@ -72,7 +72,7 @@
 void RemoteMediaResourceManager::redirectReceived(RemoteMediaResourceIdentifier identifier, ResourceRequest&& request, const ResourceResponse& response, CompletionHandler<void(WebCore::ResourceRequest&&)>&& completionHandler)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready()) {
+    if (!resource) {
         completionHandler({ });
         return;
     }
@@ -83,7 +83,7 @@
 void RemoteMediaResourceManager::dataSent(RemoteMediaResourceIdentifier identifier, uint64_t bytesSent, uint64_t totalBytesToBeSent)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready())
+    if (!resource)
         return;
 
     resource->dataSent(bytesSent, totalBytesToBeSent);
@@ -92,7 +92,7 @@
 void RemoteMediaResourceManager::dataReceived(RemoteMediaResourceIdentifier identifier, const SharedMemory::IPCHandle& bufferHandle)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready())
+    if (!resource)
         return;
 
     auto sharedMemory = SharedMemory::map(bufferHandle.handle, SharedMemory::Protection::ReadOnly);
@@ -104,7 +104,7 @@
 void RemoteMediaResourceManager::accessControlCheckFailed(RemoteMediaResourceIdentifier identifier, const ResourceError& error)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready())
+    if (!resource)
         return;
 
     resource->accessControlCheckFailed(error);
@@ -113,7 +113,7 @@
 void RemoteMediaResourceManager::loadFailed(RemoteMediaResourceIdentifier identifier, const ResourceError& error)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready())
+    if (!resource)
         return;
 
     resource->loadFailed(error);
@@ -122,7 +122,7 @@
 void RemoteMediaResourceManager::loadFinished(RemoteMediaResourceIdentifier identifier, const NetworkLoadMetrics& metrics)
 {
     auto* resource = m_remoteMediaResources.get(identifier);
-    if (!resource || !resource->ready())
+    if (!resource)
         return;
 
     resource->loadFinished(metrics);

Modified: trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp (294936 => 294937)


--- trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp	2022-05-27 13:17:15 UTC (rev 294936)
+++ trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp	2022-05-27 14:47:04 UTC (rev 294937)
@@ -1442,13 +1442,12 @@
     connection().send(Messages::RemoteMediaPlayerProxy::PlayerContentBoxRectChanged(contentRect), m_id);
 }
 
-void MediaPlayerPrivateRemote::requestResource(RemoteMediaResourceIdentifier remoteMediaResourceIdentifier, WebCore::ResourceRequest&& request, WebCore::PlatformMediaResourceLoader::LoadOptions options, CompletionHandler<void()>&& completionHandler)
+void MediaPlayerPrivateRemote::requestResource(RemoteMediaResourceIdentifier remoteMediaResourceIdentifier, WebCore::ResourceRequest&& request, WebCore::PlatformMediaResourceLoader::LoadOptions options)
 {
     ASSERT(!m_mediaResources.contains(remoteMediaResourceIdentifier));
     auto resource = m_mediaResourceLoader->requestResource(WTFMove(request), options);
 
     if (!resource) {
-        completionHandler();
         // FIXME: Get the error from MediaResourceLoader::requestResource.
         connection().send(Messages::RemoteMediaResourceManager::LoadFailed(remoteMediaResourceIdentifier, { ResourceError::Type::Cancellation }), 0);
         return;
@@ -1456,8 +1455,6 @@
     // PlatformMediaResource owns the PlatformMediaResourceClient
     resource->setClient(adoptRef(*new RemoteMediaResourceProxy(connection(), *resource, remoteMediaResourceIdentifier)));
     m_mediaResources.add(remoteMediaResourceIdentifier, WTFMove(resource));
-
-    completionHandler();
 }
 
 void MediaPlayerPrivateRemote::sendH2Ping(const URL& url, CompletionHandler<void(Expected<WTF::Seconds, WebCore::ResourceError>&&)>&& completionHandler)

Modified: trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h (294936 => 294937)


--- trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h	2022-05-27 13:17:15 UTC (rev 294936)
+++ trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h	2022-05-27 14:47:04 UTC (rev 294937)
@@ -147,7 +147,7 @@
     void updateGenericCue(TrackPrivateRemoteIdentifier, WebCore::GenericCueData&&);
     void removeGenericCue(TrackPrivateRemoteIdentifier, WebCore::GenericCueData&&);
 
-    void requestResource(RemoteMediaResourceIdentifier, WebCore::ResourceRequest&&, WebCore::PlatformMediaResourceLoader::LoadOptions, CompletionHandler<void()>&&);
+    void requestResource(RemoteMediaResourceIdentifier, WebCore::ResourceRequest&&, WebCore::PlatformMediaResourceLoader::LoadOptions);
     void removeResource(RemoteMediaResourceIdentifier);
     void sendH2Ping(const URL&, CompletionHandler<void(Expected<WTF::Seconds, WebCore::ResourceError>&&)>&&);
     void resourceNotSupported();

Modified: trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in (294936 => 294937)


--- trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in	2022-05-27 13:17:15 UTC (rev 294936)
+++ trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in	2022-05-27 14:47:04 UTC (rev 294937)
@@ -68,7 +68,7 @@
     UpdateGenericCue(WebKit::TrackPrivateRemoteIdentifier trackID, struct WebCore::GenericCueData cue)
     RemoveGenericCue(WebKit::TrackPrivateRemoteIdentifier trackID, struct WebCore::GenericCueData cue)
 
-    RequestResource(WebKit::RemoteMediaResourceIdentifier remoteMediaResourceIdentifier, WebCore::ResourceRequest request, enum:uint8_t WebCore::PlatformMediaResourceLoader::LoadOptions options) -> ()
+    RequestResource(WebKit::RemoteMediaResourceIdentifier remoteMediaResourceIdentifier, WebCore::ResourceRequest request, enum:uint8_t WebCore::PlatformMediaResourceLoader::LoadOptions options)
     RemoveResource(WebKit::RemoteMediaResourceIdentifier remoteMediaResourceIdentifier)
     SendH2Ping(URL url) -> (Expected<Seconds, WebCore::ResourceError> result)
     ResourceNotSupported()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to