REGRESSION (251426@main): [ macOS Debug wk2 ] http/tests/media/modern…

…-media-controls/skip-back-support/skip-back-support-button-click.html is a flaky crash https://bugs.webkit.org/show_bug.cgi?id=241572 rdar://problem/95059414 Reviewed by Cameron McCormack. In bug 241455 it was incorrectly assumed that checking that the IPCHandle was null was a sufficient test to check the validity of the reply. However, this ignore the fact that the IPC decoder will assert if the size isn't valid. So rather than returning a Handle directly, we return an optional<Handle> instead. Covered by the existing test. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp: (WebKit::RemoteMediaResourceManager::dataReceived): * Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.h: * Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.messages.in: * Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp: (WebKit::RemoteSourceBufferProxy::append): * Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h: * Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.messages.in: * Source/WebKit/WebProcess/GPU/media/RemoteMediaResourceProxy.cpp: (WebKit::RemoteMediaResourceProxy::dataReceived): * Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp: (WebKit::SourceBufferPrivateRemote::append): Canonical link: https://commits.webkit.org/251520@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295515 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 14, 2022 · 623a598f89fff02777796a87d35942a8dfe5a621 · 623a598
1 parent bc42958
commit 623a598f89fff02777796a87d35942a8dfe5a621
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 17 deletions.
diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -1027,7 +1027,7 @@ webkit.org/b/207632 tiled-drawing/scrolling/sticky/sticky-during-rubberband.html
 
 webkit.org/b/207631 tiled-drawing/scrolling/fixed/fixed-during-rubberband.html [ Pass ImageOnlyFailure ]
 
-webkit.org/b/207635 fast/events/before-input-prevent-insert-replacement.html [ Pass Failure ] 
+webkit.org/b/207635 fast/events/before-input-prevent-insert-replacement.html [ Pass Failure ]
 
 webkit.org/b/207728 [ Release ] accessibility/press-targets-center-point.html [ Pass Failure ]
 
@@ -1381,15 +1381,15 @@ webkit.org/b/224463 [ BigSur arm64 ] webrtc/disable-encryption.html [ Pass Timeo
 
 webkit.org/b/224633 media/presentationmodechanged-fired-once.html [ Pass Timeout ]
 
-webkit.org/b/228713 [ BigSur Debug arm64 ] compositing/video/video-object-fit.html [ Pass Timeout ] 
+webkit.org/b/228713 [ BigSur Debug arm64 ] compositing/video/video-object-fit.html [ Pass Timeout ]
 
 webkit.org/b/224698 [ BigSur Release arm64 ] inspector/console/console-oom.html [ Pass Crash ]
 
 webkit.org/b/224257 [ Debug ] webgl/2.0.0/conformance/glsl/misc/shader-uniform-packing-restrictions.html [ Slow ]
 
 webkit.org/b/238033 imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-050.html [ Pass Failure ]
 
-webkit.org/b/224963 [ BigSur arm64 ] webrtc/captureCanvas-webrtc.html [ Pass Timeout ] 
+webkit.org/b/224963 [ BigSur arm64 ] webrtc/captureCanvas-webrtc.html [ Pass Timeout ]
 
 webkit.org/b/225430 [ BigSur arm64 ] http/tests/inspector/network/resource-sizes-network.html [ Pass Failure ]
 
@@ -1721,5 +1721,3 @@ webkit.org/b/241191 [ Monterey Debug ] webgl/1.0.3/conformance/attribs/gl-vertex
 webkit.org/b/241265 [ Debug ] imported/w3c/web-platform-tests/html/rendering/replaced-elements/svg-embedded-sizing/svg-in-object-percentage.html [ Pass Crash ]
 
 webkit.org/b/241283 fast/animation/request-animation-frame-throttling-detached-iframe.html [ Pass Failure ]
-
-webkit.org/b/241572 [ Debug ] http/tests/media/modern-media-controls/skip-back-support/skip-back-support-button-click.html [ Pass Crash ]
diff --git a/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp b/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.cpp
@@ -91,12 +91,15 @@ void RemoteMediaResourceManager::dataSent(RemoteMediaResourceIdentifier identifi
     resource->dataSent(bytesSent, totalBytesToBeSent);
 }
 
-void RemoteMediaResourceManager::dataReceived(RemoteMediaResourceIdentifier identifier, IPC::SharedBufferReference&& buffer, CompletionHandler<void(SharedMemory::IPCHandle&&)>&& completionHandler)
+void RemoteMediaResourceManager::dataReceived(RemoteMediaResourceIdentifier identifier, IPC::SharedBufferReference&& buffer, CompletionHandler<void(std::optional<SharedMemory::IPCHandle>&&)>&& completionHandler)
 {
     SharedMemory::Handle handle;
 
     auto invokeCallbackAtScopeExit = makeScopeExit([&] {
-        completionHandler(SharedMemory::IPCHandle { WTFMove(handle), buffer.size() });
+        std::optional<SharedMemory::IPCHandle> response;
+        if (!handle.isNull())
+            response = SharedMemory::IPCHandle { WTFMove(handle), buffer.size() };
+        completionHandler(WTFMove(response));
     });
 
     auto* resource = m_remoteMediaResources.get(identifier);

diff --git a/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.h b/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.h
@@ -67,7 +67,7 @@ class RemoteMediaResourceManager
     void responseReceived(RemoteMediaResourceIdentifier, const WebCore::ResourceResponse&, bool, CompletionHandler<void(WebCore::ShouldContinuePolicyCheck)>&&);
     void redirectReceived(RemoteMediaResourceIdentifier, WebCore::ResourceRequest&&, const WebCore::ResourceResponse&, CompletionHandler<void(WebCore::ResourceRequest&&)>&&);
     void dataSent(RemoteMediaResourceIdentifier, uint64_t, uint64_t);
-    void dataReceived(RemoteMediaResourceIdentifier, IPC::SharedBufferReference&&, CompletionHandler<void(WebKit::SharedMemory::IPCHandle&&)>&&);
+    void dataReceived(RemoteMediaResourceIdentifier, IPC::SharedBufferReference&&, CompletionHandler<void(std::optional<WebKit::SharedMemory::IPCHandle>&&)>&&);
     void accessControlCheckFailed(RemoteMediaResourceIdentifier, const WebCore::ResourceError&);
     void loadFailed(RemoteMediaResourceIdentifier, const WebCore::ResourceError&);
     void loadFinished(RemoteMediaResourceIdentifier, const WebCore::NetworkLoadMetrics&);

diff --git a/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.messages.in b/Source/WebKit/GPUProcess/media/RemoteMediaResourceManager.messages.in
@@ -29,7 +29,7 @@ messages -> RemoteMediaResourceManager NotRefCounted {
     ResponseReceived(WebKit::RemoteMediaResourceIdentifier identifier, WebCore::ResourceResponse response, bool didPassAccessControlCheck) -> (enum:bool WebCore::ShouldContinuePolicyCheck shouldContinue)
     RedirectReceived(WebKit::RemoteMediaResourceIdentifier identifier, WebCore::ResourceRequest request, WebCore::ResourceResponse response) -> (WebCore::ResourceRequest returnRequest)
     DataSent(WebKit::RemoteMediaResourceIdentifier identifier, uint64_t bytesSent, uint64_t totalBytesToBeSent)
-    DataReceived(WebKit::RemoteMediaResourceIdentifier identifier, IPC::SharedBufferReference data) -> (WebKit::SharedMemory::IPCHandle remoteData)
+    DataReceived(WebKit::RemoteMediaResourceIdentifier identifier, IPC::SharedBufferReference data) -> (std::optional<WebKit::SharedMemory::IPCHandle> remoteData)
     AccessControlCheckFailed(WebKit::RemoteMediaResourceIdentifier identifier, WebCore::ResourceError error)
     LoadFailed(WebKit::RemoteMediaResourceIdentifier identifier, WebCore::ResourceError error)
     LoadFinished(WebKit::RemoteMediaResourceIdentifier identifier, WebCore::NetworkLoadMetrics metrics)

diff --git a/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp b/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp
@@ -194,12 +194,15 @@ void RemoteSourceBufferProxy::sourceBufferPrivateBufferedDirtyChanged(bool flag)
     m_connectionToWebProcess->connection().send(Messages::SourceBufferPrivateRemote::SourceBufferPrivateBufferedDirtyChanged(flag), m_identifier);
 }
 
-void RemoteSourceBufferProxy::append(IPC::SharedBufferReference&& buffer, CompletionHandler<void(SharedMemory::IPCHandle&&)>&& completionHandler)
+void RemoteSourceBufferProxy::append(IPC::SharedBufferReference&& buffer, CompletionHandler<void(std::optional<SharedMemory::IPCHandle>&&)>&& completionHandler)
 {
     SharedMemory::Handle handle;
 
     auto invokeCallbackAtScopeExit = makeScopeExit([&] {
-        completionHandler(SharedMemory::IPCHandle { WTFMove(handle), buffer.size() });
+        std::optional<SharedMemory::IPCHandle> response;
+        if (!handle.isNull())
+            response = SharedMemory::IPCHandle { WTFMove(handle), buffer.size() };
+        completionHandler(WTFMove(response));
     });
 
     auto sharedMemory = buffer.sharedCopy();

diff --git a/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h b/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h
@@ -89,7 +89,7 @@ class RemoteSourceBufferProxy final
     void setActive(bool);
     void canSwitchToType(const WebCore::ContentType&, CompletionHandler<void(bool)>&&);
     void setMode(WebCore::SourceBufferAppendMode);
-    void append(IPC::SharedBufferReference&&, CompletionHandler<void(WebKit::SharedMemory::IPCHandle&&)>&&);
+    void append(IPC::SharedBufferReference&&, CompletionHandler<void(std::optional<WebKit::SharedMemory::IPCHandle>&&)>&&);
     void abort();
     void resetParserState();
     void removedFromMediaSource();

diff --git a/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.messages.in b/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.messages.in
@@ -29,7 +29,7 @@ messages -> RemoteSourceBufferProxy NotRefCounted {
     SetActive(bool active)
     CanSwitchToType(WebCore::ContentType contentType) -> (bool canSwitch) Synchronous
     SetMode(WebCore::SourceBufferAppendMode appendMode)
-    Append(IPC::SharedBufferReference data) -> (WebKit::SharedMemory::IPCHandle remoteData)
+    Append(IPC::SharedBufferReference data) -> (std::optional<WebKit::SharedMemory::IPCHandle> remoteData)
     Abort()
     ResetParserState()
     RemovedFromMediaSource()

diff --git a/Source/WebKit/WebProcess/GPU/media/RemoteMediaResourceProxy.cpp b/Source/WebKit/WebProcess/GPU/media/RemoteMediaResourceProxy.cpp
@@ -76,8 +76,8 @@ void RemoteMediaResourceProxy::dataReceived(WebCore::PlatformMediaResource&, con
 {
     m_connection->sendWithAsyncReply(Messages::RemoteMediaResourceManager::DataReceived(m_id, IPC::SharedBufferReference { buffer }), [] (auto&& bufferHandle) {
         // Take ownership of shared memory and mark it as media-related memory.
-        if (!bufferHandle.handle.isNull())
-            bufferHandle.handle.takeOwnershipOfMemory(MemoryLedger::Media);
+        if (bufferHandle)
+            bufferHandle->handle.takeOwnershipOfMemory(MemoryLedger::Media);
     }, 0);
 }
 

diff --git a/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp b/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp
@@ -86,8 +86,8 @@ void SourceBufferPrivateRemote::append(Ref<SharedBuffer>&& data)
 
     m_gpuProcessConnection->connection().sendWithAsyncReply(Messages::RemoteSourceBufferProxy::Append(IPC::SharedBufferReference { WTFMove(data) }), [] (auto&& bufferHandle) {
         // Take ownership of shared memory and mark it as media-related memory.
-        if (!bufferHandle.handle.isNull())
-            bufferHandle.handle.takeOwnershipOfMemory(MemoryLedger::Media);
+        if (bufferHandle)
+            bufferHandle->handle.takeOwnershipOfMemory(MemoryLedger::Media);
     }, m_remoteSourceBufferIdentifier);
 }