Bug 680321 - Media preload state should reset in resource selection algorithm. r=cpearce
authorDavid Seifried <david.c.seifried@gmail.com>
Mon, 20 Feb 2012 10:02:08 +1300
changeset 90067 b5386d7930bf00b58aef311854e6a2a9d7a8e5d9
parent 90066 5326871f69618196da075b760a2e10b4b26b5fe6
child 90068 c49a23c7084a586d6eb50c44834fa80df72c9c70
push id783
push userlsblakk@mozilla.com
push dateTue, 24 Apr 2012 17:33:42 +0000
treeherdermozilla-beta@11faed19f136 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs680321
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 680321 - Media preload state should reset in resource selection algorithm. r=cpearce
content/html/content/public/nsHTMLMediaElement.h
content/html/content/src/nsHTMLMediaElement.cpp
content/media/test/test_preload_actions.html
--- a/content/html/content/public/nsHTMLMediaElement.h
+++ b/content/html/content/public/nsHTMLMediaElement.h
@@ -452,16 +452,22 @@ protected:
   void QueueLoadFromSourceTask();
 
   /**
    * Runs the media resource selection algorithm.
    */
   void SelectResource();
 
   /**
+   * A wrapper function that allows us to cleanly reset flags after a call
+   * to SelectResource()
+   */
+  void SelectResourceWrapper();
+
+  /**
    * Asynchronously awaits a stable state, and then causes SelectResource()
    * to be run on the main thread's event loop.
    */
   void QueueSelectResourceTask();
 
   /**
    * The resource-fetch algorithm step of the load algorithm.
    */
@@ -734,16 +740,19 @@ protected:
   // True if we're delaying the "load" event. They are delayed until either
   // an error occurs, or the first frame is loaded.
   bool mDelayingLoadEvent;
 
   // True when we've got a task queued to call SelectResource(),
   // or while we're running SelectResource().
   bool mIsRunningSelectResource;
 
+  // True when we already have select resource call queued
+  bool mHaveQueuedSelectResource;
+
   // True if we suspended the decoder because we were paused,
   // preloading metadata is enabled, autoplay was not enabled, and we loaded
   // the first frame.
   bool mSuspendedAfterFirstFrame;
 
   // True if we are allowed to suspend the decoder because we were paused,
   // preloading metdata was enabled, autoplay was not enabled, and we loaded
   // the first frame.
--- a/content/html/content/src/nsHTMLMediaElement.cpp
+++ b/content/html/content/src/nsHTMLMediaElement.cpp
@@ -533,16 +533,17 @@ void nsHTMLMediaElement::AbortExistingLo
   }
 
   mError = nsnull;
   mLoadedFirstFrame = false;
   mAutoplaying = true;
   mIsLoadingFromSourceChildren = false;
   mSuspendedAfterFirstFrame = false;
   mAllowSuspendAfterFirstFrame = true;
+  mHaveQueuedSelectResource = false;
   mLoadIsSuspended = false;
   mSourcePointer = nsnull;
 
   // TODO: The playback rate must be set to the default playback rate.
 
   if (mNetworkState != nsIDOMHTMLMediaElement::NETWORK_EMPTY) {
     mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
     ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_NOTHING);
@@ -620,21 +621,21 @@ void nsHTMLMediaElement::QueueLoadFromSo
   ChangeDelayLoadStatus(true);
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
   AsyncAwaitStableState(this, &nsHTMLMediaElement::LoadFromSourceChildren);
 }
 
 void nsHTMLMediaElement::QueueSelectResourceTask()
 {
   // Don't allow multiple async select resource calls to be queued.
-  if (mIsRunningSelectResource)
+  if (mHaveQueuedSelectResource)
     return;
-  mIsRunningSelectResource = true;
+  mHaveQueuedSelectResource = true;
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE;
-  AsyncAwaitStableState(this, &nsHTMLMediaElement::SelectResource);
+  AsyncAwaitStableState(this, &nsHTMLMediaElement::SelectResourceWrapper);
 }
 
 /* void load (); */
 NS_IMETHODIMP nsHTMLMediaElement::Load()
 {
   if (mIsRunningLoadMethod)
     return NS_OK;
   SetPlayedOrSeeked(false);
@@ -653,69 +654,78 @@ static bool HasSourceChildren(nsIContent
     if (child->IsHTML(nsGkAtoms::source))
     {
       return true;
     }
   }
   return false;
 }
 
+void nsHTMLMediaElement::SelectResourceWrapper()
+{
+  SelectResource();
+  mIsRunningSelectResource = false;
+  mHaveQueuedSelectResource = false;
+}
+
 void nsHTMLMediaElement::SelectResource()
 {
   if (!HasAttr(kNameSpaceID_None, nsGkAtoms::src) && !HasSourceChildren(this)) {
     // The media element has neither a src attribute nor any source
     // element children, abort the load.
     mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
     // This clears mDelayingLoadEvent, so AddRemoveSelfReference will be called
     ChangeDelayLoadStatus(false);
-    mIsRunningSelectResource = false;
     return;
   }
 
   ChangeDelayLoadStatus(true);
 
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
   // Load event was delayed, and still is, so no need to call
   // AddRemoveSelfReference, since it must still be held
   DispatchAsyncEvent(NS_LITERAL_STRING("loadstart"));
 
+  // Delay setting mIsRunningSeletResource until after UpdatePreloadAction
+  // so that we don't lose our state change by bailing out of the preload
+  // state update
+  UpdatePreloadAction();
+  mIsRunningSelectResource = true;
+
   // If we have a 'src' attribute, use that exclusively.
   nsAutoString src;
   if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
     nsCOMPtr<nsIURI> uri;
     nsresult rv = NewURIFromString(src, getter_AddRefs(uri));
     if (NS_SUCCEEDED(rv)) {
       LOG(PR_LOG_DEBUG, ("%p Trying load from src=%s", this, NS_ConvertUTF16toUTF8(src).get()));
       NS_ASSERTION(!mIsLoadingFromSourceChildren,
         "Should think we're not loading from source children by default");
       mLoadingSrc = uri;
       if (mPreloadAction == nsHTMLMediaElement::PRELOAD_NONE) {
         // preload:none media, suspend the load here before we make any
         // network requests.
         SuspendLoad();
-        mIsRunningSelectResource = false;
         return;
       }
 
       rv = LoadResource();
       if (NS_SUCCEEDED(rv)) {
-        mIsRunningSelectResource = false;
         return;
       }
     } else {
       const PRUnichar* params[] = { src.get() };
       ReportLoadError("MediaLoadInvalidURI", params, ArrayLength(params));
     }
     NoSupportedMediaSourceError();
   } else {
     // Otherwise, the source elements will be used.
     mIsLoadingFromSourceChildren = true;
     LoadFromSourceChildren();
   }
-  mIsRunningSelectResource = false;
 }
 
 void nsHTMLMediaElement::NotifyLoadError()
 {
   if (!mIsLoadingFromSourceChildren) {
     LOG(PR_LOG_DEBUG, ("NotifyLoadError(), no supported media error"));
     NoSupportedMediaSourceError();
   } else if (mSourceLoadCandidate) {
@@ -1436,16 +1446,17 @@ nsHTMLMediaElement::nsHTMLMediaElement(a
     mMuted(false),
     mPlayingBeforeSeek(false),
     mPausedForInactiveDocument(false),
     mWaitingFired(false),
     mIsRunningLoadMethod(false),
     mIsLoadingFromSourceChildren(false),
     mDelayingLoadEvent(false),
     mIsRunningSelectResource(false),
+    mHaveQueuedSelectResource(false),
     mSuspendedAfterFirstFrame(false),
     mAllowSuspendAfterFirstFrame(true),
     mHasPlayedOrSeeked(false),
     mHasSelfReference(false),
     mShuttingDown(false),
     mLoadIsSuspended(false),
     mMediaSecurityVerified(false),
     mCORSMode(CORS_NONE)
--- a/content/media/test/test_preload_actions.html
+++ b/content/media/test/test_preload_actions.html
@@ -380,42 +380,42 @@ var tests = [
       v.addEventListener("loadstart", function(e){v._gotLoadStart = true;}, false);
       v.addEventListener("loadedmetadata", function(e){v._gotLoadedMetaData = true;}, false);
       v.addEventListener("canplaythrough", this.canplaythrough, false);
       v.src = test.name; // Causes implicit load.
       document.body.appendChild(v);
       v.preload = "metadata";
     },
   },
-  
   {
-    // 13. Change preload value from metadata to none after load started,
-    // should still load up to metadata, should not halt immediately.
-    loadeddata:
+    // 13. Change preload value from auto to none after specifying a src
+    // should load according to preload none, no buffering should have taken place
+    suspend:
     function(e) {
       var v = e.target;
       is(v._gotLoadStart, true, "(13) Must get loadstart.");
-      is(v._gotLoadedMetaData, true, "(13) Must get loadedmetadata.");
-      ok(v.readyState >= v.HAVE_CURRENT_DATA, "(13) ReadyState must be >= HAVE_CURRENT_DATA.");
-      is(v.networkState, v.NETWORK_IDLE, "(13) NetworkState must be NETWORK_IDLE.");
+      is(v._gotLoadedMetaData, false, "(13) Must not get loadedmetadata.");
+      is(v.readyState, v.HAVE_NOTHING, "(13) ReadyState must be HAVE_NOTHING");
+      is(v.networkState, v.NETWORK_IDLE, "(13) NetworkState must be NETWORK_IDLE");
       maybeFinish(v, 13);
     },
 
     setup:
     function(v) {
       v._gotLoadStart = false;
       v._gotLoadedMetaData = false;
-      v.preload = "metadata";
-      v.addEventListener("loadstart", function(e){v._gotLoadStart = true;}, false);
+      v.preload = "auto";
+      v.src = test.name;
+      v.preload = "none";
       v.addEventListener("loadedmetadata", function(e){v._gotLoadedMetaData = true;}, false);
-      v.addEventListener("loadeddata", this.loadeddata, false);
-      v.src = test.name; // Causes implicit load.
-      document.body.appendChild(v);
-      v.preload = "none";
-    },
+      v.addEventListener("loadstart", function(e){v._gotLoadStart = true;}, false);
+      v.addEventListener("suspend", this.suspend, false);
+      document.body.appendChild(v); // Causes implicit load, should load according to preload none
+      var s = document.createElement("source");
+    }
   },
   {
     // 14. Add preload:metadata video with src to document. Play(), should play through.
     loadeddata:
     function(e) {
       var v = e.target;
       is(v._gotLoadStart, true, "(14) Must get loadstart.");
       is(v._gotLoadedMetaData, true, "(14) Must get loadedmetadata.");
@@ -516,17 +516,45 @@ var tests = [
     setup:
     function(v) {
       v.addEventListener("ended", this.ended, false);
       v.preload = "none";
       v.src = test.name; // Schedules async section to continue load algorithm.
       document.body.appendChild(v);
       v.play(); // Should cause preload:none to be overridden.
     },  
+  },
+  {
+    // 19. Set preload='auto' on first video source then switching preload='none' and swapping the video source to another.
+    // The second video should not start playing as it's preload state has been changed to 'none' from 'auto'
+    loadedmetadata: function(e) {
+      var v = e.target;
+      is(v.preload === "auto", true, "(19) preload is initially auto");
+      setTimeout(function() {
+        // set preload state to none and switch video sources
+        v.preload="none";
+        v.src = test.name + "?asdf";
+        setTimeout(function() {
+          is(v.readyState === 0, true, "(19) no buffering has taken place");
+          maybeFinish(v, 19);
+        }, 2000);
+      }, 2000);
+
+    },
+
+    setup:
+    function(v) {
+      var that = this;
+      v.preload = "auto";
+      v.src = test.name;
+      // add a listener for when the video has loaded, so we know preload auto has worked
+      v.addEventListener( "loadedmetadata", this.loadedmetadata, false);
+      document.body.appendChild(v);
     }
+  }
 ];
 
 var iterationCount = 0;
 function startTest(test, token) {
   if (test == tests[0]) {
     ++iterationCount;
   }
   if (iterationCount == 2) {