Title: [228639] trunk/Source/WebCore
Revision
228639
Author
calva...@igalia.com
Date
2018-02-19 02:47:49 -0800 (Mon, 19 Feb 2018)

Log Message

[GStreamer] Crash in WebCore::MediaPlayerRequestInstallMissingPluginsCallback::complete
https://bugs.webkit.org/show_bug.cgi?id=166733

Reviewed by Philippe Normand.

There are a couple of issues to tackle here.

First is handling getting more than one missing plugin
installation request at the same time. For this we add the request
to a Vector and handle them there.

Second is that if the player is dead and we still get the result,
bad things happen. For that we "weaked" the pointer capture by the
lambda.

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer):
Handle Vector of callbacks.
(WebCore::MediaPlayerPrivateGStreamer::handleMessage): Weak
private player pointer and put the callback in the Vector.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
Callback becomes Vector.
* platform/graphics/gstreamer/MediaPlayerRequestInstallMissingPluginsCallback.h:
(WebCore::MediaPlayerRequestInstallMissingPluginsCallback::create):
(WebCore::MediaPlayerRequestInstallMissingPluginsCallback::complete):
(WebCore::MediaPlayerRequestInstallMissingPluginsCallback::MediaPlayerRequestInstallMissingPluginsCallback):
Callback function is refactored into a "using" type and added self
as parameter to the function.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (228638 => 228639)


--- trunk/Source/WebCore/ChangeLog	2018-02-19 10:47:07 UTC (rev 228638)
+++ trunk/Source/WebCore/ChangeLog	2018-02-19 10:47:49 UTC (rev 228639)
@@ -1,3 +1,34 @@
+2018-02-19  Xabier Rodriguez Calvar  <calva...@igalia.com>
+
+        [GStreamer] Crash in WebCore::MediaPlayerRequestInstallMissingPluginsCallback::complete
+        https://bugs.webkit.org/show_bug.cgi?id=166733
+
+        Reviewed by Philippe Normand.
+
+        There are a couple of issues to tackle here.
+
+        First is handling getting more than one missing plugin
+        installation request at the same time. For this we add the request
+        to a Vector and handle them there.
+
+        Second is that if the player is dead and we still get the result,
+        bad things happen. For that we "weaked" the pointer capture by the
+        lambda.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer):
+        Handle Vector of callbacks.
+        (WebCore::MediaPlayerPrivateGStreamer::handleMessage): Weak
+        private player pointer and put the callback in the Vector.
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+        Callback becomes Vector.
+        * platform/graphics/gstreamer/MediaPlayerRequestInstallMissingPluginsCallback.h:
+        (WebCore::MediaPlayerRequestInstallMissingPluginsCallback::create):
+        (WebCore::MediaPlayerRequestInstallMissingPluginsCallback::complete):
+        (WebCore::MediaPlayerRequestInstallMissingPluginsCallback::MediaPlayerRequestInstallMissingPluginsCallback):
+        Callback function is refactored into a "using" type and added self
+        as parameter to the function.
+
 2018-02-19  Philippe Normand  <pnorm...@igalia.com>
 
         [GStreamer] Playbin3 support

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (228638 => 228639)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2018-02-19 10:47:07 UTC (rev 228638)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2018-02-19 10:47:49 UTC (rev 228639)
@@ -191,10 +191,11 @@
             reinterpret_cast<gpointer>(setAudioStreamPropertiesCallback), this);
 
     m_readyTimerHandler.stop();
-    if (m_missingPluginsCallback) {
-        m_missingPluginsCallback->invalidate();
-        m_missingPluginsCallback = nullptr;
+    for (auto& missingPluginCallback : m_missingPluginCallbacks) {
+        if (missingPluginCallback)
+            missingPluginCallback->invalidate();
     }
+    m_missingPluginCallbacks.clear();
 
     if (m_videoSink) {
         GRefPtr<GstPad> videoSinkPad = adoptGRef(gst_element_get_static_pad(m_videoSink.get(), "sink"));
@@ -1107,7 +1108,7 @@
     GST_LOG("Message %s received from element %s", GST_MESSAGE_TYPE_NAME(message), GST_MESSAGE_SRC_NAME(message));
     switch (GST_MESSAGE_TYPE(message)) {
     case GST_MESSAGE_ERROR:
-        if (m_resetPipeline || m_missingPluginsCallback || m_errorOccured)
+        if (m_resetPipeline || !m_missingPluginCallbacks.isEmpty() || m_errorOccured)
             break;
         gst_message_parse_error(message, &err.outPtr(), &debug.outPtr());
         GST_ERROR("Error %d: %s (url="" err->code, err->message, m_url.string().utf8().data());
@@ -1203,17 +1204,25 @@
     case GST_MESSAGE_ELEMENT:
         if (gst_is_missing_plugin_message(message)) {
             if (gst_install_plugins_supported()) {
-                m_missingPluginsCallback = MediaPlayerRequestInstallMissingPluginsCallback::create([this](uint32_t result) {
-                    m_missingPluginsCallback = nullptr;
+                RefPtr<MediaPlayerRequestInstallMissingPluginsCallback> missingPluginCallback = MediaPlayerRequestInstallMissingPluginsCallback::create([weakThis = createWeakPtr()](uint32_t result, MediaPlayerRequestInstallMissingPluginsCallback& missingPluginCallback) {
+                    if (!weakThis) {
+                        GST_INFO("got missing pluging installation callback in destroyed player with result %u", result);
+                        return;
+                    }
+
+                    GST_DEBUG("got missing plugin installation callback with result %u", result);
+                    RefPtr<MediaPlayerRequestInstallMissingPluginsCallback> protectedMissingPluginCallback = &missingPluginCallback;
+                    weakThis->m_missingPluginCallbacks.removeFirst(protectedMissingPluginCallback);
                     if (result != GST_INSTALL_PLUGINS_SUCCESS)
                         return;
 
-                    changePipelineState(GST_STATE_READY);
-                    changePipelineState(GST_STATE_PAUSED);
+                    weakThis->changePipelineState(GST_STATE_READY);
+                    weakThis->changePipelineState(GST_STATE_PAUSED);
                 });
+                m_missingPluginCallbacks.append(missingPluginCallback);
                 GUniquePtr<char> detail(gst_missing_plugin_message_get_installer_detail(message));
                 GUniquePtr<char> description(gst_missing_plugin_message_get_description(message));
-                m_player->client().requestInstallMissingPlugins(String::fromUTF8(detail.get()), String::fromUTF8(description.get()), *m_missingPluginsCallback);
+                m_player->client().requestInstallMissingPlugins(String::fromUTF8(detail.get()), String::fromUTF8(description.get()), *missingPluginCallback);
             }
         }
 #if ENABLE(VIDEO_TRACK) && USE(GSTREAMER_MPEGTS)

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h (228638 => 228639)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2018-02-19 10:47:07 UTC (rev 228638)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2018-02-19 10:47:49 UTC (rev 228639)
@@ -268,7 +268,7 @@
 #endif
     GRefPtr<GstElement> m_autoAudioSink;
     GRefPtr<GstElement> m_downloadBuffer;
-    RefPtr<MediaPlayerRequestInstallMissingPluginsCallback> m_missingPluginsCallback;
+    Vector<RefPtr<MediaPlayerRequestInstallMissingPluginsCallback>> m_missingPluginCallbacks;
 #if ENABLE(VIDEO_TRACK)
     HashMap<AtomicString, RefPtr<AudioTrackPrivateGStreamer>> m_audioTracks;
     HashMap<AtomicString, RefPtr<InbandTextTrackPrivateGStreamer>> m_textTracks;

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerRequestInstallMissingPluginsCallback.h (228638 => 228639)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerRequestInstallMissingPluginsCallback.h	2018-02-19 10:47:07 UTC (rev 228638)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerRequestInstallMissingPluginsCallback.h	2018-02-19 10:47:49 UTC (rev 228639)
@@ -29,7 +29,9 @@
 class MediaPlayerRequestInstallMissingPluginsCallback : public RefCounted<MediaPlayerRequestInstallMissingPluginsCallback> {
     WTF_MAKE_FAST_ALLOCATED();
 public:
-    static Ref<MediaPlayerRequestInstallMissingPluginsCallback> create(WTF::Function<void (uint32_t)>&& function)
+    using MediaPlayerRequestInstallMissingPluginsCallbackFunction = std::function<void(uint32_t, MediaPlayerRequestInstallMissingPluginsCallback&)>;
+
+    static Ref<MediaPlayerRequestInstallMissingPluginsCallback> create(MediaPlayerRequestInstallMissingPluginsCallbackFunction&& function)
     {
         return adoptRef(*new MediaPlayerRequestInstallMissingPluginsCallback(WTFMove(function)));
     }
@@ -43,17 +45,17 @@
     {
         if (!m_function)
             return;
-        m_function(result);
+        m_function(result, *this);
         m_function = nullptr;
     }
 
 private:
-    MediaPlayerRequestInstallMissingPluginsCallback(WTF::Function<void (uint32_t)>&& function)
+    MediaPlayerRequestInstallMissingPluginsCallback(MediaPlayerRequestInstallMissingPluginsCallbackFunction&& function)
         : m_function(WTFMove(function))
     {
     }
 
-    WTF::Function<void (uint32_t)> m_function;
+    MediaPlayerRequestInstallMissingPluginsCallbackFunction m_function;
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to