Title: [228663] releases/WebKitGTK/webkit-2.20
Revision
228663
Author
carlo...@webkit.org
Date
2018-02-19 05:16:52 -0800 (Mon, 19 Feb 2018)

Log Message

Merge r228271 - [GStreamer] LayoutTest webaudio/silent-audio-interrupted-in-background.html makes its subsequent test flaky crash
https://bugs.webkit.org/show_bug.cgi?id=173916

Reviewed by Xabier Rodriguez Calvar.

Source/WebCore:

This patch fixes two crashes and a runtime warning:

- The provider client configuration should be done from the main
thread but the no-more-pads signal of deinterleave was fired from
a non-main thread.

- The deinterleave pad-removed signal can be fired for a not fully
configured pipeline if the audio context is interrupted. So the
peer quark of the removed pad needs to be checked, it might be a
null pointer.

- The provider connects to the deinterleave signals only when a
client is provided, so the signal disconnection needs to check
that to avoid runtime warnings.

* platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
(WebCore::AudioSourceProviderGStreamer::AudioSourceProviderGStreamer):
Create a main thread notifier.
(WebCore::AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer):
Invalidate notifier and check a client was set before
disconnecting from deinterleave signals.
(WebCore::AudioSourceProviderGStreamer::handleRemovedDeinterleavePad):
Check validity of the pad peer.
(WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured):
Set client from main thread.
* platform/audio/gstreamer/AudioSourceProviderGStreamer.h:

LayoutTests:

* platform/gtk/TestExpectations: Unskip fixed test.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog (228662 => 228663)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog	2018-02-19 13:16:13 UTC (rev 228662)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog	2018-02-19 13:16:52 UTC (rev 228663)
@@ -1,3 +1,12 @@
+2018-02-08  Philippe Normand  <pnorm...@igalia.com>
+
+        [GStreamer] LayoutTest webaudio/silent-audio-interrupted-in-background.html makes its subsequent test flaky crash
+        https://bugs.webkit.org/show_bug.cgi?id=173916
+
+        Reviewed by Xabier Rodriguez Calvar.
+
+        * platform/gtk/TestExpectations: Unskip fixed test.
+
 2018-02-07  Youenn Fablet  <you...@apple.com>
 
         ASSERTION FAILED: m_timeOrigin in Performance::Performance()

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/TestExpectations (228662 => 228663)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/TestExpectations	2018-02-19 13:16:13 UTC (rev 228662)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/platform/gtk/TestExpectations	2018-02-19 13:16:52 UTC (rev 228663)
@@ -1293,8 +1293,6 @@
 
 webkit.org/b/172955 media/video-preload.html [ Crash Pass ]
 
-webkit.org/b/173916 webaudio/silent-audio-interrupted-in-background.html [ Skip ]
-
 webkit.org/b/175575 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/ready-states/autoplay-with-slow-text-tracks.html [ Crash Pass ]
 
 webkit.org/b/176797 accessibility/aria-listbox-no-selection.html [ Crash ]

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (228662 => 228663)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-02-19 13:16:13 UTC (rev 228662)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-02-19 13:16:52 UTC (rev 228663)
@@ -1,5 +1,39 @@
 2018-02-08  Philippe Normand  <pnorm...@igalia.com>
 
+        [GStreamer] LayoutTest webaudio/silent-audio-interrupted-in-background.html makes its subsequent test flaky crash
+        https://bugs.webkit.org/show_bug.cgi?id=173916
+
+        Reviewed by Xabier Rodriguez Calvar.
+
+        This patch fixes two crashes and a runtime warning:
+
+        - The provider client configuration should be done from the main
+        thread but the no-more-pads signal of deinterleave was fired from
+        a non-main thread.
+
+        - The deinterleave pad-removed signal can be fired for a not fully
+        configured pipeline if the audio context is interrupted. So the
+        peer quark of the removed pad needs to be checked, it might be a
+        null pointer.
+
+        - The provider connects to the deinterleave signals only when a
+        client is provided, so the signal disconnection needs to check
+        that to avoid runtime warnings.
+
+        * platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
+        (WebCore::AudioSourceProviderGStreamer::AudioSourceProviderGStreamer):
+        Create a main thread notifier.
+        (WebCore::AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer):
+        Invalidate notifier and check a client was set before
+        disconnecting from deinterleave signals.
+        (WebCore::AudioSourceProviderGStreamer::handleRemovedDeinterleavePad):
+        Check validity of the pad peer.
+        (WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured):
+        Set client from main thread.
+        * platform/audio/gstreamer/AudioSourceProviderGStreamer.h:
+
+2018-02-08  Philippe Normand  <pnorm...@igalia.com>
+
         [GStreamer][WebAudio] No need for version check in each loop iteration
         https://bugs.webkit.org/show_bug.cgi?id=182577
 

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp (228662 => 228663)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp	2018-02-19 13:16:13 UTC (rev 228662)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp	2018-02-19 13:16:52 UTC (rev 228663)
@@ -83,7 +83,8 @@
 }
 
 AudioSourceProviderGStreamer::AudioSourceProviderGStreamer()
-    : m_client(nullptr)
+    : m_notifier(MainThreadNotifier<MainThreadNotification>::create())
+    , m_client(nullptr)
     , m_deinterleaveSourcePads(0)
     , m_deinterleavePadAddedHandlerId(0)
     , m_deinterleaveNoMorePadsHandlerId(0)
@@ -96,8 +97,10 @@
 
 AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer()
 {
+    m_notifier->invalidate();
+
     GRefPtr<GstElement> deinterleave = adoptGRef(gst_bin_get_by_name(GST_BIN(m_audioSinkBin.get()), "deinterleave"));
-    if (deinterleave) {
+    if (deinterleave && m_client) {
         g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadAddedHandlerId);
         g_signal_handler_disconnect(deinterleave.get(), m_deinterleaveNoMorePadsHandlerId);
         g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadRemovedHandlerId);
@@ -318,7 +321,10 @@
 
     // Remove the queue ! appsink chain downstream of deinterleave.
     GQuark quark = g_quark_from_static_string("peer");
-    GstPad* sinkPad = reinterpret_cast<GstPad*>(g_object_get_qdata(G_OBJECT(pad), quark));
+    GstPad* sinkPad = GST_PAD_CAST(g_object_get_qdata(G_OBJECT(pad), quark));
+    if (!sinkPad)
+        return;
+
     GRefPtr<GstElement> queue = adoptGRef(gst_pad_get_parent_element(sinkPad));
     GRefPtr<GstPad> queueSrcPad = adoptGRef(gst_element_get_static_pad(queue.get(), "src"));
     GRefPtr<GstPad> appsinkSinkPad = adoptGRef(gst_pad_get_peer(queueSrcPad.get()));
@@ -331,10 +337,12 @@
 
 void AudioSourceProviderGStreamer::deinterleavePadsConfigured()
 {
-    ASSERT(m_client);
-    ASSERT(m_deinterleaveSourcePads == gNumberOfChannels);
+    m_notifier->notify(MainThreadNotification::DeinterleavePadsConfigured, [this] {
+        ASSERT(m_client);
+        ASSERT(m_deinterleaveSourcePads == gNumberOfChannels);
 
-    m_client->setFormat(m_deinterleaveSourcePads, gSampleBitRate);
+        m_client->setFormat(m_deinterleaveSourcePads, gSampleBitRate);
+    });
 }
 
 void AudioSourceProviderGStreamer::clearAdapters()

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h (228662 => 228663)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h	2018-02-19 13:16:13 UTC (rev 228662)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h	2018-02-19 13:16:52 UTC (rev 228663)
@@ -23,6 +23,7 @@
 
 #include "AudioSourceProvider.h"
 #include "GRefPtrGStreamer.h"
+#include "MainThreadNotifier.h"
 #include <gst/gst.h>
 #include <wtf/Forward.h>
 #include <wtf/Noncopyable.h>
@@ -53,6 +54,10 @@
     void clearAdapters();
 
 private:
+    enum MainThreadNotification {
+        DeinterleavePadsConfigured = 1 << 0,
+    };
+    Ref<MainThreadNotifier<MainThreadNotification>> m_notifier;
     GRefPtr<GstElement> m_audioSinkBin;
     AudioSourceProviderClient* m_client;
     int m_deinterleaveSourcePads;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to