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 b7232536c0f2f395ea56279a99c7895630bab6df..7903b5c6311e6aa5137994d2c1cbc6d2a250c8ce 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -471,8 +471,10 @@ 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);
+    invokeLoadAlgorith();
+    m_pendingActionFlags |= LoadMediaResource;
+    if (!m_loadTimer.isActive())
+        m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
 
     // 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 +503,10 @@ 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);
+            invokeLoadAlgorith();
+            m_pendingActionFlags |= LoadMediaResource;

[Srirama, 2015/12/16 07:33:43]

May be we can move this as well inside, probably while separating resource
selection algorithm, i will do it.

[philipj_slow, 2015/12/16 15:51:07]

Yes, unless all of this could be moved inside the call, I wouldn't rename it
invokeLoadAlgorithm().

+            if (!m_loadTimer.isActive())
+                m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
         }
     } else if (name == controlsAttr) {
         configureMediaControls();
@@ -580,7 +584,7 @@ void HTMLMediaElement::scheduleDelayedAction(DelayedActionType actionType)
     WTF_LOG(Media, "HTMLMediaElement::scheduleDelayedAction(%p)", this);
 
     if ((actionType & LoadMediaResource) && !(m_pendingActionFlags & LoadMediaResource)) {
-        prepareForLoad();
+        invokeLoadAlgorith();
         m_pendingActionFlags |= LoadMediaResource;
     }
 
@@ -593,7 +597,7 @@ void HTMLMediaElement::scheduleDelayedAction(DelayedActionType actionType)
 
 void HTMLMediaElement::scheduleNextSourceChild()
 {
-    // Schedule the timer to try the next <source> element WITHOUT resetting state ala prepareForLoad.
+    // Schedule the timer to try the next <source> element WITHOUT resetting state ala invokeLoadAlgorith.
     m_pendingActionFlags |= LoadMediaResource;
     m_loadTimer.startOneShot(0, BLINK_FROM_HERE);
 }
@@ -706,27 +710,18 @@ void HTMLMediaElement::load()
         m_autoplayMediaCounted = true;
     }
 
-    prepareForLoad();
+    invokeLoadAlgorith();

[philipj_slow, 2015/12/16 15:51:07]

In fact, I would keep the name prepareForLoad() for now, since loadInternal() is
still after this call.

To really make this per spec, HTMLMediaElement::scheduleDelayedAction would have
to be fixed, because that's calling prepareForLoad() without a following
loadInternal(), which is like the "media element load algorithm" without the
final "Invoke the media element's resource selection algorithm" which isn't
something the spec ever does.

I wouldn't recommend trying to sort out all of this at the same time, though.
Without trying to do the work myself, I can't really suggest which order to fix
things to make sense, you'll have to experiment I think.

     loadInternal();
     prepareToPlay();
 }
 
-void HTMLMediaElement::prepareForLoad()
+// 4.8.13.5 Loading the media resource
+void HTMLMediaElement::invokeLoadAlgorith()
 {
-    WTF_LOG(Media, "HTMLMediaElement::prepareForLoad(%p)", this);
+    WTF_LOG(Media, "HTMLMediaElement::invokeLoadAlgorith(%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);
 
     // 1 - Abort any already-running instance of the resource selection algorithm for this element.
     m_loadState = WaitingForSource;
@@ -738,13 +733,11 @@ void HTMLMediaElement::prepareForLoad()
 
     // 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) {

[Srirama, 2015/12/16 07:33:43]

Most of this section is covered in resetMediaElement, but i left it as it is for
time being, so that it will be in sync with the spec.

         // 4.1 - Queue a task to fire a simple event named emptied at the media element.
         scheduleEvent(EventTypeNames::emptied);
 
@@ -2986,7 +2979,7 @@ void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClientWithoutLockin
     }
 }
 
-void HTMLMediaElement::clearMediaPlayer(int flags)
+void HTMLMediaElement::resetMediaElement(int flags)
 {
     forgetResourceSpecificTracks();
 
@@ -2999,6 +2992,11 @@ void HTMLMediaElement::clearMediaPlayer(int flags)
         clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();
     }
 
+#if ENABLE(WEB_AUDIO)
+    if (m_audioSourceNode)
+        audioSourceProvider().setClient(m_audioSourceNode);
+#endif
+
     stopPeriodicTimers();
     m_loadTimer.stop();
 
@@ -3008,6 +3006,26 @@ void HTMLMediaElement::clearMediaPlayer(int flags)
     // 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();
 
@@ -3025,24 +3043,12 @@ void HTMLMediaElement::stop()
     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());
@@ -3391,7 +3397,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();