load() does not reset the resource selection algorithm
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Sep 2011 18:49:56 +0000 (18:49 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Sep 2011 18:49:56 +0000 (18:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=64917

Reviewed by Darin Adler.

Source/WebCore:

Test: media/video-source-load.html

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::selectMediaResource): Reset m_nextChildNodeToConsider, update
    comments and rearrange logic to more closely match logic in spec.
(WebCore::HTMLMediaElement::noneSupported): Update comments.

LayoutTests:

* media/video-source-load-expected.txt: Added.
* media/video-source-load.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/media/video-source-load-expected.txt [new file with mode: 0644]
LayoutTests/media/video-source-load.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp

index c9b77da..25b5454 100644 (file)
@@ -1,3 +1,13 @@
+2011-09-06  Eric Carlson  <eric.carlson@apple.com>
+
+        load() does not reset the resource selection algorithm
+        https://bugs.webkit.org/show_bug.cgi?id=64917
+
+        Reviewed by Darin Adler.
+
+        * media/video-source-load-expected.txt: Added.
+        * media/video-source-load.html: Added.
+
 2011-09-06  Abhishek Arya  <inferno@chromium.org>
 
         Style not propagated to anonymous boxes and anonymous
diff --git a/LayoutTests/media/video-source-load-expected.txt b/LayoutTests/media/video-source-load-expected.txt
new file mode 100644 (file)
index 0000000..450848c
--- /dev/null
@@ -0,0 +1,17 @@
+Test that the resource selection algorithm is restarted when load() is called, and that all <source> elements are reconsidered.
+
++++ Test initial networkState.
+EXPECTED (video.networkState == '0') OK
+
++++ Add <source> elements to trigger loading, test networkState.
+EXPECTED (video.networkState == '3') OK
+EVENT(error)
+EVENT(canplaythrough)
+EXPECTED (stripExtension(relativeURL(video.currentSrc)) == 'content/test') OK
+
++++ Calling load().
+EVENT(error)
+EVENT(canplaythrough)
+EXPECTED (stripExtension(relativeURL(video.currentSrc)) == 'content/test') OK
+END OF TEST
+
diff --git a/LayoutTests/media/video-source-load.html b/LayoutTests/media/video-source-load.html
new file mode 100644 (file)
index 0000000..6bf3ba5
--- /dev/null
@@ -0,0 +1,61 @@
+<!doctype html>
+<html>
+    <head>
+        <title>load() and the &lt;source&gt; element</title>
+        <script src=video-test.js></script>
+        <script src=media-file.js></script>
+        <script>
+            var sources = [];
+            var count = 0;
+
+            function canplaythrough() 
+            {
+                testExpected("stripExtension(relativeURL(video.currentSrc))", relativeURL(stripExtension(sources[1])));
+                ++count;
+                switch (count) 
+                {
+                    case 1:
+                        consoleWrite("<br>+++ Calling load().");
+                        video.load();
+                        break;
+                    case 2:
+                        endTest();
+                    return;
+                }
+            }
+
+            function addSource(type, name)
+            {
+                var source = document.createElement('source');
+                source.src = findMediaFile(type, name);
+                sources.push(source.src);
+                source.type = mimeTypeForExtension(source.src.split('.')[1]);
+                video.appendChild(source);
+            }
+
+            function setup()
+            {
+                video = mediaElement = document.getElementsByTagName('video')[0];
+
+                consoleWrite("+++ Test initial networkState.");
+                testExpected("video.networkState", HTMLMediaElement.prototype.NETWORK_EMPTY, "==");
+
+                // Add an invalid url to the first source so we test getting an error event
+                // each time resource selection runs.
+                addSource("video", "content/bogus");
+                addSource("video", "content/test");
+                addSource("audio", "content/test");
+
+                consoleWrite("<br>+++ Add &lt;source&gt; elements to trigger loading, test networkState.");
+                testExpected("video.networkState", HTMLMediaElement.prototype.NETWORK_NO_SOURCE, "==");
+
+                waitForEvent("canplaythrough", canplaythrough);
+                waitForEvent("error");
+            }
+        </script>
+    </head>
+    <body onload="setup()">
+        <video controls width=300 ></video>
+        <p>Test that the resource selection algorithm is restarted when load() is called, and that all &lt;source&gt; elements are reconsidered.</p>
+    </body>
+</html>
index 2b411b2..6a28bd1 100644 (file)
@@ -1,3 +1,17 @@
+2011-09-06  Eric Carlson  <eric.carlson@apple.com>
+
+        load() does not reset the resource selection algorithm
+        https://bugs.webkit.org/show_bug.cgi?id=64917
+
+        Reviewed by Darin Adler.
+
+        Test: media/video-source-load.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::selectMediaResource): Reset m_nextChildNodeToConsider, update
+            comments and rearrange logic to more closely match logic in spec.
+        (WebCore::HTMLMediaElement::noneSupported): Update comments.
+
 2011-09-06  Abhishek Arya  <inferno@chromium.org>
 
         Style not propagated to anonymous boxes and anonymous
index 43210b7..abb19f8 100644 (file)
@@ -635,29 +635,34 @@ void HTMLMediaElement::selectMediaResource()
     LOG(Media, "HTMLMediaElement::selectMediaResource");
 
     enum Mode { attribute, children };
-    Mode mode = attribute;
 
-    // 3 - ... the media element has neither a src attribute ...
+    // 3 - If the media element has a src attribute, then let mode be attribute.
+    Mode mode = attribute;
     if (!fastHasAttribute(srcAttr)) {
-        // ... nor a source element child: ...
         Node* node;
         for (node = firstChild(); node; node = node->nextSibling()) {
             if (node->hasTagName(sourceTag))
                 break;
         }
 
-        if (!node) {
+        // Otherwise, if the media element does not have a src attribute but has a source 
+        // element child, then let mode be children and let candidate be the first such 
+        // source element child in tree order.
+        if (node) {
+            mode = children;
+            m_nextChildNodeToConsider = 0;
+            m_currentSourceNode = 0;
+        } else {
+            // Otherwise the media element has neither a src attribute nor a source element 
+            // child: set the networkState to NETWORK_EMPTY, and abort these steps; the 
+            // synchronous section ends.
             m_loadState = WaitingForSource;
             setShouldDelayLoadEvent(false);
-
-            // ... set the networkState to NETWORK_EMPTY, and abort these steps
             m_networkState = NETWORK_EMPTY;
 
             LOG(Media, "HTMLMediaElement::selectMediaResource, nothing to load");
             return;
         }
-
-        mode = children;
     }
 
     // 4 - Set the media element's delaying-the-load-event flag to true (this delays the load event), 
@@ -665,7 +670,7 @@ void HTMLMediaElement::selectMediaResource()
     setShouldDelayLoadEvent(true);
     m_networkState = NETWORK_LOADING;
 
-    // 5
+    // 5 - Queue a task to fire a simple event named loadstart at the media element.
     scheduleEvent(eventNames().loadstartEvent);
 
     // 6 - If mode is attribute, then run these substeps
@@ -694,7 +699,6 @@ void HTMLMediaElement::selectMediaResource()
     }
 
     // Otherwise, the source elements will be used
-    m_currentSourceNode = 0;
     loadNextSourceChild();
 }
 
@@ -895,23 +899,27 @@ void HTMLMediaElement::noneSupported()
     m_loadState = WaitingForSource;
     m_currentSourceNode = 0;
 
-    // 5 - Reaching this step indicates that either the URL failed to resolve, or the media
-    // resource failed to load. Set the error attribute to a new MediaError object whose
-    // code attribute is set to MEDIA_ERR_SRC_NOT_SUPPORTED.
+    // 4.8.10.5 
+    // 6 - Reaching this step indicates that the media resource failed to load or that the given 
+    // URL could not be resolved. In one atomic operation, run the following steps:
+
+    // 6.1 - Set the error attribute to a new MediaError object whose code attribute is set to
+    // MEDIA_ERR_SRC_NOT_SUPPORTED.
     m_error = MediaError::create(MediaError::MEDIA_ERR_SRC_NOT_SUPPORTED);
 
-    // 6 - Set the element's networkState attribute to the NETWORK_NO_SOURCE value.
+    // 6.2 - Forget the media element's media-resource-specific text tracks.
+
+    // 6.3 - Set the element's networkState attribute to the NETWORK_NO_SOURCE value.
     m_networkState = NETWORK_NO_SOURCE;
 
-    // 7 - Queue a task to fire a progress event called error at the media element, in
-    // the context of the fetching process that was used to try to obtain the media
-    // resource in the resource fetch algorithm.
+    // 7 - Queue a task to fire a simple event named error at the media element.
     scheduleEvent(eventNames().errorEvent);
 
     // 8 - Set the element's delaying-the-load-event flag to false. This stops delaying the load event.
     setShouldDelayLoadEvent(false);
 
-    // 9 -Abort these steps. Until the load() method is invoked, the element won't attempt to load another resource.
+    // 9 - Abort these steps. Until the load() method is invoked or the src attribute is changed, 
+    // the element won't attempt to load another resource.
 
     updateDisplayState();