Bug 1118689 - Don't try to load an image if its container docshell is gone. r=bz, a=lmandel
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Fri, 13 Feb 2015 04:44:32 +0200
changeset 249780 878a4115c0b6219b1b74beb0bb44b5b062981885
parent 249779 8c0692b67d2d7611d4a249c8898730f8d284e052
child 249781 7917cdc3792549b7896ac0721f8b1e9bb84b263d
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, lmandel
bugs1118689
milestone37.0a2
Bug 1118689 - Don't try to load an image if its container docshell is gone. r=bz, a=lmandel
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/base/nsImageLoadingContent.cpp
dom/base/test/mochitest.ini
dom/base/test/test_bug1118689.html
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -2948,30 +2948,46 @@ nsContentUtils::IsInPrivateBrowsing(nsID
     }
   } else {
     nsCOMPtr<nsIChannel> channel = aDoc->GetChannel();
     isPrivate = channel && NS_UsePrivateBrowsing(channel);
   }
   return isPrivate;
 }
 
+bool
+nsContentUtils::DocumentInactiveForImageLoads(nsIDocument* aDocument)
+{
+  if (aDocument && !IsChromeDoc(aDocument) && !aDocument->IsResourceDoc()) {
+    nsCOMPtr<nsPIDOMWindow> win =
+      do_QueryInterface(aDocument->GetScopeObject());
+    return !win || !win->GetDocShell();
+  }
+  return false;
+}
+
 imgLoader*
 nsContentUtils::GetImgLoaderForDocument(nsIDocument* aDoc)
 {
+  NS_ENSURE_TRUE(!DocumentInactiveForImageLoads(aDoc), nullptr);
+
   if (!aDoc) {
     return imgLoader::Singleton();
   }
   bool isPrivate = IsInPrivateBrowsing(aDoc);
   return isPrivate ? imgLoader::PBSingleton() : imgLoader::Singleton();
 }
 
 // static
 imgLoader*
-nsContentUtils::GetImgLoaderForChannel(nsIChannel* aChannel)
-{
+nsContentUtils::GetImgLoaderForChannel(nsIChannel* aChannel,
+                                       nsIDocument* aContext)
+{
+  NS_ENSURE_TRUE(!DocumentInactiveForImageLoads(aContext), nullptr);
+
   if (!aChannel)
     return imgLoader::Singleton();
   nsCOMPtr<nsILoadContext> context;
   NS_QueryNotificationCallbacks(aChannel, context);
   return context && context->UsePrivateBrowsing() ? imgLoader::PBSingleton() : imgLoader::Singleton();
 }
 
 // static
@@ -3001,17 +3017,17 @@ nsContentUtils::LoadImage(nsIURI* aURI, 
   NS_PRECONDITION(aURI, "Must have a URI");
   NS_PRECONDITION(aLoadingDocument, "Must have a document");
   NS_PRECONDITION(aLoadingPrincipal, "Must have a principal");
   NS_PRECONDITION(aRequest, "Null out param");
 
   imgLoader* imgLoader = GetImgLoaderForDocument(aLoadingDocument);
   if (!imgLoader) {
     // nothing we can do here
-    return NS_OK;
+    return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsILoadGroup> loadGroup = aLoadingDocument->GetDocumentLoadGroup();
 
   nsIURI *documentURI = aLoadingDocument->GetDocumentURI();
 
   NS_ASSERTION(loadGroup || IsFontTableURI(documentURI),
                "Could not get loadgroup; onload may fire too early");
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -612,16 +612,21 @@ public:
   static bool CanLoadImage(nsIURI* aURI,
                            nsISupports* aContext,
                            nsIDocument* aLoadingDocument,
                            nsIPrincipal* aLoadingPrincipal,
                            int16_t* aImageBlockingStatus = nullptr,
                            uint32_t aContentPolicyType = nsIContentPolicy::TYPE_IMAGE);
 
   /**
+   * Returns true if objects in aDocument shouldn't initiate image loads.
+   */
+  static bool DocumentInactiveForImageLoads(nsIDocument* aDocument);
+
+  /**
    * Method to start an image load.  This does not do any security checks.
    * This method will attempt to make aURI immutable; a caller that wants to
    * keep a mutable version around should pass in a clone.
    *
    * @param aURI uri of the image to be loaded
    * @param aLoadingDocument the document we belong to
    * @param aLoadingPrincipal the principal doing the load
    * @param aReferrer the referrer URI
@@ -644,17 +649,18 @@ public:
                             imgRequestProxy** aRequest,
                             uint32_t aContentPolicyType = nsIContentPolicy::TYPE_IMAGE);
 
   /**
    * Obtain an image loader that respects the given document/channel's privacy status.
    * Null document/channel arguments return the public image loader.
    */
   static imgLoader* GetImgLoaderForDocument(nsIDocument* aDoc);
-  static imgLoader* GetImgLoaderForChannel(nsIChannel* aChannel);
+  static imgLoader* GetImgLoaderForChannel(nsIChannel* aChannel,
+                                           nsIDocument* aContext);
 
   /**
    * Returns whether the given URI is in the image cache.
    */
   static bool IsImageInCache(nsIURI* aURI, nsIDocument* aDocument);
 
   /**
    * Method to get an imgIContainer from an image loading content
--- a/dom/base/nsImageLoadingContent.cpp
+++ b/dom/base/nsImageLoadingContent.cpp
@@ -90,17 +90,17 @@ nsImageLoadingContent::nsImageLoadingCon
     mFireEventsOnDecode(false),
     mNewRequestsWillNeedAnimationReset(false),
     mStateChangerDepth(0),
     mCurrentRequestRegistered(false),
     mPendingRequestRegistered(false),
     mFrameCreateCalled(false),
     mVisibleCount(0)
 {
-  if (!nsContentUtils::GetImgLoaderForChannel(nullptr)) {
+  if (!nsContentUtils::GetImgLoaderForChannel(nullptr, nullptr)) {
     mLoadingEnabled = false;
   }
 }
 
 void
 nsImageLoadingContent::DestroyImageLoadingContent()
 {
   // Cancel our requests so they won't hold stale refs to us
@@ -342,17 +342,17 @@ nsImageLoadingContent::GetLoadingEnabled
 {
   *aLoadingEnabled = mLoadingEnabled;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsImageLoadingContent::SetLoadingEnabled(bool aLoadingEnabled)
 {
-  if (nsContentUtils::GetImgLoaderForChannel(nullptr)) {
+  if (nsContentUtils::GetImgLoaderForChannel(nullptr, nullptr)) {
     mLoadingEnabled = aLoadingEnabled;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsImageLoadingContent::GetImageBlockingStatus(int16_t* aStatus)
 {
@@ -624,17 +624,19 @@ nsImageLoadingContent::GetCurrentURI(nsI
   *aURI = GetCurrentURI(result).take();
   return result.ErrorCode();
 }
 
 already_AddRefed<nsIStreamListener>
 nsImageLoadingContent::LoadImageWithChannel(nsIChannel* aChannel,
                                             ErrorResult& aError)
 {
-  if (!nsContentUtils::GetImgLoaderForChannel(aChannel)) {
+  imgLoader* loader =
+    nsContentUtils::GetImgLoaderForChannel(aChannel, GetOurOwnerDoc());
+  if (!loader) {
     aError.Throw(NS_ERROR_NULL_POINTER);
     return nullptr;
   }
 
   nsCOMPtr<nsIDocument> doc = GetOurOwnerDoc();
   if (!doc) {
     // Don't bother
     return nullptr;
@@ -645,17 +647,17 @@ nsImageLoadingContent::LoadImageWithChan
   // XXX what about shouldProcess?
 
   // Our state might change. Watch it.
   AutoStateChanger changer(this, true);
 
   // Do the load.
   nsCOMPtr<nsIStreamListener> listener;
   nsRefPtr<imgRequestProxy>& req = PrepareNextRequest(eImageLoadType_Normal);
-  nsresult rv = nsContentUtils::GetImgLoaderForChannel(aChannel)->
+  nsresult rv = loader->
     LoadImageWithChannel(aChannel, this, doc,
                          getter_AddRefs(listener),
                          getter_AddRefs(req));
   if (NS_SUCCEEDED(rv)) {
     TrackImage(req);
     ResetAnimationIfNeeded();
   } else {
     MOZ_ASSERT(!req, "Shouldn't have non-null request here");
@@ -1185,16 +1187,21 @@ nsImageLoadingContent::StringToURI(const
                    charset.IsEmpty() ? nullptr : charset.get(),
                    baseURL,
                    nsContentUtils::GetIOService());
 }
 
 nsresult
 nsImageLoadingContent::FireEvent(const nsAString& aEventType)
 {
+  if (nsContentUtils::DocumentInactiveForImageLoads(GetOurOwnerDoc())) {
+    // Don't bother to fire any events, especially error events.
+    return NS_OK;
+  }
+
   // We have to fire the event asynchronously so that we won't go into infinite
   // loops in cases when onLoad handlers reset the src and the new src is in
   // cache.
 
   nsCOMPtr<nsINode> thisNode = do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
 
   nsRefPtr<AsyncEventDispatcher> loadBlockingAsyncDispatcher =
     new LoadBlockingAsyncEventDispatcher(thisNode, aEventType, false, false);
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -762,8 +762,10 @@ support-files = file_bug503473-frame.sjs
 skip-if = buildapp == 'b2g' || e10s
 support-files = file_bug1011748_redirect.sjs file_bug1011748_OK.sjs
 [test_bug1025933.html]
 [test_element.matches.html]
 [test_user_select.html]
 skip-if = buildapp == 'mulet' || buildapp == 'b2g' || toolkit == 'android'
 [test_bug1081686.html]
 skip-if = buildapp == 'b2g' || toolkit == 'android' || e10s
+[test_bug1118689.html]
+skip-if = buildapp == 'mulet' || buildapp == 'b2g'
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_bug1118689.html
@@ -0,0 +1,57 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1118689
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1118689</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 1118689 **/
+  SimpleTest.requestFlakyTimeout("Just need some random timeout.");
+
+  function test1() {
+    // test 1, check that error handling in data documents is still the same
+    //         as it has been for ages.
+    var d = document.implementation.createHTMLDocument();
+    d.body.innerHTML = "<img onerror='ok(false, \"EventHandler shouldn't be called in data document\")'>";
+    d.body.firstChild.addEventListener("error",
+      function() {
+        ok(true, "EventListener should be called in data document");
+        test2();
+      });
+    d.body.firstChild.addEventListener("load",
+      function() {
+        ok(false, "Images in data document shouldn't be loaded");
+      });
+    d.body.firstChild.src = "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP";
+  }
+
+  function test2() {
+    // test 2, check that load event doesn't keep up being dispatched if
+    //         window has been closed.
+    var win = window.open('data:text/html,<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP" onload="this.src = this.src">',
+                          "", "height=100,width=100");
+    setTimeout(function() {
+        win.close();
+        SimpleTest.finish();
+      }, 2500);
+  }
+
+  SimpleTest.waitForExplicitFinish();
+
+  </script>
+</head>
+<body onload="test1();">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1118689">Mozilla Bug 1118689</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>