Chris Pearce - Bug 479711 - Media elements should delay the document 'load' event
authorChris Pearce <chris@pearce.org.nz>
Mon, 09 Mar 2009 10:01:03 +1300
changeset 25850 37b4cf75a6a52dd15f4850018bf30c5c4cdf71fc
parent 25849 0d2da3338359410ac77c111cadb8771e5c7d6209
child 25851 5e2e306f28030e6aa6b42f6d9d8252f58c1a0984
push idunknown
push userunknown
push dateunknown
bugs479711
milestone1.9.2a1pre
Chris Pearce - Bug 479711 - Media elements should delay the document 'load' event
content/html/content/public/nsHTMLMediaElement.h
content/html/content/src/nsHTMLMediaElement.cpp
content/media/video/test/Makefile.in
content/media/video/test/test_delay_load.html
layout/reftests/ogg-video/aspect-ratio-1b.html
layout/reftests/ogg-video/aspect-ratio-2a.html
layout/reftests/ogg-video/aspect-ratio-2b.html
layout/reftests/ogg-video/basic-1.html
layout/reftests/ogg-video/empty-1a.html
layout/reftests/ogg-video/empty-1b.html
layout/reftests/ogg-video/zoomed-1.html
--- a/content/html/content/public/nsHTMLMediaElement.h
+++ b/content/html/content/public/nsHTMLMediaElement.h
@@ -287,32 +287,42 @@ protected:
   nsresult LoadResource(nsIURI* aURI);
 
   /**
    * Selects the next <source> child from which to load a resource. Called
    * during the media selection algorithm.
    */
   already_AddRefed<nsIURI> GetNextSource();
 
+  /**
+   * Changes mDelayingLoadEvent, and will call BlockOnLoad()/UnblockOnLoad()
+   * on the owning document, so it can delay the load event firing.
+   */
+  void ChangeDelayLoadStatus(PRBool aDelay);
+
   nsRefPtr<nsMediaDecoder> mDecoder;
 
   nsCOMPtr<nsIChannel> mChannel;
 
   // Error attribute
   nsCOMPtr<nsIDOMHTMLMediaError> mError;
 
   // The current media load ID. This is incremented every time we start a
   // new load. Async events note the ID when they're first sent, and only fire
   // if the ID is unchanged when they come to fire.
   PRUint32 mCurrentLoadID;
 
   // Points to the child source elements, used to iterate through the children
   // when selecting a resource to load.
   nsCOMPtr<nsIDOMRange> mSourcePointer;
 
+  // Points to the document whose load we're blocking. This is the document
+  // we're bound to when loading starts.
+  nsCOMPtr<nsIDocument> mLoadBlockedDoc;
+
   // Media loading flags. See: 
   //   http://www.whatwg.org/specs/web-apps/current-work/#video)
   nsMediaNetworkState mNetworkState;
   nsMediaReadyState mReadyState;
 
   enum LoadAlgorithmState {
     // Not waiting for any src/<source>.
     NOT_WAITING,
@@ -387,9 +397,17 @@ protected:
   // PR_TRUE if we're in BindToTree().
   PRPackedBool mIsBindingToTree;
 
   // PR_TRUE if we're running the "load()" method.
   PRPackedBool mIsRunningLoadMethod;
 
   // PR_TRUE if we're loading exclusively from the src attribute's resource.
   PRPackedBool mIsLoadingFromSrcAttribute;
+
+  // PR_TRUE if we're delaying the "load" event. They are delayed until either
+  // an error occurs, or the first frame is loaded.
+  PRPackedBool mDelayingLoadEvent;
+
+  // PR_TRUE when we've got a task queued to call SelectResource(),
+  // or while we're running SelectResource().
+  PRPackedBool mIsRunningSelectResource;
 };
--- a/content/html/content/src/nsHTMLMediaElement.cpp
+++ b/content/html/content/src/nsHTMLMediaElement.cpp
@@ -139,30 +139,40 @@ public:
   }
 };
 
 class nsHTMLMediaElement::SelectResourceEvent : public nsMediaEvent {
 public:
   SelectResourceEvent(nsHTMLMediaElement *aElement)
     : nsMediaEvent(aElement) {}
   NS_IMETHOD Run() {
-    if (!IsCancelled())
+    if (!IsCancelled()) {
+      NS_ASSERTION(mElement->mIsRunningSelectResource,
+                   "Should have flagged that we're running SelectResource()");
       mElement->SelectResource();
+      mElement->mIsRunningSelectResource = PR_FALSE;
+    }
     return NS_OK;
   }
 };
 
 void nsHTMLMediaElement::QueueSelectResourceTask()
 {
+  // Don't allow multiple async select resource calls to be queued.
+  if (mIsRunningSelectResource)
+    return;
+  mIsRunningSelectResource = PR_TRUE;
+  ChangeDelayLoadStatus(PR_TRUE);
   nsCOMPtr<nsIRunnable> event = new SelectResourceEvent(this);
   NS_DispatchToMainThread(event);
 }
 
 void nsHTMLMediaElement::QueueLoadFromSourceTask()
 {
+  ChangeDelayLoadStatus(PR_TRUE);
   nsCOMPtr<nsIRunnable> event = new LoadNextSourceEvent(this);
   NS_DispatchToMainThread(event);
 }
 
 class nsHTMLMediaElement::MediaLoadListener : public nsIStreamListener
 {
   NS_DECL_ISUPPORTS
   NS_DECL_NSIREQUESTOBSERVER
@@ -232,16 +242,17 @@ NS_IMETHODIMP nsHTMLMediaElement::MediaL
 
 NS_IMPL_ADDREF_INHERITED(nsHTMLMediaElement, nsGenericHTMLElement)
 NS_IMPL_RELEASE_INHERITED(nsHTMLMediaElement, nsGenericHTMLElement)
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsHTMLMediaElement)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLMediaElement, nsGenericHTMLElement)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mSourcePointer)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mLoadBlockedDoc)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLMediaElement, nsGenericHTMLElement)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mSourcePointer)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(nsHTMLMediaElement)
 NS_INTERFACE_MAP_END_INHERITING(nsGenericHTMLElement)
@@ -332,23 +343,26 @@ void nsHTMLMediaElement::AbortExistingLo
   if (mNetworkState != nsIDOMHTMLMediaElement::NETWORK_EMPTY) {
     mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
     ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_NOTHING);
     mPaused = PR_TRUE;
 
     // TODO: The current playback position must be set to 0.
     DispatchSimpleEvent(NS_LITERAL_STRING("emptied"));
   }
+
+  mIsRunningSelectResource = PR_FALSE;
 }
 
 void nsHTMLMediaElement::NoSupportedMediaError()
 {
   mError = new nsHTMLMediaError(nsIDOMHTMLMediaError::MEDIA_ERR_NONE_SUPPORTED);
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE;
   DispatchAsyncProgressEvent(NS_LITERAL_STRING("error"));
+  ChangeDelayLoadStatus(PR_FALSE);
 }
 
 /* void load (); */
 NS_IMETHODIMP nsHTMLMediaElement::Load()
 {
   if (mIsRunningLoadMethod)
     return NS_OK;
   mIsRunningLoadMethod = PR_TRUE;
@@ -385,16 +399,17 @@ static PRBool HasPotentialResource(nsICo
 
 void nsHTMLMediaElement::SelectResource()
 {
   if (!HasPotentialResource(this)) {
     // While the media element has neither a src attribute nor any source
     // element children, wait. (This steps might wait forever.) 
     mNetworkState = nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE;
     mLoadWaitStatus = WAITING_FOR_SRC_OR_SOURCE;
+    ChangeDelayLoadStatus(PR_FALSE);
     return;
   }
 
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
   DispatchAsyncProgressEvent(NS_LITERAL_STRING("loadstart"));
 
   nsAutoString src;
   nsCOMPtr<nsIURI> uri;
@@ -421,16 +436,18 @@ void nsHTMLMediaElement::NotifyLoadError
     NoSupportedMediaError();
   } else {
     QueueLoadFromSourceTask();
   }
 }
 
 void nsHTMLMediaElement::LoadFromSourceChildren()
 {
+  NS_ASSERTION(!IsInDoc() || mDelayingLoadEvent,
+               "Should delay load event while loading in document");
   while (PR_TRUE) {
     nsresult rv;
     nsCOMPtr<nsIURI> uri = GetNextSource();
     if (!uri) {
       // Exhausted candidates, wait for more candidates to be appended to
       // the media element.
       mLoadWaitStatus = WAITING_FOR_SOURCE;
       NoSupportedMediaError();
@@ -445,16 +462,18 @@ void nsHTMLMediaElement::LoadFromSourceC
 
     // If we fail to load, loop back and try loading the next resource.
   }
   NS_NOTREACHED("Execution should not reach here!");
 }
 
 nsresult nsHTMLMediaElement::LoadResource(nsIURI* aURI)
 {
+  NS_ASSERTION(!IsInDoc() || mDelayingLoadEvent,
+               "Should delay load event while loading in document");
   nsresult rv;
 
   if (mChannel) {
     mChannel->Cancel(NS_BINDING_ABORTED);
     mChannel = nsnull;
   }
 
   PRInt16 shouldLoad = nsIContentPolicy::ACCEPT;
@@ -535,16 +554,18 @@ nsresult nsHTMLMediaElement::LoadWithCha
 {
   NS_ENSURE_ARG_POINTER(aChannel);
   NS_ENSURE_ARG_POINTER(aListener);
 
   *aListener = nsnull;
 
   AbortExistingLoads();
 
+  ChangeDelayLoadStatus(PR_TRUE);
+
   nsresult rv = InitializeDecoderForChannel(aChannel, aListener);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   mBegun = PR_TRUE;
 
   DispatchAsyncProgressEvent(NS_LITERAL_STRING("loadstart"));
@@ -694,17 +715,19 @@ nsHTMLMediaElement::nsHTMLMediaElement(n
     mPaused(PR_TRUE),
     mMuted(PR_FALSE),
     mIsDoneAddingChildren(!aFromParser),
     mPlayingBeforeSeek(PR_FALSE),
     mWaitingFired(PR_FALSE),
     mIsBindingToTree(PR_FALSE),
     mLoadWaitStatus(NOT_WAITING),
     mIsLoadingFromSrcAttribute(PR_FALSE),
-    mIsRunningLoadMethod(PR_FALSE)
+    mIsRunningLoadMethod(PR_FALSE),
+    mDelayingLoadEvent(PR_FALSE),
+    mIsRunningSelectResource(PR_FALSE)
 {
 }
 
 nsHTMLMediaElement::~nsHTMLMediaElement()
 {
   if (mDecoder) {
     mDecoder->Shutdown();
     mDecoder = nsnull;
@@ -826,17 +849,16 @@ nsresult nsHTMLMediaElement::BindToTree(
   return rv;
 }
 
 void nsHTMLMediaElement::UnbindFromTree(PRBool aDeep,
                                         PRBool aNullParent)
 {
   if (!mPaused && mNetworkState != nsIDOMHTMLMediaElement::NETWORK_EMPTY)
     Pause();
-
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 }
 
 #ifdef MOZ_OGG
 // See http://www.rfc-editor.org/rfc/rfc5334.txt for the definitions
 // of Ogg media types and codec types
 static const char gOggTypes[][16] = {
   "video/ogg",
@@ -1133,16 +1155,17 @@ void nsHTMLMediaElement::MetadataLoaded(
   ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_METADATA);
   DispatchAsyncSimpleEvent(NS_LITERAL_STRING("durationchange"));
   DispatchAsyncSimpleEvent(NS_LITERAL_STRING("loadedmetadata"));
 }
 
 void nsHTMLMediaElement::FirstFrameLoaded()
 {
   ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_CURRENT_DATA);
+  ChangeDelayLoadStatus(PR_FALSE);
 }
 
 void nsHTMLMediaElement::ResourceLoaded()
 {
   mBegun = PR_FALSE;
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADED;
   ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_ENOUGH_DATA);
   DispatchAsyncProgressEvent(NS_LITERAL_STRING("load"));
@@ -1150,16 +1173,17 @@ void nsHTMLMediaElement::ResourceLoaded(
 
 void nsHTMLMediaElement::NetworkError()
 {
   mError = new nsHTMLMediaError(nsIDOMHTMLMediaError::MEDIA_ERR_NETWORK);
   mBegun = PR_FALSE;
   DispatchAsyncProgressEvent(NS_LITERAL_STRING("error"));
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
   DispatchAsyncSimpleEvent(NS_LITERAL_STRING("emptied"));
+  ChangeDelayLoadStatus(PR_FALSE);
 }
 
 void nsHTMLMediaElement::PlaybackEnded()
 {
   NS_ASSERTION(mDecoder->IsEnded(), "Decoder fired ended, but not in ended state");
   mBegun = PR_FALSE;
   mPaused = PR_TRUE;
   DispatchAsyncSimpleEvent(NS_LITERAL_STRING("ended"));
@@ -1523,8 +1547,25 @@ already_AddRefed<nsIURI> nsHTMLMediaElem
       
       NewURIFromString(src, getter_AddRefs(uri));
       return uri.forget();
     }
   }
   NS_NOTREACHED("Execution should not reach here!");
   return nsnull;
 }
+
+void nsHTMLMediaElement::ChangeDelayLoadStatus(PRBool aDelay) {
+  if (mDelayingLoadEvent == aDelay)
+    return;
+
+  LOG(PR_LOG_DEBUG, ("ChangeDelayLoadStatus(%d) doc=0x%p", aDelay, mLoadBlockedDoc.get()));
+  mDelayingLoadEvent = aDelay;
+
+  if (aDelay) {
+    mLoadBlockedDoc = GetOwnerDoc();
+    mLoadBlockedDoc->BlockOnload();
+  } else {
+    NS_ASSERTION(mLoadBlockedDoc, "Need a doc to block on");
+    mLoadBlockedDoc->UnblockOnload(PR_FALSE);
+    mLoadBlockedDoc = nsnull;
+  }
+}
--- a/content/media/video/test/Makefile.in
+++ b/content/media/video/test/Makefile.in
@@ -67,16 +67,17 @@ ifdef MOZ_OGG
 		test_bug468190.html \
 		test_can_play_type_ogg.html \
 		test_contentDuration1.html \
 		test_contentDuration2.html \
 		test_contentDuration3.html \
 		test_contentDuration4.html \
 		test_contentDuration5.html \
 		test_contentDuration6.html \
+		test_delay_load.html \
 		test_duration1.html \
 		test_ended1.html \
 		test_ended2.html \
 		test_error_on_404.html \
 		test_onloadedmetadata.html \
 		test_load_candidates.html \
 		test_play.html \
 		test_progress1.html \
new file mode 100644
--- /dev/null
+++ b/content/media/video/test/test_delay_load.html
@@ -0,0 +1,104 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=479711
+-->
+<head>
+  <title>Test for Bug 479711</title>
+  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script>
+  
+  var gLoadedDataCount = 0;
+  
+  function log(msg) {
+    //document.getElementById('log').innerHTML += "<p>"+msg+"</p>";
+    //dump(msg + "\n");
+  }
+  
+  function loadeddata() {
+    gLoadedDataCount++;
+  }
+  
+  function loaded() {
+    is(gLoadedDataCount, 5, "onload was not delayed until after metadataloaded");
+    log("doc-loaded gLoadedDataCount = " + gLoadedDataCount);
+    SimpleTest.finish();
+  }
+  
+  addLoadEvent(loaded);
+  
+  SimpleTest.waitForExplicitFinish();
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=479711">Mozilla Bug 479711</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+
+
+<video src="320x240.ogv" onloadeddata="loadeddata();"></video>
+
+
+<div id="log" style="font-size: small;"></div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 479711 **/
+
+var gEventTypes = [ 'loadstart', 'progress', 'suspend', 'load', 'abort', 'error', 'emptied', 'stalled', 'play', 'pause', 'loadedmetadata', 'loadeddata', 'waiting', 'playing', 'canplay', 'canplaythrough', 'seeking', 'seeked', 'timeupdate', 'ended', 'ratechange', 'durationchange', 'volumechange' ];
+
+
+
+function listener(evt) {
+  log('event ' + evt.target.id + " " + evt.type);
+}
+
+function createVideo(id) {
+  var v = document.createElement("video");
+  v.id = id;
+  for (var i=0; i<gEventTypes.length; i++) {
+    var t = gEventTypes[i];
+    v.addEventListener(t, listener, false);
+  }
+  v.addEventListener('loadeddata', loadeddata, false);
+  v.src = "http://localhost:8888/tests/content/media/video/test/320x240.ogv";
+  return v;
+}
+
+
+// Load, add, then remove.
+var v1 = createVideo("v1");
+v1.load();
+document.body.appendChild(v1);
+v1.parentNode.removeChild(v1);
+
+// Load and add.
+var v2 = createVideo("v2");
+v2.load();
+document.body.appendChild(v2);
+
+// Load outside of doc.
+var v3 = createVideo("v3");
+v3.load();
+
+// Load and move to another document.
+netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+var v4 = createVideo("v4");
+v4.load(); // load started while in this document, this doc's load will block until
+            // the video's finished loading (in the other document).
+var gTestWindow = window.open("", "testWindow", "width=400,height=400");
+gTestWindow.document.body.appendChild(v4);
+v4.addEventListener('load',
+  function() {
+    netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+    gTestWindow.close();
+  }, false);
+  
+</script>
+</pre>
+</body>
+</html>
--- a/layout/reftests/ogg-video/aspect-ratio-1b.html
+++ b/layout/reftests/ogg-video/aspect-ratio-1b.html
@@ -1,8 +1,7 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait">
+<html>
 <body style="background:white;">
 <video id="v" src="black140x100.ogv"
-       style="width:140px; height:300px; position:relative; left:100px;"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+       style="width:140px; height:300px; position:relative; left:100px;"></video>
 </body>
 </html>
--- a/layout/reftests/ogg-video/aspect-ratio-2a.html
+++ b/layout/reftests/ogg-video/aspect-ratio-2a.html
@@ -1,8 +1,7 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait">
+<html>
 <body style="background:white;">
 <video id="v" src="black140x100.ogv"
-       style="width:680px; height:200px; position:relative; top:200px;"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+       style="width:680px; height:200px; position:relative; top:200px;"></video>
 </body>
 </html>
--- a/layout/reftests/ogg-video/aspect-ratio-2b.html
+++ b/layout/reftests/ogg-video/aspect-ratio-2b.html
@@ -1,8 +1,7 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait">
+<html>
 <body style="background:white;">
 <video id="v" src="black140x100.ogv"
-       style="width:280px; height:600px; position:relative; left:200px;"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+       style="width:280px; height:600px; position:relative; left:200px;"></video>
 </body>
 </html>
--- a/layout/reftests/ogg-video/basic-1.html
+++ b/layout/reftests/ogg-video/basic-1.html
@@ -1,7 +1,6 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait">
+<html>
 <body style="background:white;">
-<video id="v" src="black140x100.ogv"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+<video id="v" src="black140x100.ogv"></video>
 </body>
 </html>
--- a/layout/reftests/ogg-video/empty-1a.html
+++ b/layout/reftests/ogg-video/empty-1a.html
@@ -1,7 +1,6 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait">
+<html>
 <body style="background:white;">
-<video id="v" src="black140x100.ogv" style="width:0"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+<video id="v" src="black140x100.ogv" style="width:0"></video>
 </body>
 </html>
--- a/layout/reftests/ogg-video/empty-1b.html
+++ b/layout/reftests/ogg-video/empty-1b.html
@@ -1,7 +1,6 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait">
+<html>
 <body style="background:white;">
-<video id="v" src="black140x100.ogv" style="height:0"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+<video id="v" src="black140x100.ogv" style="height:0"></video>
 </body>
 </html>
--- a/layout/reftests/ogg-video/zoomed-1.html
+++ b/layout/reftests/ogg-video/zoomed-1.html
@@ -1,7 +1,6 @@
 <!DOCTYPE HTML>
-<html class="reftest-wait" reftest-zoom="1.5">
+<html reftest-zoom="1.5">
 <body style="background:white; margin:0">
-<video id="v" src="black140x100.ogv"
-       onloadeddata="document.documentElement.removeAttribute('class')"></video>
+<video id="v" src="black140x100.ogv"></video>
 </body>
 </html>