Bug 1648064 - Switch DOM images to work like CSS images for the purposes of printing. r=tnikkel,smaug

Make them perform the image load (if needed), instead of copying the
image requests from the original document.

This is needed for CSS for stuff like:

@media print {
  #foo::before {
    content: url(bar.png);
  }
}

And so on. For images, we should do this as well. Nothing prevents you
from doing:

  <picture>
    <source srcset="print.png" media="print">
    <source srcset="screen.png" media="not print">
    <img>
  </picture>

And that should in theory work. It works after this patch, and I added a
test for that.

This patch is a bit bigger than I'd like, but I didn't find a more
reasonable way to split it up.

Making static docs able to do image loads is most of the patch and is
mostly straight-forward. This allows to remove the hacky "change the
loading document" thing that CSS images do, which is just working around
the CSP of the print document.

I need to enable background colors in printpreview_helper so as to be
able to have a reference page for all the different image types.

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

dom/base/Document.h

@@ -2627,7 +2627,8 @@ class Document : public nsINode,
   bool ShouldLoadImages() const {
     // We check IsBeingUsedAsImage() so that SVG documents loaded as
     // images can themselves have data: URL image references.
-    return IsCurrentActiveDocument() || IsBeingUsedAsImage();
+    return IsCurrentActiveDocument() || IsBeingUsedAsImage() ||
+           IsStaticDocument();
   }
 
   /**

dom/base/nsContentUtils.cpp

@@ -3298,13 +3298,19 @@ bool nsContentUtils::IsInPrivateBrowsing(nsILoadGroup* aLoadGroup) {
   return isPrivate;
 }
 
+// FIXME(emilio): This is (effectively) almost but not quite the same as
+// Document::ShouldLoadImages(), which one is right?
 bool nsContentUtils::DocumentInactiveForImageLoads(Document* aDocument) {
-  if (aDocument && !IsChromeDoc(aDocument) && !aDocument->IsResourceDoc()) {
-    nsCOMPtr<nsPIDOMWindowInner> win =
-        do_QueryInterface(aDocument->GetScopeObject());
-    return !win || !win->GetDocShell();
+  if (!aDocument) {
+    return false;
   }
-  return false;
+  if (IsChromeDoc(aDocument) || aDocument->IsResourceDoc() ||
+      aDocument->IsStaticDocument()) {
+    return false;
+  }
+  nsCOMPtr<nsPIDOMWindowInner> win =
+      do_QueryInterface(aDocument->GetScopeObject());
+  return !win || !win->GetDocShell();
 }
 
 imgLoader* nsContentUtils::GetImgLoaderForDocument(Document* aDoc) {
@@ -3442,15 +3448,6 @@ already_AddRefed<imgIContainer> nsContentUtils::GetImageFromContent(
   return imgContainer.forget();
 }
 
-// static
-already_AddRefed<imgRequestProxy> nsContentUtils::GetStaticRequest(
-    Document* aLoadingDocument, imgRequestProxy* aRequest) {
-  NS_ENSURE_TRUE(aRequest, nullptr);
-  RefPtr<imgRequestProxy> retval;
-  aRequest->GetStaticRequest(aLoadingDocument, getter_AddRefs(retval));
-  return retval.forget();
-}
-
 // static
 bool nsContentUtils::ContentIsDraggable(nsIContent* aContent) {
   MOZ_ASSERT(aContent);

dom/base/nsContentUtils.h

@@ -936,12 +936,6 @@ class nsContentUtils {
   static already_AddRefed<imgIContainer> GetImageFromContent(
       nsIImageLoadingContent* aContent, imgIRequest** aRequest = nullptr);
 
-  /**
-   * Helper method to call imgIRequest::GetStaticRequest.
-   */
-  static already_AddRefed<imgRequestProxy> GetStaticRequest(
-      Document* aLoadingDocument, imgRequestProxy* aRequest);
-
   /**
    * Method that decides whether a content node is draggable
    *

dom/base/nsDataDocumentContentPolicy.cpp

@@ -75,11 +75,29 @@ nsDataDocumentContentPolicy::ShouldLoad(nsIURI* aContentLocation,
     return NS_OK;
   }
 
-  // Nothing else is OK to load for data documents
   if (doc->IsLoadedAsData()) {
-    // ...but let static (print/print preview) documents to load fonts.
-    if (!doc->IsStaticDocument() ||
-        contentType != nsIContentPolicy::TYPE_FONT) {
+    bool allowed = [&] {
+      if (!doc->IsStaticDocument()) {
+        // If not a print/print preview doc, then nothing else is allowed for
+        // data documents.
+        return false;
+      }
+      // Let static (print/print preview) documents to load fonts and
+      // images.
+      switch (contentType) {
+        case nsIContentPolicy::TYPE_IMAGE:
+        case nsIContentPolicy::TYPE_IMAGESET:
+        case nsIContentPolicy::TYPE_FONT:
+        // This one is a bit sketchy, but nsObjectLoadingContent takes care of
+        // only getting here if it is an image.
+        case nsIContentPolicy::TYPE_OBJECT:
+          return true;
+        default:
+          return false;
+      }
+    }();
+
+    if (!allowed) {
       *aDecision = nsIContentPolicy::REJECT_TYPE;
       return NS_OK;
     }

dom/base/nsImageLoadingContent.cpp

@@ -976,8 +976,8 @@ nsImageLoadingContent::LoadImageWithChannel(nsIChannel* aChannel,
 
   // Do the load.
   RefPtr<imgRequestProxy>& req = PrepareNextRequest(eImageLoadType_Normal);
-  nsresult rv = loader->LoadImageWithChannel(aChannel, this, doc, aListener,
-                                             getter_AddRefs(req));
+  nsresult rv = loader->LoadImageWithChannel(aChannel, this, doc,
+                                             aListener, getter_AddRefs(req));
   if (NS_SUCCEEDED(rv)) {
     CloneScriptedRequests(req);
     TrackImage(req);
@@ -1103,7 +1103,12 @@ nsresult nsImageLoadingContent::LoadImage(nsIURI* aNewURI, bool aForce,
 
   // Data documents, or documents from DOMParser shouldn't perform image
   // loading.
-  if (aDocument->IsLoadedAsData()) {
+  //
+  // FIXME(emilio): Shouldn't this check be part of
+  // Document::ShouldLoadImages()? Or alternatively check ShouldLoadImages here
+  // instead? (It seems we only check ShouldLoadImages in HTMLImageElement,
+  // which seems wrong...)
+  if (aDocument->IsLoadedAsData() && !aDocument->IsStaticDocument()) {
     // This is the only codepath on which we can reach SetBlockedRequest while
     // our pending request exists.  Just clear it out here if we do have one.
     ClearPendingRequest(NS_BINDING_ABORTED, Some(OnNonvisible::DiscardImages));
@@ -1734,26 +1739,6 @@ void nsImageLoadingContent::UntrackImage(
   }
 }
 
-void nsImageLoadingContent::CreateStaticImageClone(
-    nsImageLoadingContent* aDest) const {
-  aDest->ClearScriptedRequests(CURRENT_REQUEST, NS_BINDING_ABORTED);
-  aDest->mCurrentRequest = nsContentUtils::GetStaticRequest(
-      aDest->GetOurOwnerDoc(), mCurrentRequest);
-  if (aDest->mCurrentRequest) {
-    aDest->CloneScriptedRequests(aDest->mCurrentRequest);
-  }
-  aDest->TrackImage(aDest->mCurrentRequest);
-  aDest->mForcedImageState = mForcedImageState;
-  aDest->mImageBlockingStatus = mImageBlockingStatus;
-  aDest->mLoadingEnabled = mLoadingEnabled;
-  aDest->mStateChangerDepth = mStateChangerDepth;
-  aDest->mIsImageStateForced = mIsImageStateForced;
-  aDest->mLoading = mLoading;
-  aDest->mBroken = mBroken;
-  aDest->mUserDisabled = mUserDisabled;
-  aDest->mSuppressed = mSuppressed;
-}
-
 CORSMode nsImageLoadingContent::GetCORSMode() { return CORS_NONE; }
 
 nsImageLoadingContent::ImageObserver::ImageObserver(

dom/base/nsImageLoadingContent.h

@@ -373,8 +373,6 @@ class nsImageLoadingContent : public nsIImageLoadingContent {
   nsresult StringToURI(const nsAString& aSpec,
                        mozilla::dom::Document* aDocument, nsIURI** aURI);
 
-  void CreateStaticImageClone(nsImageLoadingContent* aDest) const;
-
   /**
    * Prepare and returns a reference to the "next request". If there's already
    * a _usable_ current request (one with SIZE_AVAILABLE), this request is

dom/base/nsObjectLoadingContent.cpp

@@ -1837,10 +1837,21 @@ nsresult nsObjectLoadingContent::LoadObject(bool aNotify, bool aForceLoad,
   // XXX(johns): In these cases, we refuse to touch our content and just
   //   remain unloaded, as per legacy behavior. It would make more sense to
   //   load fallback content initially and refuse to ever change state again.
-  if (doc->IsBeingUsedAsImage() || doc->IsLoadedAsData()) {
+  if (doc->IsBeingUsedAsImage()) {
     return NS_OK;
   }
 
+  if (doc->IsLoadedAsData() && !doc->IsStaticDocument()) {
+    return NS_OK;
+  }
+  if (doc->IsStaticDocument()) {
+    // We only allow image loads in static documents, but we need to let the
+    // eType_Loading state go through too while we do so.
+    if (mType != eType_Image && mType != eType_Loading) {
+      return NS_OK;
+    }
+  }
+
   LOG(("OBJLC [%p]: LoadObject called, notify %u, forceload %u, channel %p",
        this, aNotify, aForceLoad, aLoadingChannel));
 
@@ -2590,8 +2601,6 @@ nsPluginFrame* nsObjectLoadingContent::GetExistingFrame() {
 
 void nsObjectLoadingContent::CreateStaticClone(
     nsObjectLoadingContent* aDest) const {
-  nsImageLoadingContent::CreateStaticImageClone(aDest);
-
   aDest->mType = mType;
   nsObjectLoadingContent* thisObj = const_cast<nsObjectLoadingContent*>(this);
   if (thisObj->mPrintFrame.IsAlive()) {

dom/html/HTMLImageElement.cpp

@@ -735,32 +735,25 @@ uint32_t HTMLImageElement::NaturalWidth() {
 }
 
 nsresult HTMLImageElement::CopyInnerTo(HTMLImageElement* aDest) {
-  bool destIsStatic = aDest->OwnerDoc()->IsStaticDocument();
-  if (destIsStatic) {
-    CreateStaticImageClone(aDest);
-  }
-
   nsresult rv = nsGenericHTMLElement::CopyInnerTo(aDest);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  if (!destIsStatic) {
-    // In SetAttr (called from nsGenericHTMLElement::CopyInnerTo), aDest skipped
-    // doing the image load because we passed in false for aNotify.  But we
-    // really do want it to do the load, so set it up to happen once the cloning
-    // reaches a stable state.
-    if (!aDest->InResponsiveMode() &&
-        aDest->HasAttr(kNameSpaceID_None, nsGkAtoms::src) &&
-        aDest->ShouldLoadImage()) {
-      // Mark channel as urgent-start before load image if the image load is
-      // initaiated by a user interaction.
-      mUseUrgentStartForChannel = UserActivation::IsHandlingUserInput();
+  // In SetAttr (called from nsGenericHTMLElement::CopyInnerTo), aDest skipped
+  // doing the image load because we passed in false for aNotify.  But we
+  // really do want it to do the load, so set it up to happen once the cloning
+  // reaches a stable state.
+  if (!aDest->InResponsiveMode() &&
+      aDest->HasAttr(kNameSpaceID_None, nsGkAtoms::src) &&
+      aDest->ShouldLoadImage()) {
+    // Mark channel as urgent-start before load image if the image load is
+    // initaiated by a user interaction.
+    mUseUrgentStartForChannel = UserActivation::IsHandlingUserInput();
 
-      nsContentUtils::AddScriptRunner(NewRunnableMethod<bool>(
-          "dom::HTMLImageElement::MaybeLoadImage", aDest,
-          &HTMLImageElement::MaybeLoadImage, false));
-    }
+    nsContentUtils::AddScriptRunner(NewRunnableMethod<bool>(
+        "dom::HTMLImageElement::MaybeLoadImage", aDest,
+        &HTMLImageElement::MaybeLoadImage, false));
   }
 
   return NS_OK;

dom/html/HTMLInputElement.cpp

@@ -1088,9 +1088,6 @@ nsresult HTMLInputElement::Clone(dom::NodeInfo* aNodeInfo,
       }
       break;
     case VALUE_MODE_DEFAULT:
-      if (mType == NS_FORM_INPUT_IMAGE && it->OwnerDoc()->IsStaticDocument()) {
-        CreateStaticImageClone(it);
-      }
       break;
   }
 

dom/svg/SVGImageElement.cpp

@@ -306,12 +306,5 @@ SVGElement::StringAttributesInfo SVGImageElement::GetStringInfo() {
                               ArrayLength(sStringInfo));
 }
 
-nsresult SVGImageElement::CopyInnerTo(Element* aDest) {
-  if (aDest->OwnerDoc()->IsStaticDocument()) {
-    CreateStaticImageClone(static_cast<SVGImageElement*>(aDest));
-  }
-  return SVGImageElementBase::CopyInnerTo(aDest);
-}
-
 }  // namespace dom
 }  // namespace mozilla

dom/svg/SVGImageElement.h

@@ -83,8 +83,6 @@ class SVGImageElement : public SVGImageElementBase,
 
   virtual nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
 
-  nsresult CopyInnerTo(mozilla::dom::Element* aDest);
-
   void MaybeLoadSVGImage();
 
   // WebIDL

image/imgLoader.cpp

@@ -5,6 +5,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // Undefine windows version of LoadImage because our code uses that name.
+#include "mozilla/ScopeExit.h"
 #undef LoadImage
 
 #include "imgLoader.h"
@@ -1697,7 +1698,7 @@ bool imgLoader::ValidateRequestWithNewChannel(
       validator->AddProxy(proxy);
     }
 
-    return NS_SUCCEEDED(rv);
+    return true;
   }
   // We will rely on Necko to cache this request when it's possible, and to
   // tell imgCacheValidator::OnStartRequest whether the request came from its
@@ -2125,6 +2126,29 @@ imgLoader::LoadImageXPCOM(
   return rv;
 }
 
+static void MakeRequestStaticIfNeeded(
+    Document* aLoadingDocument,
+    imgRequestProxy** aProxyAboutToGetReturned) {
+  if (!aLoadingDocument || !aLoadingDocument->IsStaticDocument()) {
+    return;
+  }
+
+  if (!*aProxyAboutToGetReturned) {
+    return;
+  }
+
+  RefPtr<imgRequestProxy> proxy = dont_AddRef(*aProxyAboutToGetReturned);
+  *aProxyAboutToGetReturned = nullptr;
+
+  RefPtr<imgRequestProxy> staticProxy =
+      proxy->GetStaticRequest(aLoadingDocument);
+  if (staticProxy != proxy) {
+    proxy->CancelAndForgetObserver(NS_BINDING_ABORTED);
+    proxy = std::move(staticProxy);
+  }
+  proxy.forget(aProxyAboutToGetReturned);
+}
+
 nsresult imgLoader::LoadImage(
     nsIURI* aURI, nsIURI* aInitialDocumentURI, nsIReferrerInfo* aReferrerInfo,
     nsIPrincipal* aTriggeringPrincipal, uint64_t aRequestContextID,
@@ -2141,6 +2165,10 @@ nsresult imgLoader::LoadImage(
     return NS_ERROR_NULL_POINTER;
   }
 
+  auto makeStaticIfNeeded = mozilla::MakeScopeExit([&] {
+    MakeRequestStaticIfNeeded(aLoadingDocument, _retval);
+  });
+
 #ifdef MOZ_GECKO_PROFILER
   AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING("imgLoader::LoadImage", NETWORK,
                                         aURI->GetSpecOrDefault());
@@ -2499,6 +2527,10 @@ nsresult imgLoader::LoadImageWithChannel(nsIChannel* channel,
 
   MOZ_ASSERT(NS_UsePrivateBrowsing(channel) == mRespectPrivacy);
 
+  auto makeStaticIfNeeded = mozilla::MakeScopeExit([&] {
+    MakeRequestStaticIfNeeded(aLoadingDocument, _retval);
+  });
+
   LOG_SCOPE(gImgLog, "imgLoader::LoadImageWithChannel");
   RefPtr<imgRequest> request;
 

image/imgRequestProxy.cpp

@@ -1053,29 +1053,21 @@ void imgRequestProxy::NullOutListener() {
 
 NS_IMETHODIMP
 imgRequestProxy::GetStaticRequest(imgIRequest** aReturn) {
-  imgRequestProxy* proxy;
-  nsresult result = GetStaticRequest(nullptr, &proxy);
-  *aReturn = proxy;
-  return result;
+  RefPtr<imgRequestProxy> proxy =
+      GetStaticRequest(static_cast<Document*>(nullptr));
+  proxy.forget(aReturn);
+  return NS_OK;
 }
 
-nsresult imgRequestProxy::GetStaticRequest(Document* aLoadingDocument,
-                                           imgRequestProxy** aReturn) {
-  *aReturn = nullptr;
+already_AddRefed<imgRequestProxy> imgRequestProxy::GetStaticRequest(
+    Document* aLoadingDocument) {
+  MOZ_DIAGNOSTIC_ASSERT(!aLoadingDocument || aLoadingDocument->IsStaticDocument());
   RefPtr<Image> image = GetImage();
 
   bool animated;
   if (!image || (NS_SUCCEEDED(image->GetAnimated(&animated)) && !animated)) {
     // Early exit - we're not animated, so we don't have to do anything.
-    NS_ADDREF(*aReturn = this);
-    return NS_OK;
-  }
-
-  // Check for errors in the image. Callers code rely on GetStaticRequest
-  // failing in this case, though with FrozenImage there's no technical reason
-  // for it anymore.
-  if (image->HasError()) {
-    return NS_ERROR_FAILURE;
+    return do_AddRef(this);
   }
 
   // We are animated. We need to create a frozen version of this image.
@@ -1090,9 +1082,7 @@ nsresult imgRequestProxy::GetStaticRequest(Document* aLoadingDocument,
       frozenImage, currentPrincipal, hadCrossOriginRedirects);
   req->Init(nullptr, nullptr, aLoadingDocument, mURI, nullptr);
 
-  NS_ADDREF(*aReturn = req);
-
-  return NS_OK;
+  return req.forget();
 }
 
 void imgRequestProxy::NotifyListener() {

image/imgRequestProxy.h

@@ -123,8 +123,7 @@ class imgRequestProxy : public mozilla::PreloaderBase,
                      Document* aLoadingDocument, imgRequestProxy** aClone);
   nsresult Clone(imgINotificationObserver* aObserver,
                  Document* aLoadingDocument, imgRequestProxy** aClone);
-  nsresult GetStaticRequest(Document* aLoadingDocument,
-                            imgRequestProxy** aReturn);
+  already_AddRefed<imgRequestProxy> GetStaticRequest(Document* aLoadingDocument);
 
   imgRequest* GetOwner() const;
 

layout/base/tests/chrome/chrome.ini

@@ -22,6 +22,8 @@ support-files =
   printpreview_font_mozprintcallback_ref.html
   printpreview_quirks.html
   printpreview_quirks_ref.html
+  printpreview_images.html
+  printpreview_images_ref.html
   test_document_adopted_styles.html
   test_document_adopted_styles_ref.html
   test_shadow_root_adopted_styles.html

layout/base/tests/chrome/printpreview_helper.xhtml

@@ -48,6 +48,7 @@ function printpreview(hasMozPrintCallback) {
   var settings = Cc["@mozilla.org/gfx/printsettings-service;1"]
                        .getService(Ci.nsIPrintSettingsService).globalPrintSettings;
   settings.showPrintProgress = false;
+  settings.printBGColors = true;
   var before = 0;
   var after = 0;
   function beforeprint() { ++before; }
@@ -435,7 +436,7 @@ async function runTest14() {
 // Crash test for bug 1615261
 async function runTest15() {
   frameElts[0].contentDocument.body.innerHTML =
-    '<style> div { width: 100px; height: 100px; background-image: url("animated.gif"); } </style>' +
+    '<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?
@@ -443,6 +444,13 @@ async function runTest15() {
 
   printpreview();
   exitprintpreview();
+
+  requestAnimationFrame(function() { setTimeout(runTest16); } );
+}
+
+// Various image tests.
+async function runTest16() {
+  await compareFiles("printpreview_images.html", "printpreview_images_ref.html");
   finish();
 }
 

layout/base/tests/chrome/printpreview_images.html

@@ -0,0 +1,25 @@
+<!doctype html>
+<style>
+  img, object, svg, input { display: block }
+  div {
+    content: url(blue-32x32.png);
+    width: 32px;
+    height: 32px;
+  }
+</style>
+<div></div>
+<picture>
+  <source srcset="blue-32x32.png">
+  <img width=32 height=32>
+</picture>
+<picture>
+  <source srcset="blue-32x32.png" media="print">
+  <source srcset="animated.gif" media="not print">
+  <img width=32 height=32>
+</picture>
+<img src="blue-32x32.png" width=32 height=32>
+<object data="blue-32x32.png" width=32 height=32></object>
+<input type="image" src="blue-32x32.png" width=32 height=32>
+<svg width="32" height="32">
+  <image x=0 y=0 href="blue-32x32.png" width=32 height=32></image>
+</svg>

layout/base/tests/chrome/printpreview_images_ref.html

@@ -0,0 +1,15 @@
+<!doctype html>
+<style>
+  div {
+    width: 32px;
+    height: 32px;
+    background-color: blue;
+  }
+</style>
+<div></div>
+<div></div>
+<div></div>
+<div></div>
+<div></div>
+<div></div>
+<div></div>

layout/style/ImageLoader.cpp

@@ -402,46 +402,15 @@ 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, loadingDoc, loadingDoc, data.Principal(), 0, data.ReferrerInfo(),
-      sImageObserver, loadFlags, u"css"_ns, getter_AddRefs(request));
+      uri, &aDocument, &aDocument, data.Principal(), 0, data.ReferrerInfo(),
+      sImageObserver, loadFlags, u"css"_ns,
+      getter_AddRefs(request));
 
   if (NS_FAILED(rv) || !request) {
     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();
 }