Bug 680321 - Media preload state should reset in resource selection algorithm. r=cpearce

content/html/content/public/nsHTMLMediaElement.h

@@ -456,6 +456,12 @@ protected:
    */
   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.
@@ -739,6 +745,9 @@ protected:
   // 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.

content/html/content/src/nsHTMLMediaElement.cpp

@@ -538,6 +538,7 @@ void nsHTMLMediaElement::AbortExistingLoads()
   mIsLoadingFromSourceChildren = false;
   mSuspendedAfterFirstFrame = false;
   mAllowSuspendAfterFirstFrame = true;
+  mHaveQueuedSelectResource = false;
   mLoadIsSuspended = false;
   mSourcePointer = nsnull;
 
@@ -625,11 +626,11 @@ void nsHTMLMediaElement::QueueLoadFromSourceTask()
 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 (); */
@@ -658,6 +659,13 @@ static bool HasSourceChildren(nsIContent *aElement)
   return false;
 }
 
+void nsHTMLMediaElement::SelectResourceWrapper()
+{
+  SelectResource();
+  mIsRunningSelectResource = false;
+  mHaveQueuedSelectResource = false;
+}
+
 void nsHTMLMediaElement::SelectResource()
 {
   if (!HasAttr(kNameSpaceID_None, nsGkAtoms::src) && !HasSourceChildren(this)) {
@@ -666,7 +674,6 @@ void nsHTMLMediaElement::SelectResource()
     mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
     // This clears mDelayingLoadEvent, so AddRemoveSelfReference will be called
     ChangeDelayLoadStatus(false);
-    mIsRunningSelectResource = false;
     return;
   }
 
@@ -677,6 +684,12 @@ void nsHTMLMediaElement::SelectResource()
   // 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)) {
@@ -691,13 +704,11 @@ void nsHTMLMediaElement::SelectResource()
         // 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 {
@@ -710,7 +721,6 @@ void nsHTMLMediaElement::SelectResource()
     mIsLoadingFromSourceChildren = true;
     LoadFromSourceChildren();
   }
-  mIsRunningSelectResource = false;
 }
 
 void nsHTMLMediaElement::NotifyLoadError()
@@ -1441,6 +1451,7 @@ nsHTMLMediaElement::nsHTMLMediaElement(already_AddRefed<nsINodeInfo> aNodeInfo)
     mIsLoadingFromSourceChildren(false),
     mDelayingLoadEvent(false),
     mIsRunningSelectResource(false),
+    mHaveQueuedSelectResource(false),
     mSuspendedAfterFirstFrame(false),
     mAllowSuspendAfterFirstFrame(true),
     mHasPlayedOrSeeked(false),

content/media/test/test_preload_actions.html

@@ -385,17 +385,16 @@ var tests = [
       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);
     },
 
@@ -403,14 +402,15 @@ var tests = [
     function(v) {
       v._gotLoadStart = false;
       v._gotLoadedMetaData = false;
-      v.preload = "metadata";
-      v.addEventListener("loadstart", function(e){v._gotLoadStart = true;}, false);
-      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 = "auto";
+      v.src = test.name;
       v.preload = "none";
-    },
+      v.addEventListener("loadedmetadata", function(e){v._gotLoadedMetaData = true;}, false);
+      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.
@@ -521,7 +521,35 @@ var tests = [
       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;