[GStreamer] media/video-controls-fullscreen-volume.html crashes
authorphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Feb 2013 17:29:09 +0000 (17:29 +0000)
committerphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Feb 2013 17:29:09 +0000 (17:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108682

Reviewed by Martin Robinson.

Source/WebCore:

Clean up various signal handlers and avoid bad interaction between
the FullscreenVideoControllerGStreamer and its subclasses,
especially when the platform video window is created.

* platform/graphics/gstreamer/FullscreenVideoControllerGStreamer.cpp:
(WebCore::FullscreenVideoControllerGStreamer::enterFullscreen):
Initialize the window before connecting to the volume/mute
signals. This ensures that the signals won't ever interfere with
an inexisting window.
* platform/graphics/gstreamer/GStreamerGWorld.cpp:
(WebCore::GStreamerGWorld::~GStreamerGWorld): Remove GstBus
synchronous handler function.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
Disconnect from volume/mute signals.
(WebCore::MediaPlayerPrivateGStreamerBase::setStreamVolumeElement):
Keep a trace of volume/mute signal handlers.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
Various forward type declarations to avoid un-necessary header includes.
(MediaPlayerPrivateGStreamerBase):
* platform/graphics/gtk/FullscreenVideoControllerGtk.cpp:
(WebCore::FullscreenVideoControllerGtk::FullscreenVideoControllerGtk):
(WebCore::FullscreenVideoControllerGtk::volumeChanged): Bail out
if volume button hasn't been created yet.
(WebCore::FullscreenVideoControllerGtk::muteChanged): Ditto.

LayoutTests:

* platform/gtk/TestExpectations: Unflag now passing tests.

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

LayoutTests/ChangeLog
LayoutTests/platform/gtk/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/gstreamer/FullscreenVideoControllerGStreamer.cpp
Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h
Source/WebCore/platform/graphics/gtk/FullscreenVideoControllerGtk.cpp

index 351af42..abdf755 100644 (file)
@@ -1,3 +1,12 @@
+2013-02-10  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer] media/video-controls-fullscreen-volume.html crashes
+        https://bugs.webkit.org/show_bug.cgi?id=108682
+
+        Reviewed by Martin Robinson.
+
+        * platform/gtk/TestExpectations: Unflag now passing tests.
+
 2013-02-10  Kent Tamura  <tkent@chromium.org>
 
         [Chromium] Test expectation update
index 7705cce..aa8dfd1 100644 (file)
@@ -530,9 +530,6 @@ webkit.org/b/107377 storage/indexeddb/mozilla/create-index-unique.html [ Crash P
 webkit.org/b/107377 storage/indexeddb/objectstore-basics.html [ Crash Pass ]
 webkit.org/b/107194 storage/indexeddb/objectstore-basics-workers.html [ Crash Failure ]
 
-webkit.org/b/108682 [ Debug ] media/video-controls-fullscreen-volume.html [ Crash ]
-webkit.org/b/108682 [ Debug ] media/video-controls-visible-exiting-fullscreen.html [ Crash ]
-webkit.org/b/108682 [ Debug ] media/video-src.html [ Crash ]
 
 webkit.org/b/108927 http/tests/xmlhttprequest/workers/xmlhttprequest-file-not-found.html [ Crash Pass ]
 
index eb0ba3a..4f3f66d 100644 (file)
@@ -1,3 +1,37 @@
+2013-02-10  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer] media/video-controls-fullscreen-volume.html crashes
+        https://bugs.webkit.org/show_bug.cgi?id=108682
+
+        Reviewed by Martin Robinson.
+
+        Clean up various signal handlers and avoid bad interaction between
+        the FullscreenVideoControllerGStreamer and its subclasses,
+        especially when the platform video window is created.
+
+        * platform/graphics/gstreamer/FullscreenVideoControllerGStreamer.cpp:
+        (WebCore::FullscreenVideoControllerGStreamer::enterFullscreen):
+        Initialize the window before connecting to the volume/mute
+        signals. This ensures that the signals won't ever interfere with
+        an inexisting window.
+        * platform/graphics/gstreamer/GStreamerGWorld.cpp:
+        (WebCore::GStreamerGWorld::~GStreamerGWorld): Remove GstBus
+        synchronous handler function.
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+        (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
+        Disconnect from volume/mute signals.
+        (WebCore::MediaPlayerPrivateGStreamerBase::setStreamVolumeElement):
+        Keep a trace of volume/mute signal handlers.
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
+        Various forward type declarations to avoid un-necessary header includes.
+        (MediaPlayerPrivateGStreamerBase):
+        * platform/graphics/gtk/FullscreenVideoControllerGtk.cpp:
+        (WebCore::FullscreenVideoControllerGtk::FullscreenVideoControllerGtk):
+        (WebCore::FullscreenVideoControllerGtk::volumeChanged): Bail out
+        if volume button hasn't been created yet.
+        (WebCore::FullscreenVideoControllerGtk::muteChanged): Ditto.
+
 2013-02-10  Andreas Kling  <akling@apple.com>
 
         RenderStyle should use copy-on-write inheritance for NinePieceImage.
index 7cf1a31..05e5316 100644 (file)
@@ -79,11 +79,11 @@ void FullscreenVideoControllerGStreamer::enterFullscreen()
     if (!m_gstreamerGWorld->enterFullscreen())
         return;
 
+    initializeWindow();
+
     GstElement* pipeline = m_gstreamerGWorld->pipeline();
     m_playerVolumeSignalHandler = g_signal_connect(pipeline, "notify::volume", G_CALLBACK(playerVolumeChangedCallback), this);
     m_playerMuteSignalHandler = g_signal_connect(pipeline, "notify::mute", G_CALLBACK(playerMuteChangedCallback), this);
-
-    initializeWindow();
 }
 
 void FullscreenVideoControllerGStreamer::exitFullscreen()
index b1dbb4b..1d2e7b9 100644 (file)
@@ -92,6 +92,14 @@ GStreamerGWorld::~GStreamerGWorld()
 {
     exitFullscreen();
 
+    GRefPtr<GstBus> bus = webkitGstPipelineGetBus(GST_PIPELINE(m_pipeline));
+    g_signal_handlers_disconnect_by_func(bus.get(), reinterpret_cast<gpointer>(gstGWorldSyncMessageCallback), this);
+#ifndef GST_API_VERSION_1
+    gst_bus_set_sync_handler(bus.get(), 0, this);
+#else
+    gst_bus_set_sync_handler(bus.get(), 0, this, 0);
+#endif
+
     m_pipeline = 0;
 }
 
index e17ec7a..9199c4f 100644 (file)
 #include <wtf/gobject/GOwnPtr.h>
 #include <wtf/text/CString.h>
 
+#ifdef GST_API_VERSION_1
+#include <gst/audio/streamvolume.h>
+#else
+#include <gst/interfaces/streamvolume.h>
+#endif
+
 // GstPlayFlags flags from playbin2. It is the policy of GStreamer to
 // not publicly expose element-specific enums. That's why this
 // GstPlayFlags enum has been copied here.
index 8bbd0e6..ea8e133 100644 (file)
 #include <gst/video/video.h>
 #include <wtf/text/CString.h>
 
+#ifdef GST_API_VERSION_1
+#include <gst/audio/streamvolume.h>
+#else
+#include <gst/interfaces/streamvolume.h>
+#endif
+
 GST_DEBUG_CATEGORY_STATIC(webkit_media_player_debug);
 #define GST_CAT_DEFAULT webkit_media_player_debug
 
@@ -121,6 +127,16 @@ MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
     if (m_volumeTimerHandler)
         g_source_remove(m_volumeTimerHandler);
 
+    if (m_volumeSignalHandler) {
+        g_signal_handler_disconnect(m_volumeElement.get(), m_volumeSignalHandler);
+        m_volumeSignalHandler = 0;
+    }
+
+    if (m_muteSignalHandler) {
+        g_signal_handler_disconnect(m_volumeElement.get(), m_muteSignalHandler);
+        m_muteSignalHandler = 0;
+    }
+
 #if USE(NATIVE_FULLSCREEN_VIDEO)
     if (m_fullscreenVideoController)
         exitFullscreen();
@@ -465,8 +481,8 @@ void MediaPlayerPrivateGStreamerBase::setStreamVolumeElement(GstStreamVolume* vo
 
     g_object_set(m_volumeElement.get(), "mute", m_player->muted(), "volume", m_player->volume(), NULL);
 
-    g_signal_connect(m_volumeElement.get(), "notify::volume", G_CALLBACK(mediaPlayerPrivateVolumeChangedCallback), this);
-    g_signal_connect(m_volumeElement.get(), "notify::mute", G_CALLBACK(mediaPlayerPrivateMuteChangedCallback), this);
+    m_volumeSignalHandler = g_signal_connect(m_volumeElement.get(), "notify::volume", G_CALLBACK(mediaPlayerPrivateVolumeChangedCallback), this);
+    m_muteSignalHandler = g_signal_connect(m_volumeElement.get(), "notify::mute", G_CALLBACK(mediaPlayerPrivateMuteChangedCallback), this);
 }
 
 unsigned MediaPlayerPrivateGStreamerBase::decodedFrameCount() const
index ace43ec..47a7330 100644 (file)
 #include "GRefPtrGStreamer.h"
 #include "MediaPlayerPrivate.h"
 
-#include <glib.h>
-#include <gst/gst.h>
 #include <wtf/Forward.h>
 
-#ifdef GST_API_VERSION_1
-#include <gst/audio/streamvolume.h>
-#else
-#include <gst/interfaces/streamvolume.h>
-#endif
-
-typedef struct _WebKitVideoSink WebKitVideoSink;
 typedef struct _GstBuffer GstBuffer;
-typedef struct _GstMessage GstMessage;
 typedef struct _GstElement GstElement;
+typedef struct _GstMessage GstMessage;
+typedef struct _GstStreamVolume GstStreamVolume;
+typedef struct _WebKitVideoSink WebKitVideoSink;
 
 namespace WebCore {
 
@@ -119,9 +112,11 @@ protected:
     RefPtr<GStreamerGWorld> m_gstGWorld;
     OwnPtr<FullscreenVideoControllerGStreamer> m_fullscreenVideoController;
 #endif
-    guint m_volumeTimerHandler;
-    guint m_muteTimerHandler;
-    guint m_repaintHandler;
+    unsigned long m_volumeTimerHandler;
+    unsigned long m_muteTimerHandler;
+    unsigned long m_repaintHandler;
+    unsigned long m_volumeSignalHandler;
+    unsigned long m_muteSignalHandler;
     GRefPtr<GstPad> m_videoSinkPad;
     mutable IntSize m_videoSize;
 };
index 46e81cc..e9d7f2f 100644 (file)
@@ -131,6 +131,7 @@ FullscreenVideoControllerGtk::FullscreenVideoControllerGtk(MediaPlayerPrivateGSt
     , m_seekLock(false)
     , m_window(0)
     , m_hudWindow(0)
+    , m_volumeButton(0)
     , m_keyPressSignalId(0)
     , m_destroySignalId(0)
     , m_isActiveSignalId(0)
@@ -332,6 +333,9 @@ void FullscreenVideoControllerGtk::playStateChanged()
 
 void FullscreenVideoControllerGtk::volumeChanged()
 {
+    if (!m_volumeButton)
+        return;
+
     g_signal_handler_block(m_volumeButton, m_volumeUpdateId);
     gtk_scale_button_set_value(GTK_SCALE_BUTTON(m_volumeButton), m_player->volume());
     g_signal_handler_unblock(m_volumeButton, m_volumeUpdateId);
@@ -339,6 +343,9 @@ void FullscreenVideoControllerGtk::volumeChanged()
 
 void FullscreenVideoControllerGtk::muteChanged()
 {
+    if (!m_volumeButton)
+        return;
+
     g_signal_handler_block(m_volumeButton, m_volumeUpdateId);
     gtk_scale_button_set_value(GTK_SCALE_BUTTON(m_volumeButton), m_player->muted() ? 0 : m_player->volume());
     g_signal_handler_unblock(m_volumeButton, m_volumeUpdateId);