[GStreamer] webrtc/disable-encryption.html is a crashing flaky
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 May 2020 12:07:18 +0000 (12:07 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 May 2020 12:07:18 +0000 (12:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=211166

Patch by Philippe Normand <pnormand@igalia.com> on 2020-05-14
Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

Make sure the audio and video mediastream source elements are correctly removed and disposed
from their parent bin when resetting the track sources. Before this change there was a
possibility of disposing the elements while they were still in PLAYING state.

* platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(WebCore::_WebKitMediaStreamSrc::SourceData::reset):
(WebCore::webkitMediaStreamSrcDispose):
(WebCore::webkitMediaStreamSrcRemoveTrackByType):

LayoutTests:

* platform/gtk/TestExpectations: Marking webrtc/disable-encryption.html as no longer flaky.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261685 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/gtk/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp

index 3e34aca..d7e7aae 100644 (file)
@@ -1,3 +1,12 @@
+2020-05-14  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer] webrtc/disable-encryption.html is a crashing flaky
+        https://bugs.webkit.org/show_bug.cgi?id=211166
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * platform/gtk/TestExpectations: Marking webrtc/disable-encryption.html as no longer flaky.
+
 2020-05-13  Myles C. Maxfield  <mmaxfield@apple.com>
 
         fast/text/multiple-codeunit-vertical-upright.html is failing
index 23ed29a..88d64dc 100644 (file)
@@ -2901,8 +2901,6 @@ webkit.org/b/210538 imported/w3c/web-platform-tests/css/css-position/position-st
 
 webkit.org/b/210541 imported/w3c/web-platform-tests/html/cross-origin-embedder-policy/require-corp-load-from-cache-storage.https.html [ Failure Pass ]
 
-webkit.org/b/211166 webrtc/disable-encryption.html [ Crash Pass ]
-
 webkit.org/b/211614 imported/w3c/web-platform-tests/wasm/jsapi/constructor/instantiate.any.worker.html [ Failure Pass ]
 
 webkit.org/b/211764 http/wpt/webrtc/generateCertificate.html [ Failure Pass ]
index 53ace4c..8147e5f 100644 (file)
@@ -1,3 +1,19 @@
+2020-05-14  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer] webrtc/disable-encryption.html is a crashing flaky
+        https://bugs.webkit.org/show_bug.cgi?id=211166
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Make sure the audio and video mediastream source elements are correctly removed and disposed
+        from their parent bin when resetting the track sources. Before this change there was a
+        possibility of disposing the elements while they were still in PLAYING state.
+
+        * platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
+        (WebCore::_WebKitMediaStreamSrc::SourceData::reset):
+        (WebCore::webkitMediaStreamSrcDispose):
+        (WebCore::webkitMediaStreamSrcRemoveTrackByType):
+
 2020-05-14  Adrian Perez de Castro  <aperez@igalia.com>
 
         Non-unified build fixed, mid May 2020 edition
index df699a0..7351059 100644 (file)
@@ -191,6 +191,16 @@ struct _WebKitMediaStreamSrc {
         {
             m_firstBufferPts = GST_CLOCK_TIME_NONE;
             m_isVideo = isVideo;
+
+            if (!m_src)
+                return;
+
+            gst_element_set_locked_state(m_src.get(), true);
+            gst_element_set_state(m_src.get(), GST_STATE_NULL);
+            auto parent = adoptGRef(gst_object_get_parent(GST_OBJECT_CAST(m_src.get())));
+            if (parent)
+                gst_bin_remove(GST_BIN_CAST(parent.get()), m_src.get());
+            gst_element_set_locked_state(m_src.get(), false);
             m_src = nullptr;
         }
 
@@ -199,24 +209,25 @@ struct _WebKitMediaStreamSrc {
             return m_src.get();
         }
 
-        static void needDataCb(GstElement*, guint, WebKitMediaStreamSrc::SourceData *self)
+        static void needDataCallback(WebKitMediaStreamSrc::SourceData* self, unsigned)
         {
             self->setEnoughData(false);
         }
 
-        static void enoughDataCb(GstElement*, WebKitMediaStreamSrc::SourceData *self)
+        static void enoughDataCallback(WebKitMediaStreamSrc::SourceData* self)
         {
             self->setEnoughData(true);
         }
 
-        void setSrc(GstElement *src)
+        void setSrc(GRefPtr<GstElement>&& src)
         {
-            m_src = adoptGRef(GST_ELEMENT(g_object_ref_sink(src)));
-            if (GST_IS_APP_SRC(src)) {
-                g_object_set(src, "is-live", true, "format", GST_FORMAT_TIME, "emit-signals", TRUE, "min-percent", 100, nullptr);
-                g_signal_connect(src, "enough-data", G_CALLBACK(enoughDataCb), this);
-                g_signal_connect(src, "need-data", G_CALLBACK(needDataCb), this);
-            }
+            m_src = WTFMove(src);
+            if (!GST_IS_APP_SRC(m_src.get()))
+                return;
+
+            g_object_set(m_src.get(), "is-live", true, "format", GST_FORMAT_TIME, "emit-signals", true, "min-percent", 100, nullptr);
+            g_signal_connect_swapped(m_src.get(), "enough-data", G_CALLBACK(enoughDataCallback), this);
+            g_signal_connect_swapped(m_src.get(), "need-data", G_CALLBACK(needDataCallback), this);
         }
 
         bool isUsed()
@@ -369,15 +380,8 @@ static void webkitMediaStreamSrcDispose(GObject* object)
 {
     WebKitMediaStreamSrc* self = WEBKIT_MEDIA_STREAM_SRC(object);
 
-    if (self->audioSrc.isUsed()) {
-        gst_bin_remove(GST_BIN(self), self->audioSrc.src());
-        self->audioSrc.reset(false);
-    }
-
-    if (self->videoSrc.isUsed()) {
-        gst_bin_remove(GST_BIN(self), self->videoSrc.src());
-        self->videoSrc.reset(true);
-    }
+    self->audioSrc.reset(false);
+    self->videoSrc.reset(true);
 }
 
 static void webkitMediaStreamSrcFinalize(GObject* object)
@@ -584,7 +588,8 @@ static gboolean webkitMediaStreamSrcSetupAppSrc(WebKitMediaStreamSrc* self,
     MediaStreamTrackPrivate* track, WebKitMediaStreamSrc::SourceData* data,
     GstStaticPadTemplate* pad_template, bool onlyTrack)
 {
-    data->setSrc(gst_element_factory_make("appsrc", nullptr));
+    GRefPtr<GstElement> src = gst_element_factory_make("appsrc", nullptr);
+    data->setSrc(WTFMove(src));
     if (track->isCaptureTrack())
         g_object_set(data->src(), "do-timestamp", true, nullptr);
 
@@ -624,19 +629,11 @@ bool webkitMediaStreamSrcAddTrack(WebKitMediaStreamSrc* self, MediaStreamTrackPr
 
 static void webkitMediaStreamSrcRemoveTrackByType(WebKitMediaStreamSrc* self, RealtimeMediaSource::Type trackType)
 {
-    if (trackType == RealtimeMediaSource::Type::Audio) {
-        if (self->audioSrc.isUsed()) {
-            gst_element_set_state(self->audioSrc.src(), GST_STATE_NULL);
-            gst_bin_remove(GST_BIN(self), self->audioSrc.src());
-            self->audioSrc.reset(false);
-        }
-    } else if (trackType == RealtimeMediaSource::Type::Video) {
-        if (self->videoSrc.isUsed()) {
-            gst_element_set_state(self->videoSrc.src(), GST_STATE_NULL);
-            gst_bin_remove(GST_BIN(self), self->videoSrc.src());
-            self->videoSrc.reset(true);
-        }
-    } else
+    if (trackType == RealtimeMediaSource::Type::Audio)
+        self->audioSrc.reset(false);
+    else if (trackType == RealtimeMediaSource::Type::Video)
+        self->videoSrc.reset(true);
+    else
         GST_INFO("Unsupported track type: %d", static_cast<int>(trackType));
 }