2009-08-26 Eric Carlson <eric.carlson@apple.com>
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Aug 2009 00:47:41 +0000 (00:47 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 27 Aug 2009 00:47:41 +0000 (00:47 +0000)
        Reviewed by Simon Fraser.

        HTMLMediaElement sometimes loses events
        https://bugs.webkit.org/show_bug.cgi?id=28762
        <rdar://problem/7172437>

        A media element triggers the resource selection algorithm if 
        play() is called when the networkState attribute is NETWORK_EMPTY, but
        it also immediately queues 'play' and either 'waiting' or 'playing' events.
        One of the steps in preparing to load is to flush pending events, so those 
        events were lost if the load timer fired quickly enough. Fixed by deleting
        pending events before scheduling the load timer.

        Test: media/audio-play-event.html

        * html/HTMLMediaElement.cpp:
        (WebCore::HTMLMediaElement::scheduleLoad):
            Do nothing if the load timer is already scheduled. Call prepareForLoad so queue of
            pending events is flushed immediately.
        (WebCore::HTMLMediaElement::scheduleNextSourceChild):
            New, start the load timer without flushing pending events.
        (WebCore::HTMLMediaElement::load):
            Call prepareForLoad();
        (WebCore::HTMLMediaElement::prepareForLoad):
            New, load setup code moved from loadInternal.
        (WebCore::HTMLMediaElement::loadInternal):
            Moved some setup code to prepareForLoad so it can be invoked immediately before
            arming the load timer.
        (WebCore::HTMLMediaElement::setNetworkState):
            Call scheduleNextSourceChild instead of scheduleLoad as the later now clears
            pending events.
        * html/HTMLMediaElement.h:
            Declare scheduleNextSourceChild.

2009-08-26  Eric Carlson  <eric.carlson@apple.com>

        Reviewed by Simon Fraser.

        HTMLMediaElement sometimes loses events
        https://bugs.webkit.org/show_bug.cgi?id=28762
        <rdar://problem/7172437>

        * media/audio-play-event.html:
        * media/audio-play-event-expected.txt:
             Added.

        * media/video-play-empty-events-expected.txt:
            Updated results for previously missed events.

        * media/video-src-remove.html:
        * media/video-timeupdate-during-playback-expected.txt:
            Restructured to make results less timing dependent.

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

LayoutTests/ChangeLog
LayoutTests/media/audio-play-event-expected.txt [new file with mode: 0644]
LayoutTests/media/audio-play-event.html [new file with mode: 0644]
LayoutTests/media/video-play-empty-events-expected.txt
LayoutTests/media/video-src-remove.html
LayoutTests/media/video-timeupdate-during-playback-expected.txt
WebCore/ChangeLog
WebCore/html/HTMLMediaElement.cpp
WebCore/html/HTMLMediaElement.h

index e241622..b499719 100644 (file)
@@ -1,3 +1,22 @@
+2009-08-26  Eric Carlson  <eric.carlson@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        HTMLMediaElement sometimes loses events
+        https://bugs.webkit.org/show_bug.cgi?id=28762
+        <rdar://problem/7172437>
+
+        * media/audio-play-event.html:
+        * media/audio-play-event-expected.txt:
+             Added.
+
+        * media/video-play-empty-events-expected.txt:
+            Updated results for previously missed events.
+
+        * media/video-src-remove.html:
+        * media/video-timeupdate-during-playback-expected.txt:
+            Restructured to make results less timing dependent.
+
 2009-08-26  Dave Hyatt  <hyatt@apple.com>
 
         Add layout test for relpositioned inline regression.
diff --git a/LayoutTests/media/audio-play-event-expected.txt b/LayoutTests/media/audio-play-event-expected.txt
new file mode 100644 (file)
index 0000000..e394f24
--- /dev/null
@@ -0,0 +1,12 @@
+Test that a 'play' event listener is triggered when fired by a new audio element.
+
+RUN(mediaElement.src = 'content/silence.mpg')
+RUN(mediaElement.volume = 1)
+RUN(mediaElement.play())
+EVENT(play)
+EVENT(loadedmetadata)
+EVENT(canplay)
+EVENT(canplaythrough)
+EVENT(playing)
+END OF TEST
+
diff --git a/LayoutTests/media/audio-play-event.html b/LayoutTests/media/audio-play-event.html
new file mode 100644 (file)
index 0000000..48f8e6e
--- /dev/null
@@ -0,0 +1,27 @@
+<html>
+    <head>
+        <title>'play' event</title>
+        <script src=video-test.js></script>
+
+        <script>
+            function start()
+            {
+                mediaElement = new Audio();
+                waitForEvent('error');
+                waitForEvent('loadedmetadata');
+                waitForEvent('canplay');
+                waitForEvent('canplaythrough');
+                waitForEvent('play');
+                waitForEvent('playing', function() { endTest(); });
+                run("mediaElement.src = 'content/silence.mpg'");
+                run("mediaElement.volume = 1");
+                run("mediaElement.play()");
+            }    
+        </script>
+    </head>
+
+    <body onload="start()">
+    <p>Test that a 'play' event listener is triggered when fired by a new audio element.</p>
+    </body>
+</html>
+
index 3507178..083ae35 100644 (file)
@@ -3,6 +3,8 @@ Test that play() from EMPTY network state triggers load() and async play event.
 EXPECTED (video.networkState == '0') OK
 RUN(video.play())
 SCRIPT DONE
+EVENT(play)
+EVENT(waiting)
 EVENT(loadstart)
 EVENT(durationchange)
 EVENT(loadedmetadata)
index 9fce5b8..23ff18f 100644 (file)
@@ -2,7 +2,7 @@
 <body>
     <script src=video-test.js></script>
 
-    <video src=content/silence.mpg controls>
+    <video src=content/silence.mpg controls onloadedmetadata="loadedmetadata()" >
         <source src=content/test.mp4>
     </video>
 
@@ -22,6 +22,8 @@
 
         function loadedmetadata()
         {
+            consoleWrite("EVENT(loadedmetadata)");
+
             ++loadCount;
             if (loadCount == 1)
             {
@@ -34,6 +36,7 @@
                 endTest();
             }
             consoleWrite("");
+            setTimeout(someTimeLater, 100) ;
         }
         
         function someTimeLater()
@@ -44,8 +47,6 @@
         }
 
         consoleWrite("");
-        waitForEvent('loadedmetadata', loadedmetadata);
-        setTimeout(someTimeLater, 400) ;
     </script>
 
 </body>
index 671b160..cdf95f4 100644 (file)
@@ -2,6 +2,8 @@ Test 'timeupdate' events are posted while playing but not while paused.
 
 RUN(video.play())
 
+EVENT(play)
+EVENT(waiting)
 EVENT(loadstart)
 EVENT(durationchange)
 EVENT(loadedmetadata)
index 1c9e9af..a1120e9 100644 (file)
@@ -1,3 +1,39 @@
+2009-08-26  Eric Carlson  <eric.carlson@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        HTMLMediaElement sometimes loses events
+        https://bugs.webkit.org/show_bug.cgi?id=28762
+        <rdar://problem/7172437>
+
+        A media element triggers the resource selection algorithm if 
+        play() is called when the networkState attribute is NETWORK_EMPTY, but
+        it also immediately queues 'play' and either 'waiting' or 'playing' events.
+        One of the steps in preparing to load is to flush pending events, so those 
+        events were lost if the load timer fired quickly enough. Fixed by deleting
+        pending events before scheduling the load timer.
+
+        Test: media/audio-play-event.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::scheduleLoad):
+            Do nothing if the load timer is already scheduled. Call prepareForLoad so queue of 
+            pending events is flushed immediately.
+        (WebCore::HTMLMediaElement::scheduleNextSourceChild):
+            New, start the load timer without flushing pending events.
+        (WebCore::HTMLMediaElement::load):
+            Call prepareForLoad();
+        (WebCore::HTMLMediaElement::prepareForLoad):
+            New, load setup code moved from loadInternal.
+        (WebCore::HTMLMediaElement::loadInternal):
+            Moved some setup code to prepareForLoad so it can be invoked immediately before
+            arming the load timer.
+        (WebCore::HTMLMediaElement::setNetworkState):
+            Call scheduleNextSourceChild instead of scheduleLoad as the later now clears
+            pending events.
+        * html/HTMLMediaElement.h:
+            Declare scheduleNextSourceChild.
+
 2009-08-26  Peter Kasting  <pkasting@google.com>
 
         Reviewed by Eric Seidel.
index 335989f..2143918 100644 (file)
@@ -272,6 +272,15 @@ void HTMLMediaElement::recalcStyle(StyleChange change)
 
 void HTMLMediaElement::scheduleLoad()
 {
+    if (m_loadTimer.isActive())
+        return;
+    prepareForLoad();
+    m_loadTimer.startOneShot(0);
+}
+
+void HTMLMediaElement::scheduleNextSourceChild()
+{
+    // Schedule the timer to try the next <source> element WITHOUT resetting state ala prepareForLoad.
     m_loadTimer.startOneShot(0);
 }
 
@@ -411,17 +420,15 @@ void HTMLMediaElement::load(ExceptionCode& ec)
 {
     if (m_restrictions & RequireUserGestureForLoadRestriction && !processingUserGesture())
         ec = INVALID_STATE_ERR;
-    else
+    else {
+        prepareForLoad();
         loadInternal();
+    }
 }
 
-void HTMLMediaElement::loadInternal()
+void HTMLMediaElement::prepareForLoad()
 {
-    // 1 - If the load() method for this element is already being invoked, then abort these steps.
-    if (m_processingLoad)
-        return;
-    m_processingLoad = true;
-    
+    // Perform the cleanup required for the resource load algorithm to run.
     stopPeriodicTimers();
     m_loadTimer.stop();
     m_sentStalledEvent = false;
@@ -433,6 +440,16 @@ void HTMLMediaElement::loadInternal()
     // 3 - If there are any tasks from the media element's media element event task source in 
     // one of the task queues, then remove those tasks.
     cancelPendingEventsAndCallbacks();
+}
+
+void HTMLMediaElement::loadInternal()
+{
+    // 1 - If the load() method for this element is already being invoked, then abort these steps.
+    if (m_processingLoad)
+        return;
+    m_processingLoad = true;
+    
+    // Steps 2 and 3 were done in prepareForLoad()
     
     // 4 - If the media element's networkState is set to NETWORK_LOADING or NETWORK_IDLE, set
     // the error attribute to a new MediaError object whose code attribute is set to
@@ -689,7 +706,7 @@ void HTMLMediaElement::setNetworkState(MediaPlayer::NetworkState state)
         if (m_readyState < HAVE_METADATA && m_loadState == LoadingFromSourceElement) {
             m_currentSourceNode->scheduleErrorEvent();
             if (havePotentialSourceChild())
-                scheduleLoad();
+                scheduleNextSourceChild();
             return;
         }
 
index dd7ed4e..3aeb653 100644 (file)
@@ -202,6 +202,7 @@ private:
     // loading
     void selectMediaResource();
     void loadResource(const KURL&, ContentType&);
+    void scheduleNextSourceChild();
     void loadNextSourceChild();
     void userCancelledLoad();
     bool havePotentialSourceChild();
@@ -217,6 +218,8 @@ private:
     void loadInternal();
     void playInternal();
     void pauseInternal();
+
+    void prepareForLoad();
     
     bool processingUserGesture() const;
     bool processingMediaPlayerCallback() const { return m_processingMediaPlayerCallback > 0; }