REGRESSION(r274358) [GStreamer] http/tests/images/mp4-partial-load.ht…

…ml is flaky crashing inside GStreamer https://bugs.webkit.org/show_bug.cgi?id=223636 Patch by Philippe Normand <pnormand@igalia.com> on 2021-04-06 Reviewed by Carlos Alberto Lopez Perez. Source/WebCore: Ensure the GStreamer messages are always processed synchronously, even when the handler is called from another thread. Without blocking in the latter case, the select-streams event might be sent to decodebin too late. It has to be sent as a synchronous reaction to the stream-collection message. * platform/graphics/gstreamer/ImageDecoderGStreamer.cpp: (WebCore::ImageDecoderGStreamer::InnerDecoder::handleMessage): (WebCore::ImageDecoderGStreamer::InnerDecoder::preparePipeline): * platform/graphics/gstreamer/ImageDecoderGStreamer.h: LayoutTests: * platform/glib/TestExpectations: Update tests expectations. * platform/gtk/TestExpectations: Ditto. Canonical link: https://commits.webkit.org/236173@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275516 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 6, 2021 · 5f6e80a5e0077df752220390f9ef620368b1a50f · 5f6e80a
1 parent 0473762
commit 5f6e80a5e0077df752220390f9ef620368b1a50f
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 5 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2021-04-06  Philippe Normand  <pnormand@igalia.com>
+
+        REGRESSION(r274358) [GStreamer] http/tests/images/mp4-partial-load.html is flaky crashing inside GStreamer
+        https://bugs.webkit.org/show_bug.cgi?id=223636
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        * platform/glib/TestExpectations: Update tests expectations.
+        * platform/gtk/TestExpectations: Ditto.
+
 2021-04-06  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][Integration] Enable inline box support (RenderInline)

diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -584,7 +584,7 @@ webkit.org/b/214798 http/tests/images/image-supports-video.html [ Missing Timeou
 
 webkit.org/b/207723 http/tests/contentextensions/hide-on-ping-with-ping-that-redirects.html [ Pass Failure ]
 
-webkit.org/b/217961 webkit.org/b/223636 http/tests/images/mp4-partial-load.html [ Crash Timeout Pass ]
+webkit.org/b/217961 http/tests/images/mp4-partial-load.html [ Timeout Pass ]
 
 webkit.org/b/224109 http/tests/privateClickMeasurement/store-private-click-measurement-with-source-nonce.html [ Failure Pass ]
 
@@ -2045,7 +2045,6 @@ webkit.org/b/224068 http/wpt/webrtc/webrtc-late-transform.html [ Failure ]
 webkit.org/b/224071 webgl/pending/conformance/context/context-attributes-alpha-depth-stencil-antialias.html [ Timeout ]
 
 webkit.org/b/155196 security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star.html [ ImageOnlyFailure Pass ]
-webkit.org/b/224107 fast/images/animated-image-mp4-crash.html [ Crash Timeout Pass ]
 webkit.org/b/224114 imported/w3c/web-platform-tests/wasm/webapi/instantiateStreaming-bad-imports.any.worker.html [ Failure Pass ]
 webkit.org/b/224115 imported/w3c/web-platform-tests/service-workers/service-worker/unregister-immediately-before-installed.https.html [ Failure Pass ]
 webkit.org/b/224116 imported/w3c/web-platform-tests/navigation-timing/test_performance_attributes.sub.html [ Failure Pass ]

diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations
@@ -1573,8 +1573,6 @@ webkit.org/b/224106 fast/forms/textfield-onchange-without-focus.html [ ImageOnly
 
 webkit.org/b/224108 gamepad/gamepad-visibility-1.html [ Failure Pass ]
 
-webkit.org/b/224112 media/video-as-img-output-pts.html [ Crash Timeout Pass ]
-
 webkit.org/b/224117 intersection-observer/intersection-observer-keeps-js-wrapper-of-target-alive.html [ Failure Pass ]
 
 #////////////////////////////////////////////////////////////////////////////////////////

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2021-04-06  Philippe Normand  <pnormand@igalia.com>
+
+        REGRESSION(r274358) [GStreamer] http/tests/images/mp4-partial-load.html is flaky crashing inside GStreamer
+        https://bugs.webkit.org/show_bug.cgi?id=223636
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        Ensure the GStreamer messages are always processed synchronously, even when the handler is
+        called from another thread. Without blocking in the latter case, the select-streams event
+        might be sent to decodebin too late. It has to be sent as a synchronous reaction to the
+        stream-collection message.
+
+        * platform/graphics/gstreamer/ImageDecoderGStreamer.cpp:
+        (WebCore::ImageDecoderGStreamer::InnerDecoder::handleMessage):
+        (WebCore::ImageDecoderGStreamer::InnerDecoder::preparePipeline):
+        * platform/graphics/gstreamer/ImageDecoderGStreamer.h:
+
 2021-04-06  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][Integration] Enable inline box support (RenderInline)

diff --git a/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp
@@ -33,6 +33,7 @@
 #include <wtf/Lock.h>
 #include <wtf/MainThread.h>
 #include <wtf/Optional.h>
+#include <wtf/Scope.h>
 #include <wtf/Threading.h>
 
 namespace WebCore {
@@ -302,6 +303,14 @@ void ImageDecoderGStreamer::InnerDecoder::handleMessage(GstMessage* message)
 {
     ASSERT(&m_runLoop == &RunLoop::current());
 
+    auto scopeExit = makeScopeExit([protectedThis = makeWeakPtr(this)] {
+        if (!protectedThis)
+            return;
+        LockHolder lock(protectedThis->m_messageLock);
+        protectedThis->m_messageDispatched = true;
+        protectedThis->m_messageCondition.notifyOne();
+    });
+
     GUniqueOutPtr<GError> error;
     GUniqueOutPtr<gchar> debug;
 
@@ -321,7 +330,7 @@ void ImageDecoderGStreamer::InnerDecoder::handleMessage(GstMessage* message)
     case GST_MESSAGE_STREAM_COLLECTION: {
         GRefPtr<GstStreamCollection> collection;
         gst_message_parse_stream_collection(message, &collection.outPtr());
-        if (collection) {
+        if (collection && GST_MESSAGE_SRC(message) == GST_OBJECT_CAST(m_decodebin.get())) {
             unsigned size = gst_stream_collection_get_size(collection.get());
             GList* streams = nullptr;
             for (unsigned i = 0 ; i < size; i++) {
@@ -354,6 +363,12 @@ void ImageDecoderGStreamer::InnerDecoder::preparePipeline()
 
     gst_bus_set_sync_handler(bus.get(), [](GstBus*, GstMessage* message, gpointer userData) {
         auto& decoder = *static_cast<ImageDecoderGStreamer::InnerDecoder*>(userData);
+
+        {
+            LockHolder lock(decoder.m_messageLock);
+            decoder.m_messageDispatched = false;
+            decoder.m_messageCondition.notifyOne();
+        }
         if (&decoder.m_runLoop == &RunLoop::current())
             decoder.handleMessage(message);
         else {
@@ -364,6 +379,10 @@ void ImageDecoderGStreamer::InnerDecoder::preparePipeline()
                     weakThis->handleMessage(protectedMessage.get());
             });
         }
+        if (!decoder.m_messageDispatched) {
+            LockHolder lock(decoder.m_messageLock);
+            decoder.m_messageCondition.wait(decoder.m_messageLock);
+        }
         gst_message_unref(message);
         return GST_BUS_DROP;
     }, this, nullptr);

diff --git a/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h
@@ -113,6 +113,10 @@ class ImageDecoderGStreamer final : public ImageDecoder {
         GRefPtr<GInputStream> m_memoryStream;
         GRefPtr<GstElement> m_decodebin;
         RunLoop& m_runLoop;
+
+        Condition m_messageCondition;
+        Lock m_messageLock;
+        bool m_messageDispatched { false };
     };
 
     void handleSample(GRefPtr<GstSample>&&);