Title: [219305] trunk
Revision
219305
Author
commit-qu...@webkit.org
Date
2017-07-10 12:49:29 -0700 (Mon, 10 Jul 2017)

Log Message

media element handle adding source immediately before src.
https://bugs.webkit.org/show_bug.cgi?id=174284
rdar://problem/33115439

Patch by Jeremy Jones <jere...@apple.com> on 2017-07-10
Reviewed by David Kilzer.

Source/WebCore:

Test: media/video-source-before-src.html

Adding a source causes a selectMediaResource block to be enqueued.
If dataLoadingPermitted prevents creating the m_player but sets the srcAttr, then
the enqueued selectMediaResource will be in a bad state, with a srcAttr but no m_player.

This fix prevents selectMediaResource from being called, if data loading is not permitted
when adding a source element, to match how it prevents player creation when setting srcAttr.

This fix also adds a debug assert to catch the problem earlier and adds an early return to
prevent the crash in release builds.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::selectMediaResource):
(WebCore::HTMLMediaElement::sourceWasAdded):

LayoutTests:

* media/video-source-before-src.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (219304 => 219305)


--- trunk/LayoutTests/ChangeLog	2017-07-10 19:46:01 UTC (rev 219304)
+++ trunk/LayoutTests/ChangeLog	2017-07-10 19:49:29 UTC (rev 219305)
@@ -1,3 +1,13 @@
+2017-07-10  Jeremy Jones  <jere...@apple.com>
+
+        media element handle adding source immediately before src.
+        https://bugs.webkit.org/show_bug.cgi?id=174284
+        rdar://problem/33115439
+
+        Reviewed by David Kilzer.
+
+        * media/video-source-before-src.html: Added.
+
 2017-07-10  Matt Lewis  <jlew...@apple.com>
 
         Fixed test expectations for http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html.

Added: trunk/LayoutTests/media/video-source-before-src.html (0 => 219305)


--- trunk/LayoutTests/media/video-source-before-src.html	                        (rev 0)
+++ trunk/LayoutTests/media/video-source-before-src.html	2017-07-10 19:49:29 UTC (rev 219305)
@@ -0,0 +1,28 @@
+<html>
+<head>
+<script>
+
+if (window.testRunner) {
+    internals.settings.setVideoPlaybackRequiresUserGesture(true);
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+window._onload_ = () => {
+    var video = document.getElementsByTagName("video")[0]
+    video.appendChild(document.createElement("source"))
+    video.src = ""
+
+    setTimeout(()=>{
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }, 1)
+}
+
+</script>
+</head>
+<body>
+Append source element before setting src attribute.<br>
+<video width=320 height=240></video>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (219304 => 219305)


--- trunk/Source/WebCore/ChangeLog	2017-07-10 19:46:01 UTC (rev 219304)
+++ trunk/Source/WebCore/ChangeLog	2017-07-10 19:49:29 UTC (rev 219305)
@@ -1,3 +1,27 @@
+2017-07-10  Jeremy Jones  <jere...@apple.com>
+
+        media element handle adding source immediately before src.
+        https://bugs.webkit.org/show_bug.cgi?id=174284
+        rdar://problem/33115439
+
+        Reviewed by David Kilzer.
+
+        Test: media/video-source-before-src.html
+
+        Adding a source causes a selectMediaResource block to be enqueued.
+        If dataLoadingPermitted prevents creating the m_player but sets the srcAttr, then
+        the enqueued selectMediaResource will be in a bad state, with a srcAttr but no m_player.
+
+        This fix prevents selectMediaResource from being called, if data loading is not permitted
+        when adding a source element, to match how it prevents player creation when setting srcAttr.
+
+        This fix also adds a debug assert to catch the problem earlier and adds an early return to
+        prevent the crash in release builds.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::selectMediaResource):
+        (WebCore::HTMLMediaElement::sourceWasAdded):
+
 2017-07-10  Megan Gardner  <megan_gard...@apple.com>
 
         Add location to NavigationActionData

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (219304 => 219305)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2017-07-10 19:46:01 UTC (rev 219304)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2017-07-10 19:49:29 UTC (rev 219305)
@@ -1323,6 +1323,11 @@
         } else if (hasAttributeWithoutSynchronization(srcAttr)) {
             //    Otherwise, if the media element has no assigned media provider object but has a src attribute, then let mode be attribute.
             mode = Attribute;
+            ASSERT(m_player);
+            if (!m_player) {
+                RELEASE_LOG_ERROR(Media, "HTMLMediaElement::selectMediaResource(%p) - has srcAttr but m_player is not created", this);
+                return;
+            }
         } else if (auto firstSource = childrenOfType<HTMLSourceElement>(*this).first()) {
             //    Otherwise, if the media element does not have an assigned media provider object and does not have a src attribute,
             //    but does have a source element child, then let mode be children and let candidate be the first such source element
@@ -4362,9 +4367,12 @@
     // 4.8.8 - If a source element is inserted as a child of a media element that has no src 
     // attribute and whose networkState has the value NETWORK_EMPTY, the user agent must invoke 
     // the media element's resource selection algorithm.
-    if (networkState() == HTMLMediaElement::NETWORK_EMPTY) {
+    if (m_networkState == NETWORK_EMPTY) {
         m_nextChildNodeToConsider = &source;
-        selectMediaResource();
+#if PLATFORM(IOS)
+        if (m_mediaSession->dataLoadingPermitted(*this))
+#endif
+            selectMediaResource();
         return;
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to