Allow HTMLMediaElement.currentTime to be set before the transition to HAVE_METADATA

Source/core/html/HTMLMediaElement.cpp

Index: Source/core/html/HTMLMediaElement.cpp
diff --git a/Source/core/html/HTMLMediaElement.cpp b/Source/core/html/HTMLMediaElement.cpp
index 91ef6708e6f5e78594ef9b36ad751aec9955cc39..3ac7fb361e4a8d52d83bcdc7aa87ff4502ab0289 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -324,6 +324,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_duration(std::numeric_limits<double>::quiet_NaN())
     , m_lastTimeUpdateEventWallTime(0)
     , m_lastTimeUpdateEventMovieTime(std::numeric_limits<double>::max())
+    , m_defaultPlaybackStartPosition(0)
     , m_loadState(WaitingForSource)
     , m_deferredLoadState(NotDeferred)
     , m_deferredLoadTimer(this, &HTMLMediaElement::deferredLoadTimerFired)
@@ -1822,6 +1823,12 @@ void HTMLMediaElement::setReadyState(ReadyState state)
         if (isHTMLVideoElement())
             scheduleEvent(EventTypeNames::resize);
         scheduleEvent(EventTypeNames::loadedmetadata);
+
+        if (m_defaultPlaybackStartPosition > 0) {

[philipj_slow, 2014/09/02 13:03:31]

This looks like the correct place for this logic, but per spec there's a jumped
flag that will prevent the media fragment URI handling from kicking in. We need
a test for setting currentTime to e.g. 1 on a resource with #t=2, verifying that
we end up at currentTime 1 and not 2. I don't think the existing code will do
this. Fixing that should also allow you to remove m_fragmentStartTime.

[amogh.bihani, 2014/09/02 13:39:54]

Acknowledged.

+            seek(m_defaultPlaybackStartPosition, IGNORE_EXCEPTION);
+            m_defaultPlaybackStartPosition = 0;
+        }
+
         if (hasMediaControls())
             mediaControls()->reset();
         if (renderer())
@@ -1928,9 +1935,10 @@ void HTMLMediaElement::seek(double time, ExceptionState& exceptionState)
 
     // 4.8.10.9 Seeking
 
-    // 1 - If the media element's readyState is HAVE_NOTHING, then raise an InvalidStateError exception.
+    // 1 - If the media element's readyState is HAVE_NOTHING, then set the default playback start
+    // position to that time.
     if (m_readyState == HAVE_NOTHING) {
-        exceptionState.throwDOMException(InvalidStateError, "The element's readyState is HAVE_NOTHING.");
+        m_defaultPlaybackStartPosition = time;

[philipj_slow, 2014/09/02 13:03:31]

This should be moved to setCurrentTime, per "if the media element's readyState
is HAVE_NOTHING, then it must set the media element's default playback start
position to the new value"

[amogh.bihani, 2014/09/02 13:39:54]

Acknowledged.

         return;
     }
 
@@ -2075,7 +2083,7 @@ double HTMLMediaElement::currentTime() const
 #endif
 
     if (m_readyState == HAVE_NOTHING)
-        return 0;
+        return m_defaultPlaybackStartPosition;

[philipj_slow, 2014/09/02 13:03:31]

The spec says "return the media element's default playback start position,
unless that is zero" so "if (m_defaultPlaybackStartPosition) return
m_defaultPlaybackStartPosition;" would be appropriate. If that can only happen
while readyState is HAVE_NOTHING, you can assert that.

[amogh.bihani, 2014/09/02 13:39:54]

Ok, I'll do that. I thought that its value is set to zero in constructor, so if
it is not set to anything else, we will be returning zero.

 
     if (m_seeking) {
         WTF_LOG(Media, "HTMLMediaElement::currentTime - seeking, returning %f", m_lastSeekTime);
@@ -2102,6 +2110,12 @@ void HTMLMediaElement::setCurrentTime(double time, ExceptionState& exceptionStat
         exceptionState.throwDOMException(InvalidStateError, "The element is slaved to a MediaController.");
         return;
     }
+
+    if (m_networkState == NETWORK_EMPTY) {
+        exceptionState.throwDOMException(InvalidStateError, "There is no media attached.");

[philipj_slow, 2014/09/02 13:03:31]

This condition is not in the spec, why is it needed?

[amogh.bihani, 2014/09/02 13:39:54]

Oh Sorry! I got confused with this line "Test that seeking video with no 'src'
attribute throws an INVALID_STATE_ERR exception." in the video-seek-no-src
layout test and presumed that when there is not src attribute, then
InvalidStateError has to be thrown. I'll change this.

+        return;
+    }
+
     seek(time, exceptionState);
 }