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 441 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);

[philipj_slow, 2016/02/09 07:34:34]

As I wrote in some earlier CL, I think this belongs inside
invokeLoadAlgorithm(), so that it's always just a single call where the spec
says "invoke the media element load algorithm", although this isn't one of those
places.

[Srirama, 2016/02/09 14:50:46]

Done.

-    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.
     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);

[philipj_slow, 2016/02/09 07:34:34]

Here too.

[Srirama, 2016/02/09 14:50:46]

Done.

-            scheduleDelayedAction(LoadMediaResource);
+            invokeLoadAlgorithm();
         }
     } 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);
+            invokeLoadAlgorithm();
     }
 
     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);
@@ skipping 105 matching lines @@
         // they don't feel like such to the users and hence we don't want to
         // count them for the purposes of metrics.
         m_autoplayMediaCounted = true;
     }
 
     prepareForLoad();
     loadInternal();
     prepareToPlay();
 }
 
+void HTMLMediaElement::invokeLoadAlgorithm()
+{
+    WTF_LOG(Media, "HTMLMediaElement::invokeLoadAlgorithm(%p)", this);
+
+    prepareForLoad();
+    scheduleNextSourceChild();

[philipj_slow, 2016/02/09 07:34:34]

scheduleNextSourceChild() belongs somewhere in the resource selection algorithm.
If that move works, you can simply rename prepareForLoad() to
invokeLoadAlgorithm() which would make for one less internal concept to
understand.

[Srirama, 2016/02/09 14:50:46]

As you said in the below comment, we can move scheduleNextSourceChild() inside
invokeResourceSelectionAlgorithm() but the load() function is where it may
create problem. There we directly invoke loadInternal() and then prepareToPlay()
without using scheduleNextSourceChild(). So what should we do about
prepareToPlay()? Should we remove that and just invoke resource load algorithm
as the spec says?

[philipj_slow, 2016/02/10 05:02:30]

OK, this is interesting. Per spec load() absolutely should invoke the media
element load algorithm, in fact it is the only thing it should do.

In addition, there's an extra prepareToPlay() call, and that is needed so that
preload="none" is overridden. The spec bug is
https://www.w3.org/Bugs/Public/show_bug.cgi?id=28921 but I haven't gotten around
to updating the spec for that yet.

It looks like prepareToPlay() has to be called after loadInternal(), because
before that there may not be a media resource to act on in
HTMLMediaElement::executeDeferredLoad.

The cleanest approach would be to make sure that "Delaying load because preload
== 'none'" path in HTMLMediaElement::loadResource is not taken if load() was
called, and currently that depends on m_havePreparedToPlay. I poked at it in
https://codereview.chromium.org/1687793002 and it would be nice if only
effectivePreloadType() were used there.

So, I would lean towards renaming prepareToPlay() and m_havePreparedToPlay to
something that has the semantics of "we will now ignore preload=none, clamping
the effective preload state to metadata or higher depending on other state". Not
sure what to call it, maybe you can see if it would actually work first?

[Srirama, 2016/02/10 11:16:19]

ok, i have tried the changes locally and all the layout tests are passing. But
one problem here is the flag m_havePreparedToPlay is currently being reset in
prepareForLoad(), so right now if i set the flag  inbetween prepareforload and
loadinternal it is working fine. But once we remove all this infavor of
invokeLoadAlgorithm, how can we handle this flag? Should i reset the flag at the
call sites ?

[philipj_slow, 2016/02/16 12:19:34]

Let's see, m_havePreparedToPlay is used in two ways:

1. It's combined with effectivePreloadType() to possibly defer the load in
HTMLMediaElement::loadResource, which is very much related to
https://www.w3.org/Bugs/Public/show_bug.cgi?id=28921

2. It can cause an early return in HTMLMediaElement::prepareToPlay(), and
removing that could cause startDeferredLoad() to be called twice if
loadIsDeferred(), but judging by the various methods involved in load deferral,
it looks like perhaps this isn't observable at all.

So, I would propose removing prepareToPlay() and m_havePreparedToPlay and
replacing them with two things that explicitly do what seems to be the important
bits:

1. A flag which causes effectivePreloadType() to return PreloadAuto if it is
set, and it is set by playing and seeking.

2. Whatever changes, if any, are required to ensure that a deferred load is
started precisely when it should be, at least when seeking and playing. May
require some changes to the whole deferred load machinery.

+}
+
 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;
@@ skipping 56 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.
+    // 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();
 
     // 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.
+    // 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

[philipj_slow, 2016/02/09 07:34:34]

How does this currently work? I think it's the earlier scheduleNextSourceChild()
that should be moved here.

[Srirama, 2016/02/09 14:50:46]

We can move it expect for the problem mentioned in my above comment.

 }
 
 void HTMLMediaElement::loadInternal()
 {
     // HTMLMediaElement::textTracksAreReady will need "... the text tracks whose mode was not in the
     // disabled state when the element's resource selection algorithm last started".
     m_textTracksWhenResourceSelectionBegan.clear();
     if (m_textTracks) {
         for (unsigned i = 0; i < m_textTracks->length(); ++i) {
             TextTrack* track = m_textTracks->anonymousIndexedGetter(i);
@@ skipping 1146 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) {
+        invokeResourceSelectionAlgorithm();
+        scheduleNextSourceChild();

[philipj_slow, 2016/02/09 07:34:34]

Try to get rid of scheduleNextSourceChild() here and below.

Traversing source children is what the resource selection algorithm does, having
scheduleNextSourceChild() *after* the invocation here is a sign of some
mismatch.

[Srirama, 2016/02/09 14:50:46]

Probably this is also linked to the above issue.

+    }
 
     // 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) {
+        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);
+        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 368 matching lines @@
 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
+    // Clear everything in the Media Element
     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;
@@ skipping 345 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 219 matching lines @@
 {
     visitor->trace(m_client);
 }
 
 DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)
 {
     visitor->trace(m_client);
 }
 
 } // namespace blink