Changeset 275516 in webkit

Timestamp:

Apr 6, 2021 7:37:27 AM (3 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION(r274358) [GStreamer] http/tests/images/mp4-partial-load.html 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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/glib/TestExpectations (modified) (2 diffs)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp (modified) (5 diffs)
• Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/platform/glib/TestExpectations

@@ -585 +585 @@
 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 ]
@@ -2046 +2046 @@
 
 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 ]

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -1574 +1574 @@
 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 ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp

@@ -34 +34 @@
 #include <wtf/MainThread.h>
 #include <wtf/Optional.h>
+#include <wtf/Scope.h>
 #include <wtf/Threading.h>
 
@@ -303 +304 @@
     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;
@@ -322 +331 @@
         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;
@@ -355 +364 @@
     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);
@@ -364 +379 @@
                     weakThis->handleMessage(protectedMessage.get());
             });
+        }
+        if (!decoder.m_messageDispatched) {
+            LockHolder lock(decoder.m_messageLock);
+            decoder.m_messageCondition.wait(decoder.m_messageLock);
         }
         gst_message_unref(message);

trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h

@@ -114 +114 @@
         GRefPtr<GstElement> m_decodebin;
         RunLoop& m_runLoop;
+
+        Condition m_messageCondition;
+        Lock m_messageLock;
+        bool m_messageDispatched { false };
     };