Bug 1615261 - Fix ImageLoader setup for animated images in print preview. r=tnikkel

It's not clear to me this ever worked before either, I don't think the logic
before my patch was sound before.

But oh well. This should work, gotta add a test for it.

Differential Revision: https://phabricator.services.mozilla.com/D62777

layout/base/tests/chrome/printpreview_helper.xhtml

@@ -419,6 +419,20 @@ async function compareFiles(src1, src2) {
 // bug 1567105
 async function runTest11() {
   await compareFiles("printpreview_quirks.html", "printpreview_quirks_ref.html");
+  requestAnimationFrame(function() { setTimeout(runTest12); } );
+}
+
+// Crash test for bug 1615261
+async function runTest12() {
+  frameElts[0].contentDocument.body.innerHTML =
+    '<style> div { width: 100px; height: 100px; background-image: url("animated.gif"); } </style>' +
+    '<div>Firefox will crash if you try and print this page</div>';
+
+  // XXX Is there a more reliable way to wait for the background-image to load?
+  await new Promise(resolve => setTimeout(resolve, 500));
+
+  printpreview();
+  exitprintpreview();
   finish();
 }
 

layout/style/ImageLoader.cpp

@@ -399,7 +399,7 @@ static CORSMode EffectiveCorsMode(nsIURI* aURI,
 
 /* static */
 already_AddRefed<imgRequestProxy> ImageLoader::LoadImage(
-    const StyleComputedImageUrl& aImage, Document& aLoadingDoc) {
+    const StyleComputedImageUrl& aImage, Document& aDocument) {
   MOZ_ASSERT(NS_IsMainThread());
   nsIURI* uri = aImage.GetURI();
   if (!uri) {
@@ -412,9 +412,18 @@ already_AddRefed<imgRequestProxy> ImageLoader::LoadImage(
 
   const URLExtraData& data = aImage.ExtraData();
 
+  // NB: If aDocument is not the original document, we may not be able to load
+  // images from aDocument.  Instead we do the image load from the original
+  // doc and clone it to aDocument.
+  Document* loadingDoc = aDocument.GetOriginalDocument();
+  const bool isPrint = !!loadingDoc;
+  if (!loadingDoc) {
+    loadingDoc = &aDocument;
+  }
+
   RefPtr<imgRequestProxy> request;
   nsresult rv = nsContentUtils::LoadImage(
-      uri, &aLoadingDoc, &aLoadingDoc, data.Principal(), 0, data.ReferrerInfo(),
+      uri, loadingDoc, loadingDoc, data.Principal(), 0, data.ReferrerInfo(),
       sImageObserver, loadFlags, NS_LITERAL_STRING("css"),
       getter_AddRefs(request));
 
@@ -422,6 +431,28 @@ already_AddRefed<imgRequestProxy> ImageLoader::LoadImage(
     return nullptr;
   }
 
+  if (isPrint) {
+    RefPtr<imgRequestProxy> ret;
+    request->GetStaticRequest(&aDocument, getter_AddRefs(ret));
+    // Now we have a static image. If it is different from the one from the
+    // loading doc (that is, `request` is an animated image, and `ret` is a
+    // frozen version of it), we can forget about notifications from the
+    // animated image (assuming nothing else cares about it already).
+    //
+    // This is not technically needed for correctness, but helps keep the
+    // invariant that we only receive notifications for images that are in
+    // `sImages`.
+    if (ret != request) {
+      if (!sImages->Contains(request)) {
+        request->CancelAndForgetObserver(NS_BINDING_ABORTED);
+      }
+      if (!ret) {
+        return nullptr;
+      }
+      request = std::move(ret);
+    }
+  }
+
   sImages->LookupForAdd(request).OrInsert([] { return new ImageTableEntry(); });
   return request.forget();
 }

layout/style/nsStyleStruct.cpp

@@ -152,23 +152,7 @@ void StyleComputedUrl::ResolveImage(Document& aDocument,
       css::ImageLoader::NoteSharedLoad(request);
     }
   } else {
-    // NB: If aDocument is not the original document, we may not be able to load
+    request = css::ImageLoader::LoadImage(*this, aDocument);
-    // images from aDocument.  Instead we do the image load from the original
-    // doc and clone it to aDocument.
-    Document* loadingDoc = aDocument.GetOriginalDocument();
-    const bool isPrint = !!loadingDoc;
-    if (!loadingDoc) {
-      loadingDoc = &aDocument;
-    }
-
-    // Kick off the load in the loading document.
-    request = css::ImageLoader::LoadImage(*this, *loadingDoc);
-
-    if (isPrint && request) {
-      RefPtr<imgRequestProxy> ret;
-      request->GetStaticRequest(&aDocument, getter_AddRefs(ret));
-      request = std::move(ret);
-    }
   }
 
   if (!request) {