Merge r247778 - [GStreamer] Don't crash with empty video src

https://bugs.webkit.org/show_bug.cgi?id=200081 LayoutTests/imported/w3c: Reviewed by Philippe Normand. * web-platform-tests/html/semantics/embedded-content/the-video-element/video_crash_empty_src.html: Added. Source/WebCore: When a <video> element is set to load empty or about:blank, a player is still created, but no pipeline is loaded. This patch fixes some assertion errors that manifested in that case. Reviewed by Philippe Normand. Test: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_crash_empty_src.html * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::loadFull): (WebCore::MediaPlayerPrivateGStreamer::platformDuration const): (WebCore::MediaPlayerPrivateGStreamer::paused const):
WebKit · Aug 4, 2019 · 4b0dfe3 · 4b0dfe3
1 parent e24680b
commit 4b0dfe3
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 1 deletion.
diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,12 @@
+2019-07-24  Alicia Boya García  <aboya@igalia.com>
+
+        [GStreamer] Don't crash with empty video src
+        https://bugs.webkit.org/show_bug.cgi?id=200081
+
+        Reviewed by Philippe Normand.
+
+        * web-platform-tests/html/semantics/embedded-content/the-video-element/video_crash_empty_src.html: Added.
+
 2019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
 
         appendChild should throw when inserting an ancestor of a template into its content adopted to another document

diff --git a/...atform-tests/html/semantics/embedded-content/the-video-element/video_crash_empty_src.html b/...atform-tests/html/semantics/embedded-content/the-video-element/video_crash_empty_src.html
@@ -0,0 +1,29 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+    <title>HTML5 Media Elements: An empty src should not crash the player.</title>
+    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
+    <link rel="author" title="Alicia Boya García" href="mailto:aboya@igalia.com"/>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+    function makeCrashTest(src) {
+        async_test(function (test) {
+            const video = document.createElement("video");
+            video.src = src;
+            video.controls = true;
+            document.body.appendChild(video);
+            test.step_timeout(test.step_func(() => {
+                document.body.removeChild(video);
+                test.done();
+            }), 0);
+        }, `src="${src}" does not crash.`);
+    }
+
+    makeCrashTest("about:blank");
+    makeCrashTest("");
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2019-07-24  Alicia Boya García  <aboya@igalia.com>
+
+        [GStreamer] Don't crash with empty video src
+        https://bugs.webkit.org/show_bug.cgi?id=200081
+
+        When a <video> element is set to load empty or about:blank, a player is still
+        created, but no pipeline is loaded. This patch fixes some assertion errors that
+        manifested in that case.
+
+        Reviewed by Philippe Normand.
+
+        Test: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_crash_empty_src.html
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::loadFull):
+        (WebCore::MediaPlayerPrivateGStreamer::platformDuration const):
+        (WebCore::MediaPlayerPrivateGStreamer::paused const):
+
 2019-07-19  Charlie Turner  <cturner@igalia.com>
 
         [GStreamer] Flush get_range calls during PAUSED->READY in WebKitWebSource

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
@@ -281,8 +281,10 @@ void MediaPlayerPrivateGStreamer::loadFull(const String& urlString, const String
     }
 
     URL url(URL(), urlString);
-    if (url.protocolIsAbout())
+    if (url.protocolIsAbout()) {
+        loadingFailed(MediaPlayer::FormatError, MediaPlayer::HaveNothing, true);
         return;
+    }
 
     if (!m_pipeline)
         createGSTPlayBin(url, pipelineName);
@@ -487,6 +489,9 @@ void MediaPlayerPrivateGStreamer::pause()
 
 MediaTime MediaPlayerPrivateGStreamer::platformDuration() const
 {
+    if (!m_pipeline)
+        return MediaTime::invalidTime();
+
     GST_TRACE_OBJECT(pipeline(), "errorOccured: %s, pipeline state: %s", boolForPrinting(m_errorOccured), gst_element_state_get_name(GST_STATE(m_pipeline.get())));
     if (m_errorOccured)
         return MediaTime::invalidTime();
@@ -655,6 +660,9 @@ void MediaPlayerPrivateGStreamer::updatePlaybackRate()
 
 bool MediaPlayerPrivateGStreamer::paused() const
 {
+    if (!m_pipeline)
+        return true;
+
     if (m_isEndReached) {
         GST_DEBUG_OBJECT(pipeline(), "Ignoring pause at EOS");
         return true;