Bug 493915. Fix leak in media element when an error occurs early in the resource load. r=cpearce

2009-07-10 14:03:03 +12:00 · 2009-07-10 14:03:03 +12:00 · 8657c138f2
--- a/content/html/content/src/nsHTMLMediaElement.cpp
+++ b/content/html/content/src/nsHTMLMediaElement.cpp
@ -172,6 +172,12 @@ void nsHTMLMediaElement::QueueLoadFromSourceTask()
  NS_DispatchToMainThread(event);
 }

+/**
+ * There is a reference cycle involving this class: MediaLoadListener
+ * holds a reference to the nsHTMLMediaElement, which holds a reference
+ * to an nsIChannel, which holds a reference to this listener.
+ * We break the reference cycle in OnStartRequest by clearing mElement.
+ */
 class nsHTMLMediaElement::MediaLoadListener : public nsIStreamListener,
                                              public nsIChannelEventSink,
                                              public nsIInterfaceRequestor
@ -200,30 +206,35 @@ NS_IMPL_ISUPPORTS4(nsHTMLMediaElement::MediaLoadListener, nsIRequestObserver,

 NS_IMETHODIMP nsHTMLMediaElement::MediaLoadListener::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext)
 {
+  // The element is only needed until we've had a chance to call
+  // InitializeDecoderForChannel. So make sure mElement is cleared here.
+  nsRefPtr<nsHTMLMediaElement> element;
+  element.swap(mElement);
+
  // Don't continue to load if the request failed or has been canceled.
  nsresult rv;
  nsresult status;
  rv = aRequest->GetStatus(&status);
  NS_ENSURE_SUCCESS(rv, rv);
  if (NS_FAILED(status)) {
-    if (mElement)
-      mElement->NotifyLoadError();
+    if (element)
+      element->NotifyLoadError();
    return status;
  }

  nsCOMPtr<nsIChannel> channel = do_QueryInterface(aRequest);
  if (channel &&
-      mElement &&
+      element &&
      NS_SUCCEEDED(rv = mElement->InitializeDecoderForChannel(channel, getter_AddRefs(mNextListener))) &&
      mNextListener) {
    rv = mNextListener->OnStartRequest(aRequest, aContext);
  } else {
    // If InitializeDecoderForChannel() returned an error, fire a network
    // error.
-    if (NS_FAILED(rv) && !mNextListener && mElement) {
+    if (NS_FAILED(rv) && !mNextListener && element) {
      // Load failed, attempt to load the next candidate resource. If there
      // are none, this will trigger a MEDIA_ERR_NONE_SUPPORTED error.
-      mElement->NotifyLoadError();
+      element->NotifyLoadError();
    }
    // If InitializeDecoderForChannel did not return a listener (but may
    // have otherwise succeeded), we abort the connection since we aren't
@ -231,10 +242,6 @@ NS_IMETHODIMP nsHTMLMediaElement::MediaLoadListener::OnStartRequest(nsIRequest*
    rv = NS_BINDING_ABORTED;
  }

-  // The element is only needed until we've had a chance to call
-  // InitializeDecoderForChannel.
-  mElement = nsnull;
-
  return rv;
 }

--- a/content/media/test/crashtests/493915-1.html
+++ b/content/media/test/crashtests/493915-1.html
@ -0,0 +1,18 @@
+<html>
+<head>
+<script type="text/javascript">
+
+function boom()
+{
+  s = document.createElement("span");
+  a = document.createElement("audio");
+  a['src'] = "javascript:4";
+  a['loopend'] = 3;
+  s.appendChild(a);
+}
+
+</script>
+</head>
+
+<body onload="boom();"></body>
+</html>
--- a/content/media/test/crashtests/crashtests.list
+++ b/content/media/test/crashtests/crashtests.list
@ -1,3 +1,4 @@
 load 468763-1.html
 load 474744-1.html
+load 493915-1.html
 load 495794-1.html