Refactor resource load and resource selection algorithms as per spec

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

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 453 matching lines @@
 
     removeElementFromDocumentMap(this, &oldDocument);
     addElementToDocumentMap(this, &document());
 
     // FIXME: This is a temporary fix to prevent this object from causing the
     // MediaPlayer to dereference LocalFrame and FrameLoader pointers from the
     // previous document. This restarts the load, as if the src attribute had been set.
     // 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);
+    prepareForLoad();
+    scheduleNextSourceChild();
 
     // 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.
     oldDocument.decrementLoadEventDelayCount();
 
     ActiveDOMObject::didMoveToNewExecutionContext(&document());
     HTMLElement::didMoveToNewDocument(oldDocument);
 }
 
 bool HTMLMediaElement::supportsFocus() const
 {
     if (ownerDocument()->isMediaDocument())
         return false;
 
     // If no controls specified, we should still be able to focus the element if it has tabIndex.
     return shouldShowControls() || HTMLElement::supportsFocus();
 }
 
 bool HTMLMediaElement::isMouseFocusable() const
 {
     return false;
 }
 
 void HTMLMediaElement::parseAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& value)
 {
     if (name == srcAttr) {
         // Trigger a reload, as long as the 'src' attribute is present.
         if (!value.isNull()) {
-            clearMediaPlayer(LoadMediaResource);
-            scheduleDelayedAction(LoadMediaResource);
+            prepareForLoad();

[philipj_slow, 2016/01/21 12:48:11]

The spec says "If a src attribute of a media element is set or changed, the user
agent must invoke the media element's media element load algorithm." If you want
to wrap these two in a new invokeLoadAlgorithm() that might be nice.

[Srirama, 2016/01/28 11:31:03]

Done.

+            scheduleNextSourceChild();
         }
     } else if (name == controlsAttr) {
         configureMediaControls();
     } else if (name == preloadAttr) {
         setPlayerPreload();
     } else if (name == disableremoteplaybackAttr) {
         UseCounter::count(document(), UseCounter::DisableRemotePlaybackAttribute);
     } else {
         HTMLElement::parseAttribute(name, oldValue, value);
     }
 }
 
 void HTMLMediaElement::finishParsingChildren()
 {
     HTMLElement::finishParsingChildren();
 
     if (Traversal<HTMLTrackElement>::firstChild(*this))
-        scheduleDelayedAction(LoadTextTrackResource);
+        scheduleTextTrackResourceLoad();
 }
 
 bool HTMLMediaElement::layoutObjectIsNeeded(const ComputedStyle& style)
 {
     return shouldShowControls() && HTMLElement::layoutObjectIsNeeded(style);
 }
 
 LayoutObject* HTMLMediaElement::createLayoutObject(const ComputedStyle&)
 {
     return new LayoutMedia(this);
 }
 
 Node::InsertionNotificationRequest HTMLMediaElement::insertedInto(ContainerNode* insertionPoint)
 {
     WTF_LOG(Media, "HTMLMediaElement::insertedInto(%p, %p)", this, insertionPoint);
 
     HTMLElement::insertedInto(insertionPoint);
     if (insertionPoint->inDocument()) {
-        if (!getAttribute(srcAttr).isEmpty() && m_networkState == NETWORK_EMPTY)
-            scheduleDelayedAction(LoadMediaResource);
+        if (!getAttribute(srcAttr).isEmpty() && m_networkState == NETWORK_EMPTY) {
+            if (!(m_pendingActionFlags & LoadMediaResource))

[philipj_slow, 2016/01/21 12:48:11]

I'm not sure I understand these changes. The existing behavior is not per spec,
as inserting a media element into a document shouldn't actually do anything.
IIRC, this is actually needed here because removing from a document will destroy
the media player. Would it work to just invoke the resource selection algorithm
here? Or the media element load algorithm?

It also doesn't really make sense that this is looking at the src attribute,
when there could also be source element children. If the reason for that is that
inserting a source element child will invoke the resource selection algorithm,
then I think doing the same here makes the most sense. Please also decorate this
with a TODO explaining why it's actually wrong and not per spec.

[Srirama, 2016/01/28 11:31:03]

Removed unneeded conditions. But i couldn't exactly figure the spec location for
this, i could see the spec corresponding to source element insertion but
couldn't see the same thing for media element.

+                prepareForLoad();
+
+            scheduleNextSourceChild();
+        }
     }
 
     return InsertionShouldCallDidNotifySubtreeInsertions;
 }
 
 void HTMLMediaElement::didNotifySubtreeInsertionsToDocument()
 {
     configureMediaControls();
 }
 
@@ skipping 16 matching lines @@
     if (layoutObject())
         layoutObject()->updateFromElement();
 }
 
 void HTMLMediaElement::didRecalcStyle(StyleRecalcChange)
 {
     if (layoutObject())
         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);
 }
 
 void HTMLMediaElement::scheduleNextSourceChild()
 {
     // Schedule the timer to try the next <source> element WITHOUT resetting state ala prepareForLoad.
     m_pendingActionFlags |= LoadMediaResource;
-    m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
+    if (!m_loadTimer.isActive())

[philipj_slow, 2016/01/21 12:48:11]

Why was this change needed?

[Srirama, 2016/01/28 11:31:03]

Removed, not needed, i have added it to fix layout test case failures, but i
think those failures may be because of the changes in above places. After fixing
your review comments all the test cases are passing.

+        m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
 }
 
 void HTMLMediaElement::scheduleEvent(const AtomicString& eventName)
 {
     scheduleEvent(Event::createCancelable(eventName));
 }
 
 void HTMLMediaElement::scheduleEvent(PassRefPtrWillBeRawPtr<Event> event)
 {
 #if LOG_MEDIA_EVENTS
@@ skipping 99 matching lines @@
 
     prepareForLoad();
     loadInternal();
     prepareToPlay();
 }
 
 void HTMLMediaElement::prepareForLoad()
 {
     WTF_LOG(Media, "HTMLMediaElement::prepareForLoad(%p)", this);
 
-    // Perform the cleanup required for the resource load algorithm to run.
-    stopPeriodicTimers();
-    m_loadTimer.stop();
-    cancelDeferredLoad();
-    // FIXME: Figure out appropriate place to reset LoadTextTrackResource if necessary and set m_pendingActionFlags to 0 here.
-    m_pendingActionFlags &= ~LoadMediaResource;
-    m_sentEndEvent = false;
-    m_sentStalledEvent = false;
-    m_haveFiredLoadedData = false;
-    m_completelyLoaded = false;
-    m_havePreparedToPlay = false;
-    m_displayMode = Unknown;
+    NetworkState networkState = m_networkState;
+    resetMediaElement(LoadMediaResource);

[philipj_slow, 2016/01/21 12:48:10]

This will now be the only place where resetMediaElement is called with an
argument other than -1. Isn't it possible to just always clear all flags here?
Any async continuation of this method should be scheduled after this point.

If you make that change, also make sure to cancel any pending callback.

[Srirama, 2016/01/28 11:31:03]

passing -1 is resetting all the flags and because of that text track related
test cases are failing.

 
     // 1 - Abort any already-running instance of the resource selection algorithm for this element.
     m_loadState = WaitingForSource;
     m_currentSourceNode = nullptr;
 
     // 2 - 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();
 
     // 3 - If the media element's networkState is set to NETWORK_LOADING or NETWORK_IDLE, queue
     // a task to fire a simple event named abort at the media element.
-    if (m_networkState == NETWORK_LOADING || m_networkState == NETWORK_IDLE)
+    if (networkState == NETWORK_LOADING || networkState == NETWORK_IDLE)
         scheduleEvent(EventTypeNames::abort);
 
-    resetMediaPlayerAndMediaSource();
-
     // 4 - If the media element's networkState is not set to NETWORK_EMPTY, then run these substeps
-    if (m_networkState != NETWORK_EMPTY) {
+    if (networkState != NETWORK_EMPTY) {
         // 4.1 - Queue a task to fire a simple event named emptied at the media element.
         scheduleEvent(EventTypeNames::emptied);
 
         // 4.2 - If a fetching process is in progress for the media element, the user agent should stop it.
         setNetworkState(NETWORK_EMPTY);
 
         // 4.3 - Forget the media element's media-resource-specific tracks.
         forgetResourceSpecificTracks();
 
         // 4.4 - If readyState is not set to HAVE_NOTHING, then set it to that state.
@@ skipping 22 matching lines @@
     }
 
     // 5 - Set the playbackRate attribute to the value of the defaultPlaybackRate attribute.
     setPlaybackRate(defaultPlaybackRate());
 
     // 6 - Set the error attribute to null and the autoplaying flag to true.
     m_error = nullptr;
     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.

[philipj_slow, 2016/01/21 12:48:11]

Why was this comment removed? The spec presumably said this when this code was
implemented, and it's still not quite per spec with regard to awaiting a stable
state. If you remove this, instead sync with the new spec steps and add TODOs
for the bits that aren't correct.

Probably at the end of this method is where you should schedule an async
continuation corresponding to "await a stable state". (That should use a
microtask per spec, but let's try to not introduce any observable changes with
this cleanup.)

[Srirama, 2016/01/28 11:31:03]

Done.

-
     m_playedTimeRanges = TimeRanges::create();
 
     // FIXME: Investigate whether these can be moved into m_networkState != NETWORK_EMPTY block above
     // so they are closer to the relevant spec steps.
     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.
@@ skipping 1163 matching lines @@
 {
     WTF_LOG(Media, "HTMLMediaElement::playInternal(%p)", this);
 
     // Always return the buffering strategy to normal when not paused,
     // regardless of the cause. (In contrast with aggressive buffering which is
     // only enabled by pause(), not pauseInternal().)
     if (webMediaPlayer())
         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) {

[philipj_slow, 2016/01/21 12:48:11]

Can this simply be invokeResourceSelectionAlgorithm() as in the spec? If that
change has any observable side-effects, a TODO explaining the difference and how
to remove it would be good.

[Srirama, 2016/01/28 11:31:03]

Done. Do you want me to remove scheduleNextSourceChild() here? Removing
scheduleNextSourceChild() is causing test failure. So keeping it for now.

+        if (!(m_pendingActionFlags & LoadMediaResource))
+            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
     // ended. See http://crbug.com/364442
     if (endedPlayback(LoopCondition::Ignored))
         seek(0);
 
     if (m_paused) {
         m_paused = false;
         invalidateCachedTime();
@@ skipping 38 matching lines @@
     if (webMediaPlayer() && UserGestureIndicator::processingUserGesture())
         webMediaPlayer()->setBufferingStrategy(WebMediaPlayer::BufferingStrategy::Aggressive);
 
     pauseInternal();
 }
 
 void HTMLMediaElement::pauseInternal()
 {
     WTF_LOG(Media, "HTMLMediaElement::pauseInternal(%p)", this);
 
-    if (m_networkState == NETWORK_EMPTY)
-        scheduleDelayedAction(LoadMediaResource);
+    if (m_networkState == NETWORK_EMPTY) {
+        if (!(m_pendingActionFlags & LoadMediaResource))

[philipj_slow, 2016/01/21 12:48:11]

Same as for playInternal().

[Srirama, 2016/01/28 11:31:03]

Done.

+            invokeResourceSelectionAlgorithm();
+
+        scheduleNextSourceChild();
+    }
 
     m_autoplayHelper.pauseMethodCalled();
 
     m_autoplaying = false;
 
     if (!m_paused) {
         recordMetricsIfPausing();
 
         m_paused = true;
         scheduleTimeupdateEvent(false);
@@ skipping 292 matching lines @@
     // 5. Populate the new text track's list of cues with the cues parsed so far, folllowing the guidelines for exposing
     // cues, and begin updating it dynamically as necessary.
     //   - Thess are all done by the media engine.
 
     // 6. Set the new text track's readiness state to loaded.
     textTrack->setReadinessState(TextTrack::Loaded);
 
     // 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
     // interface, with the track attribute initialized to the text track's TextTrack object, at the media element's
     // textTracks attribute's TextTrackList object.
     addTextTrack(textTrack);
 }
 
 void HTMLMediaElement::removeTextTrack(WebInbandTextTrack* webTrack)
 {
@@ skipping 99 matching lines @@
     // media element's list of text tracks ... [continues in TextTrackList::append]
     TextTrack* textTrack = trackElement->track();
     if (!textTrack)
         return;
 
     addTextTrack(textTrack);
 
     // 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)
 {
 #if !LOG_DISABLED
     KURL url = trackElement->getNonEmptyURLAttribute(srcAttr);
     WTF_LOG(Media, "HTMLMediaElement::didRemoveTrackElement(%p) - 'src' is %s", this, urlForLoggingMedia(url).utf8().data());
 #endif
 
     TextTrack* textTrack = trackElement->track();
@@ skipping 156 matching lines @@
 #endif
 
     // We should only consider a <source> element when there is not src attribute at all.
     if (fastHasAttribute(srcAttr))
         return;
 
     // 4.8.8 - If a source element is inserted as a child of a media element that has no src
     // attribute and whose networkState has the value NETWORK_EMPTY, the user agent must invoke
     // the media element's resource selection algorithm.
     if (networkState() == HTMLMediaElement::NETWORK_EMPTY) {
-        scheduleDelayedAction(LoadMediaResource);
+        if (!(m_pendingActionFlags & LoadMediaResource))

[philipj_slow, 2016/01/21 12:48:11]

Here too, can we just invoke the resource selection algorithm?

[Srirama, 2016/01/28 11:31:03]

Done.

+            invokeResourceSelectionAlgorithm();
+
+        scheduleNextSourceChild();
         m_nextChildNodeToConsider = source;
         return;
     }
 
     if (m_currentSourceNode && source == m_currentSourceNode->nextSibling()) {
         WTF_LOG(Media, "HTMLMediaElement::sourceWasAdded(%p) - <source> inserted immediately after current source", this);
         m_nextChildNodeToConsider = source;
         return;
     }
 
@@ skipping 333 matching lines @@
     audioSourceProvider().setClient(nullptr);
 #endif
     if (m_webMediaPlayer) {
 #if ENABLE(WEB_AUDIO)
         m_audioSourceProvider.wrap(nullptr);
 #endif
         m_webMediaPlayer.clear();
     }
 }
 
-void HTMLMediaElement::clearMediaPlayer(int flags)
+void HTMLMediaElement::resetMediaElement(int flags)
 {
     forgetResourceSpecificTracks();
 
     closeMediaSource();
 
     cancelDeferredLoad();
 
     {
         AudioSourceProviderClientLockScope scope(*this);
         clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();
     }
 
+#if ENABLE(WEB_AUDIO)
+    if (m_audioSourceNode)
+        audioSourceProvider().setClient(m_audioSourceNode);
+#endif
+
     stopPeriodicTimers();
     m_loadTimer.stop();
 
     m_pendingActionFlags &= ~flags;
     m_loadState = WaitingForSource;
 
     // We can't cast if we don't have a media player.
     m_remoteRoutesAvailable = false;
     m_playingRemotely = false;
+
+    // FIXME: Figure out appropriate place to reset LoadTextTrackResource if necessary and set m_pendingActionFlags to 0 here.
+    m_sentEndEvent = false;
+    m_sentStalledEvent = false;
+    m_haveFiredLoadedData = false;
+    m_completelyLoaded = false;
+    m_havePreparedToPlay = false;
+    m_displayMode = Unknown;
+
+    m_readyState = HAVE_NOTHING;
+    m_readyStateMaximum = HAVE_NOTHING;
+    setNetworkState(NETWORK_EMPTY);
+    setShouldDelayLoadEvent(false);
+    m_currentSourceNode = nullptr;
+    invalidateCachedTime();
+    cueTimeline().updateActiveCues(0);
+    m_playing = false;
+    m_paused = true;
+    m_seeking = false;
+
     if (mediaControls())
         mediaControls()->refreshCastButtonVisibilityWithoutUpdate();
 
     if (layoutObject())
         layoutObject()->setShouldDoFullPaintInvalidation();
 }
 
 void HTMLMediaElement::stop()
 {
     WTF_LOG(Media, "HTMLMediaElement::stop(%p)", this);
 
     recordMetricsIfPausing();
 
     // Close the async event queue so that no events are enqueued.
     cancelPendingEventsAndCallbacks();
     m_asyncEventQueue->close();
 
-    // Stop the playback without generating events
-    clearMediaPlayer(-1);
-    m_readyState = HAVE_NOTHING;
-    m_readyStateMaximum = HAVE_NOTHING;
-    setNetworkState(NETWORK_EMPTY);
-    setShouldDelayLoadEvent(false);
-    m_currentSourceNode = nullptr;
-    invalidateCachedTime();
-    cueTimeline().updateActiveCues(0);
-    m_playing = false;
-    m_paused = true;
-    m_seeking = false;
+    // Clear everything in the Media Element
+    resetMediaElement(-1);
 
     if (layoutObject())
         layoutObject()->updateFromElement();
 
-    stopPeriodicTimers();
-
     // Ensure that hasPendingActivity() is not preventing garbage collection, since otherwise this
     // media element will simply leak.
     ASSERT(!hasPendingActivity());
 }
 
 bool HTMLMediaElement::hasPendingActivity() const
 {
     // The delaying-the-load-event flag is set by resource selection algorithm when looking for a
     // resource to load, before networkState has reached to NETWORK_LOADING.
     if (m_shouldDelayLoadEvent)
@@ skipping 328 matching lines @@
 }
 
 void* HTMLMediaElement::preDispatchEventHandler(Event* event)
 {
     if (event && event->type() == EventTypeNames::webkitfullscreenchange)
         configureMediaControls();
 
     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();
 
     {
         AudioSourceProviderClientLockScope scope(*this);
         clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();
     }
 
     // We haven't yet found out if any remote routes are available.
@@ skipping 227 matching lines @@
     visitor->trace(m_client);
 }
 
 DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)
 {
     visitor->trace(m_client);
 }
 #endif
 
 }