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);
+    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
     // 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);
+            m_pendingActionFlags &= ~LoadMediaResource;
+            forgetResourceSpecificTracks();
             scheduleDelayedAction(LoadMediaResource);
         }
     } else if (name == controlsAttr) {
         configureMediaControls();
     } else if (name == preloadAttr) {
         setPlayerPreload();
     } else {
         HTMLElement::parseAttribute(name, oldValue, value);
     }
 }
@@ skipping 2464 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)
-{
-    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);
 
     recordMetricsIfPausing();
 
     // Close the async event queue so that no events are enqueued.
     cancelPendingEventsAndCallbacks();
     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);
     setShouldDelayLoadEvent(false);
     m_currentSourceNode = nullptr;
     invalidateCachedTime();
     cueTimeline().updateActiveCues(0);
     m_playing = false;
     m_paused = true;
     m_seeking = false;
@@ skipping 344 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.
 void HTMLMediaElement::resetMediaPlayerAndMediaSource()
 {
     closeMediaSource();
 
     {
         AudioSourceProviderClientLockScope scope(*this);
         clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();
     }
 
     // We haven't yet found out if any remote routes are available.
     m_remoteRoutesAvailable = false;
     m_playingRemotely = false;
 
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode)
         audioSourceProvider().setClient(m_audioSourceNode);
 #endif
+    if (mediaControls())
+        mediaControls()->refreshCastButtonVisibilityWithoutUpdate();
+    if (layoutObject())
+        layoutObject()->setShouldDoFullPaintInvalidation();
 }
 
 #if ENABLE(WEB_AUDIO)
 void HTMLMediaElement::setAudioSourceNode(AudioSourceProviderClient* sourceNode)
 {
     ASSERT(isMainThread());
     m_audioSourceNode = sourceNode;
 
     AudioSourceProviderClientLockScope scope(*this);
     audioSourceProvider().setClient(m_audioSourceNode);
@@ skipping 210 matching lines @@
     visitor->trace(m_client);
 }
 
 DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)
 {
     visitor->trace(m_client);
 }
 #endif
 
 }