Changeset 68181 in webkit

Timestamp:

Sep 23, 2010 12:37:09 PM (14 years ago)

Author:

eric.carlson@apple.com

Message:

2010-09-23 Eric Carlson <​eric.carlson@apple.com>

Reviewed by Simon Fraser

'seeking' event should always fire
​https://bugs.webkit.org/show_bug.cgi?id=45694

Update seek algorithm to match current spec.

• html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::setReadyState): Don't need to fire 'seeking' event here. (WebCore::HTMLMediaElement::seek): Always fire 'seeking'. Update comments. (WebCore::HTMLMediaElement::finishSeek): Update comments. (WebCore::HTMLMediaElement::mediaPlayerTimeChanged): Ditto.

2010-09-23 Eric Carlson <​eric.carlson@apple.com>

Reviewed by Simon Fraser

'seeking' event should always fire
​https://bugs.webkit.org/show_bug.cgi?id=45694

• media/event-attributes-expected.txt: Update for changes.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/event-attributes-expected.txt (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/html/HTMLMediaElement.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-09-23  Eric Carlson  <eric.carlson@apple.com>
+
+        Reviewed by Simon Fraser
+
+        'seeking' event should always fire
+        https://bugs.webkit.org/show_bug.cgi?id=45694
+
+        * media/event-attributes-expected.txt: Update for changes.
+
 2010-09-23  Andrew Wilson  <atwilson@chromium.org>
 

trunk/LayoutTests/media/event-attributes-expected.txt

@@ -26 +26 @@
 *** seeking
 RUN(video.currentTime = 5.6)
+EVENT(seeking)
 EVENT(seeked)
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-09-23  Eric Carlson  <eric.carlson@apple.com>
+
+        Reviewed by Simon Fraser
+
+        'seeking' event should always fire
+        https://bugs.webkit.org/show_bug.cgi?id=45694
+
+        Update seek algorithm to match current spec.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::setReadyState): Don't need to fire 'seeking' event here.
+        (WebCore::HTMLMediaElement::seek): Always fire 'seeking'. Update comments.
+        (WebCore::HTMLMediaElement::finishSeek): Update comments.
+        (WebCore::HTMLMediaElement::mediaPlayerTimeChanged): Ditto.
+
 2010-09-23  Eric Uhrhane  <ericu@chromium.org>
 

trunk/WebCore/html/HTMLMediaElement.cpp

@@ -964 +964 @@
 
     if (m_seeking) {
-        // 4.8.10.10, step 8
+        // 4.8.10.9, step 11
         if (wasPotentiallyPlaying && m_readyState < HAVE_FUTURE_DATA)
             scheduleEvent(eventNames().waitingEvent);
 
-        // 4.8.10.10, step 9
-        if (m_readyState < HAVE_CURRENT_DATA) {
-            if (oldState >= HAVE_CURRENT_DATA)
-                scheduleEvent(eventNames().seekingEvent);
-        } else {
-            // 4.8.10.10 step 12 & 13.
+        // 4.8.10.10 step 14 & 15.
+        if (m_readyState >= HAVE_CURRENT_DATA)
             finishSeek();
-        }
-
     } else {
         if (wasPotentiallyPlaying && m_readyState < HAVE_FUTURE_DATA) {
-            // 4.8.10.9
+            // 4.8.10.8
             scheduleTimeupdateEvent(false);
             scheduleEvent(eventNames().waitingEvent);
@@ -1102 +1096 @@
     float now = currentTime();
 
+    // 2 - If the element's seeking IDL attribute is true, then another instance of this algorithm is
+    // already running. Abort that other instance of the algorithm without waiting for the step that
+    // it is running to complete.
+    // Nothing specific to be done here.
+
     // 3 - Set the seeking IDL attribute to true.
-    // The flag will be cleared when the engine tells is the time has actually changed
+    // The flag will be cleared when the engine tells us the time has actually changed.
     m_seeking = true;
 
-    // 4 - Queue a task to fire a simple event named timeupdate at the element.
-    scheduleTimeupdateEvent(false);
-
-    // 6 - If the new playback position is later than the end of the media resource, then let it be the end 
+    // 5 - If the new playback position is later than the end of the media resource, then let it be the end 
     // of the media resource instead.
     time = min(time, duration());
 
-    // 7 - If the new playback position is less than the earliest possible position, let it be that position instead.
+    // 6 - If the new playback position is less than the earliest possible position, let it be that position instead.
     float earliestTime = m_player->startTime();
     time = max(time, earliestTime);
 
-    // 8 - If the (possibly now changed) new playback position is not in one of the ranges given in the 
+    // 7 - If the (possibly now changed) new playback position is not in one of the ranges given in the 
     // seekable attribute, then let it be the position in one of the ranges given in the seekable attribute 
     // that is the nearest to the new playback position. ... If there are no ranges given in the seekable
     // attribute then set the seeking IDL attribute to false and abort these steps.
     RefPtr<TimeRanges> seekableRanges = seekable();
-    if (!seekableRanges->length() || time == now) {
+
+    // Short circuit seeking to the current time by just firing the events if no seek is required.
+    // Don't skip calling the media engine if we are in poster mode because a seek should always 
+    // cancel poster display.
+    bool noSeekRequired = !seekableRanges->length() || (time == now && displayMode() != Poster);
+    if (noSeekRequired) {
+        if (time == now) {
+            scheduleEvent(eventNames().seekingEvent);
+            scheduleTimeupdateEvent(false);
+            scheduleEvent(eventNames().seekedEvent);
+        }
         m_seeking = false;
         return;
@@ -1135 +1141 @@
     m_sentEndEvent = false;
 
-    // 9 - Set the current playback position to the given new playback position
+    // 8 - Set the current playback position to the given new playback position
     m_player->seek(time);
 
-    // 10-15 are handled, if necessary, when the engine signals a readystate change.
+    // 9 - Queue a task to fire a simple event named seeking at the element.
+    scheduleEvent(eventNames().seekingEvent);
+
+    // 10 - Queue a task to fire a simple event named timeupdate at the element.
+    scheduleTimeupdateEvent(false);
+
+    // 11-15 are handled, if necessary, when the engine signals a readystate change.
 }
 
@@ -1145 +1157 @@
     LOG(Media, "HTMLMediaElement::finishSeek");
 
-    // 4.8.10.10 Seeking step 12
+    // 4.8.10.9 Seeking step 14
     m_seeking = false;
 
-    // 4.8.10.10 Seeking step 13
+    // 4.8.10.9 Seeking step 15
     scheduleEvent(eventNames().seekedEvent);
 
@@ -1661 +1673 @@
     scheduleTimeupdateEvent(false);
 
-    // 4.8.10.10 step 12 & 13.  Needed if no ReadyState change is associated with the seek.
-    if (m_readyState >= HAVE_CURRENT_DATA && m_seeking)
+    // 4.8.10.9 step 14 & 15.  Needed if no ReadyState change is associated with the seek.
+    if (m_seeking && m_readyState >= HAVE_CURRENT_DATA)
         finishSeek();