Refactor resource load and resource selection algorithms as per spec

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

Index: third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
index 854d8426503f2c81853c17629b5fe71399e3f62f..a5b5aec19b3277dd43f6c0e234b0f7ed505b55af 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -332,7 +332,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_sentStalledEvent(false)
     , m_sentEndEvent(false)
     , m_closedCaptionsVisible(false)
-    , m_havePreparedToPlay(false)
+    , m_forceEffectivePreloadAuto(false)
     , m_tracksAreReady(true)
     , m_processingPreferenceChange(false)
     , m_remoteRoutesAvailable(false)
@@ -471,8 +471,7 @@ void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument)
     // A proper fix would provide a mechanism to allow this object to refresh
     // the MediaPlayer's LocalFrame and FrameLoader references on
     // document changes so that playback can be resumed properly.
-    clearMediaPlayer(LoadMediaResource);
-    scheduleDelayedAction(LoadMediaResource);
+    invokeLoadAlgorithm();
 
     // Decrement the load event delay count on oldDocument now that m_webMediaPlayer has been destroyed
     // and there is no risk of dispatching a load event from within the destructor.
@@ -501,8 +500,7 @@ void HTMLMediaElement::parseAttribute(const QualifiedName& name, const AtomicStr
     if (name == srcAttr) {
         // Trigger a reload, as long as the 'src' attribute is present.
         if (!value.isNull()) {
-            clearMediaPlayer(LoadMediaResource);
-            scheduleDelayedAction(LoadMediaResource);
+            invokeLoadAlgorithm();
         }
     } else if (name == controlsAttr) {
         UseCounter::count(document(), UseCounter::HTMLMediaElementControlsAttribute);
@@ -521,7 +519,7 @@ void HTMLMediaElement::finishParsingChildren()
     HTMLElement::finishParsingChildren();
 
     if (Traversal<HTMLTrackElement>::firstChild(*this))
-        scheduleDelayedAction(LoadTextTrackResource);
+        scheduleTextTrackResourceLoad();
 }
 
 bool HTMLMediaElement::layoutObjectIsNeeded(const ComputedStyle& style)
@@ -542,7 +540,7 @@ Node::InsertionNotificationRequest HTMLMediaElement::insertedInto(ContainerNode*
     if (insertionPoint->inDocument()) {
         UseCounter::count(document(), UseCounter::HTMLMediaElementInDocument);
         if (!getAttribute(srcAttr).isEmpty() && m_networkState == NETWORK_EMPTY)
-            scheduleDelayedAction(LoadMediaResource);
+            invokeLoadAlgorithm();
     }
 
     return InsertionShouldCallDidNotifySubtreeInsertions;
@@ -579,17 +577,11 @@ void HTMLMediaElement::didRecalcStyle(StyleRecalcChange)
         layoutObject()->updateFromElement();
 }
 
-void HTMLMediaElement::scheduleDelayedAction(DelayedActionType actionType)
+void HTMLMediaElement::scheduleTextTrackResourceLoad()
 {
-    WTF_LOG(Media, "HTMLMediaElement::scheduleDelayedAction(%p)", this);
+    WTF_LOG(Media, "HTMLMediaElement::scheduleTextTrackResourceLoad(%p)", this);
 
-    if ((actionType & LoadMediaResource) && !(m_pendingActionFlags & LoadMediaResource)) {
-        prepareForLoad();
-        m_pendingActionFlags |= LoadMediaResource;
-    }
-
-    if (actionType & LoadTextTrackResource)
-        m_pendingActionFlags |= LoadTextTrackResource;
+    m_pendingActionFlags |= LoadTextTrackResource;
 
     if (!m_loadTimer.isActive())
         m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
@@ -710,9 +702,16 @@ void HTMLMediaElement::load()
         m_autoplayMediaCounted = true;
     }
 
+    m_forceEffectivePreloadAuto = true;
+    invokeLoadAlgorithm();
+}
+
+void HTMLMediaElement::invokeLoadAlgorithm()
+{
+    WTF_LOG(Media, "HTMLMediaElement::invokeLoadAlgorithm(%p)", this);
+
     prepareForLoad();
-    loadInternal();
-    prepareToPlay();
+    scheduleNextSourceChild();
 }
 
 void HTMLMediaElement::prepareForLoad()
@@ -728,7 +727,6 @@ void HTMLMediaElement::prepareForLoad()
     m_sentEndEvent = false;
     m_sentStalledEvent = false;
     m_haveFiredLoadedData = false;
-    m_havePreparedToPlay = false;

[philipj_slow, 2016/03/11 12:52:41]

Actually, the renamed state bit should also be reset here, otherwise it'll be
impossible to reuse a media element for a new load with preload=auto.  This
could be a problem in the current shape of load() because the
`m_forceEffectivePreloadAuto = true` before invokeLoadAlgorithm() is immediately
undone, but setting it after is too late. Once the resource selection algorithm
is async, however, the order shouldn't matter.

If necessary to get things working in the interim, maybe you can do
`m_forceEffectivePreloadAuto = false` before call to invokeLoadAlgorithm()
except the one in load(), with TODOs to move it into prepareForLoad() together
with the microtask fix?

[Srirama, 2016/03/11 14:58:20]

Done.

     m_displayMode = Unknown;
 
     // 1 - Abort any already-running instance of the resource selection algorithm for this element.
@@ -792,14 +790,20 @@ void HTMLMediaElement::prepareForLoad()
     m_autoplaying = true;
 
     // 7 - Invoke the media element's resource selection algorithm.
+    invokeResourceSelectionAlgorithm();
 
     // 8 - Note: Playback of any previously playing media resource for this element stops.
+}
 
+void HTMLMediaElement::invokeResourceSelectionAlgorithm()
+{
+    WTF_LOG(Media, "HTMLMediaElement::invokeResourceSelectionAlgorithm(%p)", this);
     // The resource selection algorithm
     // 1 - Set the networkState to NETWORK_NO_SOURCE
     setNetworkState(NETWORK_NO_SOURCE);
 
-    // 2 - Asynchronously await a stable state.
+    // 2 - Set the element's show poster flag to true
+    // TODO(srirama.m): Introduce show poster flag and update it as per spec
 
     m_playedTimeRanges = TimeRanges::create();
 
@@ -808,12 +812,13 @@ void HTMLMediaElement::prepareForLoad()
     m_lastSeekTime = 0;
     m_duration = std::numeric_limits<double>::quiet_NaN();
 
-    // The spec doesn't say to block the load event until we actually run the asynchronous section
-    // algorithm, but do it now because we won't start that until after the timer fires and the
-    // event may have already fired by then.
+    // 3 - Set the media element's delaying-the-load-event flag to true (this delays the load event)
     setShouldDelayLoadEvent(true);
     if (mediaControls())
         mediaControls()->reset();
+
+    // 4 - Await a stable state, allowing the task that invoked this algorithm to continue
+    // TODO(srirama.m): Schedule an async continuation of this algorithm as per the spec
 }
 
 void HTMLMediaElement::loadInternal()
@@ -976,7 +981,7 @@ void HTMLMediaElement::loadResource(const KURL& url, ContentType& contentType)
     if (attemptLoad && canLoadURL(url, contentType)) {
         ASSERT(!webMediaPlayer());
 
-        if (!m_havePreparedToPlay && effectivePreloadType() == WebMediaPlayer::PreloadNone) {
+        if (effectivePreloadType() == WebMediaPlayer::PreloadNone) {
             WTF_LOG(Media, "HTMLMediaElement::loadResource(%p) : Delaying load because preload == 'none'", this);
             deferLoad();
         } else {
@@ -1054,7 +1059,7 @@ void HTMLMediaElement::setPlayerPreload()
     if (m_webMediaPlayer)
         m_webMediaPlayer->setPreload(effectivePreloadType());
 
-    if (loadIsDeferred() && preloadType() != WebMediaPlayer::PreloadNone)
+    if (loadIsDeferred() && effectivePreloadType() != WebMediaPlayer::PreloadNone)
         startDeferredLoad();
 }
 
@@ -1610,12 +1615,8 @@ bool HTMLMediaElement::supportsSave() const
 void HTMLMediaElement::prepareToPlay()

[philipj_slow, 2016/03/11 12:52:41]

This isn't only in preparation to play, but what would be a good name? If you
can't think of one, maybe just setIgnorePreloadNone()?

[Srirama, 2016/03/11 14:58:20]

I will go with setIgnorePreloadNone.

 {
     WTF_LOG(Media, "HTMLMediaElement::prepareToPlay(%p)", this);
-    if (m_havePreparedToPlay)
-        return;
-    m_havePreparedToPlay = true;
-
-    if (loadIsDeferred())
-        startDeferredLoad();
+    m_forceEffectivePreloadAuto = true;
+    setPlayerPreload();
 }
 
 void HTMLMediaElement::seek(double time)
@@ -1944,7 +1945,14 @@ WebMediaPlayer::Preload HTMLMediaElement::preloadType() const
 
 WebMediaPlayer::Preload HTMLMediaElement::effectivePreloadType() const
 {
-    return autoplay() ? WebMediaPlayer::PreloadAuto : preloadType();
+    if (autoplay())
+        return WebMediaPlayer::PreloadAuto;
+
+    WebMediaPlayer::Preload preload = preloadType();
+    if (m_forceEffectivePreloadAuto && preload == WebMediaPlayer::PreloadNone)

[philipj_slow, 2016/03/11 12:52:41]

Looking at https://www.w3.org/Bugs/Public/show_bug.cgi?id=28921#c3 again, I see
that this isn't quite what Firefox does.

If this instead returns WebMediaPlayer::PreloadMetaData to match Firefox, a very
explicit name would be m_clampEffectivePreloadToMetadataOrHigher, but maybe
m_ignorePreloadNone would be an OK name.

As for the difference, if the distinction is actually made in
WebMediaPlayerImpl, it would be in the amount of data that is buffered after
seeking. Either as little as possible (metadata) or as much as possible (auto).
Might be worth testing manually, but it's a hard to write a fast and reliable
test for this kind of thing. Maybe att effectivePreload to Internals.idl to test
this?

[Srirama, 2016/03/11 14:58:20]

You mean *"Maybe add effectivePreload"? I will try adding it to internals.idl
and write a test.

[philipj_slow, 2016/03/11 18:57:37]

Yes, right. The tests could simply verify that load(), play() and setting
currentTime forces the effective preload state to metadata if it was none.

+        return WebMediaPlayer::PreloadAuto;
+
+    return preload;
 }
 
 ScriptPromise HTMLMediaElement::playForBindings(ScriptState* scriptState)
@@ -2015,8 +2023,13 @@ void HTMLMediaElement::playInternal()
         webMediaPlayer()->setBufferingStrategy(WebMediaPlayer::BufferingStrategy::Normal);
 
     // 4.8.10.9. Playing the media resource
-    if (m_networkState == NETWORK_EMPTY)
-        scheduleDelayedAction(LoadMediaResource);
+    if (m_networkState == NETWORK_EMPTY) {
+        // TODO(srirama.m): Remove scheduleNextSourceChild() here and
+        // post a micro task inside invokeResourceSelectionAlgorithm().
+        // See http://crbug.com/593289 for more details.
+        invokeResourceSelectionAlgorithm();
+        scheduleNextSourceChild();
+    }
 
     // Generally "ended" and "looping" are exclusive. Here, the loop attribute
     // is ignored to seek back to start in case loop was set after playback
@@ -2078,8 +2091,13 @@ void HTMLMediaElement::pauseInternal()
 {
     WTF_LOG(Media, "HTMLMediaElement::pauseInternal(%p)", this);
 
-    if (m_networkState == NETWORK_EMPTY)
-        scheduleDelayedAction(LoadMediaResource);
+    if (m_networkState == NETWORK_EMPTY) {
+        // TODO(srirama.m): Remove scheduleNextSourceChild() here and
+        // post a micro task inside invokeResourceSelectionAlgorithm().
+        // See http://crbug.com/593289 for more details.
+        invokeResourceSelectionAlgorithm();
+        scheduleNextSourceChild();
+    }
 
     m_autoplayHelper.pauseMethodCalled();
 
@@ -2415,7 +2433,7 @@ void HTMLMediaElement::addTextTrack(WebInbandTextTrack* webTrack)
     // 7. Set the new text track's mode to the mode consistent with the user's preferences and the requirements of
     // the relevant specification for the data.
     //  - This will happen in honorUserPreferencesForAutomaticTextTrackSelection()
-    scheduleDelayedAction(LoadTextTrackResource);
+    scheduleTextTrackResourceLoad();
 
     // 8. Add the new text track to the media element's list of text tracks.
     // 9. Fire an event with the name addtrack, that does not bubble and is not cancelable, and that uses the TrackEvent
@@ -2535,7 +2553,7 @@ void HTMLMediaElement::didAddTrackElement(HTMLTrackElement* trackElement)
     // Do not schedule the track loading until parsing finishes so we don't start before all tracks
     // in the markup have been added.
     if (isFinishedParsingChildren())
-        scheduleDelayedAction(LoadTextTrackResource);
+        scheduleTextTrackResourceLoad();
 }
 
 void HTMLMediaElement::didRemoveTrackElement(HTMLTrackElement* trackElement)
@@ -2720,17 +2738,24 @@ void HTMLMediaElement::sourceWasAdded(HTMLSourceElement* source)
     // attribute and whose networkState has the value NETWORK_EMPTY, the user agent must invoke
     // the media element's resource selection algorithm.
     if (getNetworkState() == HTMLMediaElement::NETWORK_EMPTY) {
-        scheduleDelayedAction(LoadMediaResource);
+        // TODO(srirama.m): Remove scheduleNextSourceChild() here and
+        // post a micro task inside invokeResourceSelectionAlgorithm().
+        // See http://crbug.com/593289 for more details.
+        invokeResourceSelectionAlgorithm();
+        scheduleNextSourceChild();
+        // Ignore current |m_nextChildNodeToConsider| and consider |source|.
         m_nextChildNodeToConsider = source;
         return;
     }
 
     if (m_currentSourceNode && source == m_currentSourceNode->nextSibling()) {
         WTF_LOG(Media, "HTMLMediaElement::sourceWasAdded(%p) - <source> inserted immediately after current source", this);
+        // Ignore current |m_nextChildNodeToConsider| and consider |source|.
         m_nextChildNodeToConsider = source;
         return;
     }
 
+    // Consider current |m_nextChildNodeToConsider| as it is already in the middle of processing.
     if (m_nextChildNodeToConsider)
         return;
 
@@ -3115,7 +3140,7 @@ void HTMLMediaElement::stop()
     cancelPendingEventsAndCallbacks();
     m_asyncEventQueue->close();
 
-    // Stop the playback without generating events
+    // Clear everything in the Media Element
     clearMediaPlayer(-1);
     m_readyState = HAVE_NOTHING;
     m_readyStateMaximum = HAVE_NOTHING;
@@ -3487,7 +3512,7 @@ void* HTMLMediaElement::preDispatchEventHandler(Event* event)
     return nullptr;
 }
 
-// TODO(srirama.m): Refactor this and clearMediaPlayer to the extent possible.
+// TODO(srirama.m): Merge it to resetMediaElement if possible and remove it.
 void HTMLMediaElement::resetMediaPlayerAndMediaSource()
 {
     closeMediaSource();