Seeking media fragment URI before loadeddata event

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 fca716c65c1b7730f2006a041e52c31bc379a1da..625cd0585ec2f3356f9b08a73e073d91bf9490ce 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -334,7 +334,6 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_cachedTime(MediaPlayer::invalidTime())
     , m_cachedTimeWallClockUpdateTime(0)
     , m_minimumWallClockTimeToCacheMediaTime(0)
-    , m_fragmentStartTime(MediaPlayer::invalidTime())
     , m_fragmentEndTime(MediaPlayer::invalidTime())
     , m_pendingActionFlags(0)
     , m_userGestureRequiredForPlay(false)
@@ -1813,10 +1812,11 @@ void HTMLMediaElement::setReadyState(ReadyState state)
     if (m_readyState >= HAVE_METADATA && oldState < HAVE_METADATA) {
         createPlaceholderTracksIfNecessary();
 
-        prepareMediaFragmentURI();
-
         selectInitialTracksIfNecessary();
 
+        MediaFragmentURIParser fragmentParser(m_currentSrc);
+        m_fragmentEndTime = fragmentParser.endTime();
+
         m_duration = duration();
         scheduleEvent(EventTypeNames::durationchange);
 
@@ -1824,11 +1824,27 @@ void HTMLMediaElement::setReadyState(ReadyState state)
             scheduleEvent(EventTypeNames::resize);
         scheduleEvent(EventTypeNames::loadedmetadata);
 
+        bool jumped = false;
         if (m_defaultPlaybackStartPosition > 0) {
             seek(m_defaultPlaybackStartPosition);
+            jumped = true;
             m_defaultPlaybackStartPosition = 0;
         }
 
+        if (!jumped) {
+            double start = fragmentParser.startTime();
+            if (start != MediaPlayer::invalidTime() && start > 0) {
+                m_sentEndEvent = false;
+                UseCounter::count(document(), UseCounter::HTMLMediaElementSeekToFragmentStart);
+                seek(start);
+                jumped = true;
+            }
+            // FIXME: Add support for selecting tracks by ID with the Media Fragments track dimension.
+        }
+
+        if (jumped && m_mediaController && m_mediaController->currentTime() < currentTime())

[philipj_slow, 2014/09/10 12:25:48]

This bit would be clearer if the initial playback position were made explicit.
I've filed a spec bug pointing out that it's only used locally in these steps:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26771

I suggest a double initialPlaybackPosition locally in these steps. The structure
should be something like:

if (m_mediaController) {
    if (jumped && initialPlaybackPosition > m_mediaController->currentTime())
        m_mediaController->setCurrentTime(initialPlaybackPosition);
    else
        seek(m_mediaController->currentTime());
}

Writing that out, it's clear that this step is trying to get the controller and
media element in sync, which is relevant to the bug you filed. Maybe there is no
bug in the spec?

[amogh.bihani, 2014/09/11 13:35:59]

I guess then it would be going away from media element load algorithm (step
8)... right?
Ah yes! It works as intended after this. :)

[philipj_slow, 2014/09/11 14:38:26]

Yes, AFAICT that step doesn't amount to anything that couldn't be kept local.
Should I close the spec bug then?

[amogh.bihani, 2014/09/12 11:02:23]

Yes sure :)

+            m_mediaController->setCurrentTime(currentTime());
+
         if (hasMediaControls())
             mediaControls()->reset();
         if (renderer())
@@ -1842,7 +1858,6 @@ void HTMLMediaElement::setReadyState(ReadyState state)
         shouldUpdateDisplayState = true;
         scheduleEvent(EventTypeNames::loadeddata);
         setShouldDelayLoadEvent(false);
-        applyMediaFragmentURI();
     }
 
     bool isPotentiallyPlaying = potentiallyPlaying();
@@ -3869,42 +3884,6 @@ bool HTMLMediaElement::isBlockedOnMediaController() const
     return false;
 }
 
-void HTMLMediaElement::prepareMediaFragmentURI()
-{
-    MediaFragmentURIParser fragmentParser(m_currentSrc);
-    double dur = duration();
-
-    double start = fragmentParser.startTime();
-    if (start != MediaFragmentURIParser::invalidTimeValue() && start > 0) {
-        m_fragmentStartTime = start;
-        if (m_fragmentStartTime > dur)
-            m_fragmentStartTime = dur;
-    } else
-        m_fragmentStartTime = MediaPlayer::invalidTime();
-
-    double end = fragmentParser.endTime();
-    if (end != MediaFragmentURIParser::invalidTimeValue() && end > 0 && end > m_fragmentStartTime) {
-        m_fragmentEndTime = end;
-        if (m_fragmentEndTime > dur)
-            m_fragmentEndTime = dur;
-    } else
-        m_fragmentEndTime = MediaPlayer::invalidTime();
-
-    // FIXME: Add support for selecting tracks by ID with the Media Fragments track dimension.
-
-    if (m_fragmentStartTime != MediaPlayer::invalidTime() && m_readyState < HAVE_FUTURE_DATA)
-        prepareToPlay();
-}
-
-void HTMLMediaElement::applyMediaFragmentURI()
-{
-    if (m_fragmentStartTime != MediaPlayer::invalidTime()) {
-        m_sentEndEvent = false;
-        UseCounter::count(document(), UseCounter::HTMLMediaElementSeekToFragmentStart);
-        seek(m_fragmentStartTime);
-    }
-}
-
 WebMediaPlayer::CORSMode HTMLMediaElement::corsMode() const
 {
     const AtomicString& crossOriginMode = fastGetAttribute(crossoriginAttr);