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..c4972bb2642b47da759aa33ea9b36374a2686382 100644
--- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -471,7 +471,8 @@ 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);
+    m_pendingActionFlags &= ~LoadMediaResource;

[philipj_slow, 2015/12/11 15:01:51]

This does much less than clearMediaPlayer, is the change really not observable?

[Srirama, 2015/12/11 15:05:47]

resetMediaPlayerAndMediaSource is internally called in scheduleDelayedAction()
and remaining bits are done in prepareForLoad().

[philipj_slow, 2015/12/14 14:30:40]

OK, so assuming that this CL does not change any observable behavior, that the
bits left is exactly the code that was previously run, in the same order to the
extent that it matters...

I don't think this is really any clearer. It's weird that we have both
clearMediaPlayer and resetMediaPlayerAndMediaSource, but I was hoping that they
could be merged into a method. Otherwise the intent of the code is pretty
mysterious. Why forget resource specific text tracks here and in parseAttribute?
If anything I would hope that both of these places would be reduced into a
single call corresponding to "invoke the media element's media element load
algorithm", as the spec puts it.

If you have an end state in mind for these changes, maybe I can make sense of
the intermediate states :)

+    forgetResourceSpecificTracks();
     scheduleDelayedAction(LoadMediaResource);
 
     // Decrement the load event delay count on oldDocument now that m_webMediaPlayer has been destroyed
@@ -501,7 +502,8 @@ 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);
+            m_pendingActionFlags &= ~LoadMediaResource;
+            forgetResourceSpecificTracks();
             scheduleDelayedAction(LoadMediaResource);
         }
     } else if (name == controlsAttr) {
@@ -2986,35 +2988,6 @@ void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClientWithoutLockin
     }
 }
 
-void HTMLMediaElement::clearMediaPlayer(int flags)
-{
-    forgetResourceSpecificTracks();
-
-    closeMediaSource();
-
-    cancelDeferredLoad();
-
-    {
-        AudioSourceProviderClientLockScope scope(*this);
-        clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();
-    }
-
-    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;
-    if (mediaControls())
-        mediaControls()->refreshCastButtonVisibilityWithoutUpdate();
-
-    if (layoutObject())
-        layoutObject()->setShouldDoFullPaintInvalidation();
-}
-
 void HTMLMediaElement::stop()
 {
     WTF_LOG(Media, "HTMLMediaElement::stop(%p)", this);
@@ -3026,7 +2999,11 @@ void HTMLMediaElement::stop()
     m_asyncEventQueue->close();
 
     // Stop the playback without generating events
-    clearMediaPlayer(-1);
+    resetMediaPlayerAndMediaSource();
+    forgetResourceSpecificTracks();
+    m_loadTimer.stop();
+    m_pendingActionFlags = 0;
+    m_loadState = WaitingForSource;
     m_readyState = HAVE_NOTHING;
     m_readyStateMaximum = HAVE_NOTHING;
     setNetworkState(NETWORK_EMPTY);
@@ -3391,7 +3368,6 @@ void* HTMLMediaElement::preDispatchEventHandler(Event* event)
     return nullptr;
 }
 
-// TODO(srirama.m): Refactor this and clearMediaPlayer to the extent possible.
 void HTMLMediaElement::resetMediaPlayerAndMediaSource()
 {
     closeMediaSource();
@@ -3409,6 +3385,10 @@ void HTMLMediaElement::resetMediaPlayerAndMediaSource()
     if (m_audioSourceNode)
         audioSourceProvider().setClient(m_audioSourceNode);
 #endif
+    if (mediaControls())
+        mediaControls()->refreshCastButtonVisibilityWithoutUpdate();
+    if (layoutObject())
+        layoutObject()->setShouldDoFullPaintInvalidation();
 }
 
 #if ENABLE(WEB_AUDIO)