Media element resource selection algorithm should "await a stable state"

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 06d4a93b6dfca9cb3d42b26427ecb7be2d3f6673..f0584fceb21620212a88249d38fb745c8aa00f31 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -27,6 +27,7 @@
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ExceptionStatePlaceholder.h"
+#include "bindings/core/v8/Microtask.h"
 #include "bindings/core/v8/ScriptController.h"
 #include "bindings/core/v8/ScriptEventListener.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
@@ -329,7 +330,6 @@ private:
     Member<HTMLMediaElement> m_element;
 };
 
-
 void HTMLMediaElement::recordAutoplayMetric(AutoplayMetrics metric)
 {
     DEFINE_STATIC_LOCAL(EnumerationHistogram, autoplayHistogram, ("Blink.MediaElement.Autoplay", NumberOfAutoplayMetrics));
@@ -373,7 +373,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     : HTMLElement(tagName, document)
     , ActiveScriptWrappable(this)
     , ActiveDOMObject(&document)
-    , m_loadTimer(this, &HTMLMediaElement::loadTimerFired)
+    , m_textTrackLoadTimer(this, &HTMLMediaElement::textTrackLoadTimerFired)
     , m_progressEventTimer(this, &HTMLMediaElement::progressEventTimerFired)
     , m_playbackProgressTimer(this, &HTMLMediaElement::playbackProgressTimerFired)
     , m_audioTracksTimer(this, &HTMLMediaElement::audioTracksTimerFired)
@@ -398,7 +398,6 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_displayMode(Unknown)
     , m_cachedTime(std::numeric_limits<double>::quiet_NaN())
     , m_fragmentEndTime(std::numeric_limits<double>::quiet_NaN())
-    , m_pendingActionFlags(0)
     , m_userGestureRequiredForPlay(false)
     , m_playing(false)
     , m_shouldDelayLoadEvent(false)
@@ -422,6 +421,8 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_textTracks(nullptr)
     , m_playPromiseResolveTask(CancellableTaskFactory::create(this, &HTMLMediaElement::resolvePlayPromises))
     , m_playPromiseRejectTask(CancellableTaskFactory::create(this, &HTMLMediaElement::rejectPlayPromises))
+    , m_resourceSelectionAlgorithmContinuationTask(CancellableTaskFactory::create(this, &HTMLMediaElement::continueResourceSelectionAlgorithm))
+    , m_loadNextSourceChildOnErrorTask(CancellableTaskFactory::create(this, &HTMLMediaElement::loadNextSourceChildOnError))
     , m_audioSourceNode(nullptr)
     , m_autoplayHelperClient(AutoplayHelperClientImpl::create(this))
     , m_autoplayHelper(AutoplayExperimentHelper::create(m_autoplayHelperClient.get()))
@@ -469,6 +470,9 @@ HTMLMediaElement::~HTMLMediaElement()
 
     closeMediaSource();
 
+    if (m_resourceSelectionAlgorithmContinuationTask->isPending())
+        m_resourceSelectionAlgorithmContinuationTask->cancel();
+
     removeElementFromDocumentMap(this, &document());
 
     // Destroying the player may cause a resource load to be canceled,
@@ -675,17 +679,8 @@ void HTMLMediaElement::scheduleTextTrackResourceLoad()
 {
     WTF_LOG(Media, "HTMLMediaElement::scheduleTextTrackResourceLoad(%p)", this);
 
-    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 invokeLoadAlgorithm.
-    m_pendingActionFlags |= LoadMediaResource;
-    m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
+    if (!m_textTrackLoadTimer.isActive())
+        m_textTrackLoadTimer.startOneShot(0, BLINK_FROM_HERE);
 }
 
 void HTMLMediaElement::scheduleEvent(const AtomicString& eventName)
@@ -701,19 +696,10 @@ void HTMLMediaElement::scheduleEvent(RawPtr<Event> event)
     m_asyncEventQueue->enqueueEvent(event);
 }
 
-void HTMLMediaElement::loadTimerFired(Timer<HTMLMediaElement>*)
+void HTMLMediaElement::textTrackLoadTimerFired(Timer<HTMLMediaElement>*)
 {
-    if (m_pendingActionFlags & LoadTextTrackResource)
-        honorUserPreferencesForAutomaticTextTrackSelection();
-
-    if (m_pendingActionFlags & LoadMediaResource) {
-        if (m_loadState == LoadingFromSourceElement)
-            loadNextSourceChild();
-        else
-            loadInternal();
-    }
-
-    m_pendingActionFlags = 0;
+    WTF_LOG(Media, "HTMLMediaElement::textTrackLoadTimerFired(%p)", this);
+    honorUserPreferencesForAutomaticTextTrackSelection();
 }
 
 MediaError* HTMLMediaElement::error() const
@@ -774,16 +760,15 @@ void HTMLMediaElement::invokeLoadAlgorithm()
 
     // 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_displayMode = Unknown;
 
     // 1 - Abort any already-running instance of the resource selection algorithm for this element.
+    m_resourceSelectionAlgorithmContinuationTask->cancel();
+    m_loadNextSourceChildOnErrorTask->cancel();
     m_loadState = WaitingForSource;
     m_currentSourceNode = nullptr;
 
@@ -879,9 +864,19 @@ void HTMLMediaElement::invokeResourceSelectionAlgorithm()
         mediaControls()->reset();
 
     // 4 - Await a stable state, allowing the task that invoked this algorithm to continue
-    // TODO(srirama.m): Remove scheduleNextSourceChild() and post a microtask instead.
-    // See http://crbug.com/593289 for more details.
-    scheduleNextSourceChild();
+    enqueueMicrotaskForResourceSelectionAlgorithmContinuation();
+}
+
+void HTMLMediaElement::continueResourceSelectionAlgorithm()
+{
+    WTF_LOG(Media, "HTMLMediaElement::continueResourceSelectionAlgorithm(%p)", this);
+    loadInternal();
+}
+
+void HTMLMediaElement::enqueueMicrotaskForResourceSelectionAlgorithmContinuation()

[philipj_slow, 2016/04/19 14:52:44]

This is called in just a single place, inline it?

+{
+    OwnPtr<WebTaskRunner::Task> task = adoptPtr(m_resourceSelectionAlgorithmContinuationTask->cancelAndCreate());
+    Microtask::enqueueMicrotask(task.release());
 }
 
 void HTMLMediaElement::loadInternal()
@@ -984,6 +979,13 @@ void HTMLMediaElement::loadNextSourceChild()
     loadResource(mediaURL, contentType);
 }
 
+void HTMLMediaElement::loadNextSourceChildOnError()

[philipj_slow, 2016/04/19 14:52:44]

Maybe AfterError since it happens after awaiting a stable state?

+{
+    // 9.Otherwise.11 - Forget the media element's media-resource-specific tracks.
+    forgetResourceSpecificTracks();
+    loadNextSourceChild();
+}
+
 void HTMLMediaElement::loadResource(const KURL& url, ContentType& contentType)
 {
     ASSERT(isMainThread());
@@ -1414,13 +1416,10 @@ void HTMLMediaElement::mediaLoadingFailed(WebMediaPlayer::NetworkState error)
             WTF_LOG(Media, "HTMLMediaElement::setNetworkState(%p) - error event not sent, <source> was removed", this);
 
         // 9.Otherwise.10 - Asynchronously await a stable state. The synchronous section consists of all the remaining steps of this algorithm until the algorithm says the synchronous section has ended.
-
-        // 9.Otherwise.11 - Forget the media element's media-resource-specific tracks.
-        forgetResourceSpecificTracks();
-
         if (havePotentialSourceChild()) {
             WTF_LOG(Media, "HTMLMediaElement::setNetworkState(%p) - scheduling next <source>", this);
-            scheduleNextSourceChild();
+            OwnPtr<WebTaskRunner::Task> task = adoptPtr(m_loadNextSourceChildOnErrorTask->cancelAndCreate());
+            Microtask::enqueueMicrotask(task.release());
         } else {
             WTF_LOG(Media, "HTMLMediaElement::setNetworkState(%p) - no more <source> elements, waiting", this);
             waitForSourceChange();
@@ -2789,7 +2788,7 @@ void HTMLMediaElement::sourceWasAdded(HTMLSourceElement* source)
 
     // 25. Jump back to the find next candidate step above.
     m_nextChildNodeToConsider = source;
-    scheduleNextSourceChild();
+    loadNextSourceChild();
 }
 
 void HTMLMediaElement::sourceWasRemoved(HTMLSourceElement* source)
@@ -3139,9 +3138,8 @@ void HTMLMediaElement::clearMediaPlayer()
     }
 
     stopPeriodicTimers();
-    m_loadTimer.stop();
+    m_textTrackLoadTimer.stop();
 
-    m_pendingActionFlags = 0;
     m_loadState = WaitingForSource;
 
     // We can't cast if we don't have a media player.