Title: [213798] releases/WebKitGTK/webkit-2.16
- Revision
- 213798
- Author
- [email protected]
- Date
- 2017-03-13 02:49:20 -0700 (Mon, 13 Mar 2017)
Log Message
Merge r213224 - [GTK] fast/canvas/canvas-createPattern-video-loading.html makes its subsequent test timeout
https://bugs.webkit.org/show_bug.cgi?id=169019
Reviewed by Xabier Rodriguez-Calvar.
Source/WebCore:
The timeout happens normally when the media player is deleted and the pipeline state is set to NULL. The call to
gst_element_set_state() never returns because of different deadlocks with the video sink. Sometimes the deadlock
happens with the sample mutex used by VideoRenderRequestScheduler. VideoRenderRequestScheduler::requestRender()
calls webkitVideoSinkRepaintRequested() with the lock held, that ends up calling
MediaPlayerPrivateGStreamerBase::triggerRepaint(). When rendering can't be accelerated the draw timer is
scheduled and triggerRepaint blocks until the timer is fired. If the media player is destroyed before the timer
is fired, when setting the pipeline state to NULL, other VideoRenderRequestScheduler methods can be called, like
stop() that tries to get the sample mutex that is still held by requestRender(). So, first we need to make
sure that requestRender() releases the lock before calling webkitVideoSinkRepaintRequested(). But that's not
enough, we also need to ensure that the pipeline is set to NULL state after everyting has been properly
stopped. This is currently done in ~MediaPlayerPrivateGStreamer that happens before
~MediaPlayerPrivateGStreamerBase, so gst_element_set_state() is hanging before allowing the
MediaPlayerPrivateGStreamerBase to be cleaned up. We should move the call to the end of
~MediaPlayerPrivateGStreamerBase and ensure the draw timer and mutex are properly cleaned up before.
Fixes: fast/canvas/canvas-createPattern-video-loading.html
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer): Do not reset pipeline here.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase): Stop the draw mutex and notify the
lock to ensure we unblock. Do the pipeline reset at the end.
* platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
(VideoRenderRequestScheduler::requestRender): Release the mutex lock before calling webkitVideoSinkRepaintRequested().
LayoutTests:
Unskip tests previously skipped because of this timeout.
* platform/gtk/TestExpectations:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (213797 => 213798)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-03-13 09:48:07 UTC (rev 213797)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-03-13 09:49:20 UTC (rev 213798)
@@ -1,3 +1,14 @@
+2017-03-01 Carlos Garcia Campos <[email protected]>
+
+ [GTK] fast/canvas/canvas-createPattern-video-loading.html makes its subsequent test timeout
+ https://bugs.webkit.org/show_bug.cgi?id=169019
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ Unskip tests previously skipped because of this timeout.
+
+ * platform/gtk/TestExpectations:
+
2017-02-27 Chris Dumez <[email protected]>
LayoutTest fast/events/currentTarget-gc-crash.html is a flaky failure
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (213797 => 213798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-03-13 09:48:07 UTC (rev 213797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-03-13 09:49:20 UTC (rev 213798)
@@ -1,3 +1,35 @@
+2017-03-01 Carlos Garcia Campos <[email protected]>
+
+ [GTK] fast/canvas/canvas-createPattern-video-loading.html makes its subsequent test timeout
+ https://bugs.webkit.org/show_bug.cgi?id=169019
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ The timeout happens normally when the media player is deleted and the pipeline state is set to NULL. The call to
+ gst_element_set_state() never returns because of different deadlocks with the video sink. Sometimes the deadlock
+ happens with the sample mutex used by VideoRenderRequestScheduler. VideoRenderRequestScheduler::requestRender()
+ calls webkitVideoSinkRepaintRequested() with the lock held, that ends up calling
+ MediaPlayerPrivateGStreamerBase::triggerRepaint(). When rendering can't be accelerated the draw timer is
+ scheduled and triggerRepaint blocks until the timer is fired. If the media player is destroyed before the timer
+ is fired, when setting the pipeline state to NULL, other VideoRenderRequestScheduler methods can be called, like
+ stop() that tries to get the sample mutex that is still held by requestRender(). So, first we need to make
+ sure that requestRender() releases the lock before calling webkitVideoSinkRepaintRequested(). But that's not
+ enough, we also need to ensure that the pipeline is set to NULL state after everyting has been properly
+ stopped. This is currently done in ~MediaPlayerPrivateGStreamer that happens before
+ ~MediaPlayerPrivateGStreamerBase, so gst_element_set_state() is hanging before allowing the
+ MediaPlayerPrivateGStreamerBase to be cleaned up. We should move the call to the end of
+ ~MediaPlayerPrivateGStreamerBase and ensure the draw timer and mutex are properly cleaned up before.
+
+ Fixes: fast/canvas/canvas-createPattern-video-loading.html
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+ (WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer): Do not reset pipeline here.
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+ (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase): Stop the draw mutex and notify the
+ lock to ensure we unblock. Do the pipeline reset at the end.
+ * platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
+ (VideoRenderRequestScheduler::requestRender): Release the mutex lock before calling webkitVideoSinkRepaintRequested().
+
2017-03-01 Tomas Popela <[email protected]>
[Cairo] Incorrectly determining height in GraphicsContext::roundToDevicePixels()
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (213797 => 213798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2017-03-13 09:48:07 UTC (rev 213797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2017-03-13 09:49:20 UTC (rev 213798)
@@ -222,7 +222,6 @@
gst_bus_remove_signal_watch(bus.get());
gst_bus_set_sync_handler(bus.get(), nullptr, nullptr, nullptr);
g_signal_handlers_disconnect_matched(m_pipeline.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
- gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
}
}
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp (213797 => 213798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2017-03-13 09:48:07 UTC (rev 213797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp 2017-03-13 09:49:20 UTC (rev 213798)
@@ -225,6 +225,14 @@
#endif
m_notifier.cancelPendingNotifications();
+#if USE(GSTREAMER_GL) || USE(COORDINATED_GRAPHICS_THREADED)
+ m_drawTimer.stop();
+ {
+ LockHolder locker(m_drawMutex);
+ m_drawCondition.notifyOne();
+ }
+#endif
+
if (m_videoSink) {
g_signal_handlers_disconnect_matched(m_videoSink.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
#if USE(GSTREAMER_GL)
@@ -250,6 +258,9 @@
#if ENABLE(LEGACY_ENCRYPTED_MEDIA)
m_cdmSession = nullptr;
#endif
+
+ if (m_pipeline)
+ gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
}
void MediaPlayerPrivateGStreamerBase::setPipeline(GstElement* pipeline)
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp (213797 => 213798)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2017-03-13 09:48:07 UTC (rev 213797)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp 2017-03-13 09:49:20 UTC (rev 213798)
@@ -113,9 +113,10 @@
return false;
#if USE(COORDINATED_GRAPHICS_THREADED)
- if (LIKELY(GST_IS_SAMPLE(m_sample.get())))
- webkitVideoSinkRepaintRequested(sink, m_sample.get());
- m_sample = nullptr;
+ auto sample = WTFMove(m_sample);
+ locker.unlockEarly();
+ if (LIKELY(GST_IS_SAMPLE(sample.get())))
+ webkitVideoSinkRepaintRequested(sink, sample.get());
#else
m_sink = sink;
m_timer.startOneShot(0);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
