Bug 1528451 - Move code that starts image loads to DidSetComputedStyle. r=heycam

This is more consistent with all the other image request code, and handles
pseudo-elements properly without having to add more out-of-band calls to
UpdateStyleOfOwnedChildFrame and such.

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

layout/base/RestyleManager.cpp

@@ -2811,13 +2811,6 @@ bool RestyleManager::ProcessPostTraversal(Element* aElement,
   if (wasRestyled && oldOrDisplayContentsStyle) {
     MOZ_ASSERT(styleFrame || isDisplayContents);
 
-    // Note that upToDateContext could be the same as oldOrDisplayContentsStyle,
-    // but it doesn't matter, since the only point of it is calling
-    // TriggerImageLoads on the relevant structs, and those don't matter for
-    // display: contents.
-    upToDateContext->StartImageLoads(*mPresContext->Document(),
-                                     oldOrDisplayContentsStyle);
-
     // We want to walk all the continuations here, even the ones with different
     // styles.  In practice, the only reason we get continuations with different
     // styles here is ::first-line (::first-letter never affects element

layout/generic/nsFrame.cpp

@@ -588,7 +588,6 @@ void nsFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
                   NS_FRAME_MAY_BE_TRANSFORMED |
                   NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN));
   } else {
-    mComputedStyle->StartImageLoads(*PresContext()->Document());
     PresContext()->ConstructedFrame();
   }
   if (GetParent()) {
@@ -847,11 +846,9 @@ static void CompareLayers(
 }
 
 static void AddAndRemoveImageAssociations(
-    nsFrame* aFrame, const nsStyleImageLayers* aOldLayers,
+    ImageLoader& aImageLoader, nsFrame* aFrame,
+    const nsStyleImageLayers* aOldLayers,
     const nsStyleImageLayers* aNewLayers) {
-  ImageLoader* imageLoader =
-      aFrame->PresContext()->Document()->StyleImageLoader();
-
   // If the old context had a background-image image, or mask-image image,
   // and new context does not have the same image, clear the image load
   // notifier (which keeps the image loading, if it still is) for the frame.
@@ -861,16 +858,14 @@ static void AddAndRemoveImageAssociations(
   // when we paint, although we could miss a notification in that
   // interval.)
   if (aOldLayers && aFrame->HasImageRequest()) {
-    CompareLayers(aOldLayers, aNewLayers,
-                  [&imageLoader, aFrame](imgRequestProxy* aReq) {
-                    imageLoader->DisassociateRequestFromFrame(aReq, aFrame);
-                  });
+    CompareLayers(aOldLayers, aNewLayers, [&](imgRequestProxy* aReq) {
+      aImageLoader.DisassociateRequestFromFrame(aReq, aFrame);
+    });
   }
 
-  CompareLayers(aNewLayers, aOldLayers,
-                [&imageLoader, aFrame](imgRequestProxy* aReq) {
-                  imageLoader->AssociateRequestToFrame(aReq, aFrame, 0);
-                });
+  CompareLayers(aNewLayers, aOldLayers, [&](imgRequestProxy* aReq) {
+    aImageLoader.AssociateRequestToFrame(aReq, aFrame, 0);
+  });
 }
 
 void nsIFrame::AddDisplayItem(nsDisplayItem* aItem) {
@@ -1042,16 +1037,32 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {
     }
   }
 
+  Document* doc = PresContext()->Document();
+  ImageLoader* imageLoader = doc->StyleImageLoader();
+  // Continuing text frame doesn't initialize its continuation pointer before
+  // reaching here for the first time, so we have to exclude text frames. This
+  // doesn't affect correctness because text can't match selectors.
+  //
+  // FIXME(emilio): We should consider fixing that.
+  const bool isNonTextFirstContinuation =
+      !IsTextFrame() && !GetPrevContinuation();
+  if (isNonTextFirstContinuation) {
+    mComputedStyle->StartImageLoads(*doc);
+  }
+
+  // TODO(emilio): Can we avoid doing some / all of this when
+  // isNonTextFirstContinuation is false? We should consider doing this just for
+  // primary frames and pseudos.
   const nsStyleImageLayers* oldLayers =
       aOldComputedStyle ? &aOldComputedStyle->StyleBackground()->mImage
                         : nullptr;
   const nsStyleImageLayers* newLayers = &StyleBackground()->mImage;
-  AddAndRemoveImageAssociations(this, oldLayers, newLayers);
+  AddAndRemoveImageAssociations(*imageLoader, this, oldLayers, newLayers);
 
   oldLayers =
       aOldComputedStyle ? &aOldComputedStyle->StyleSVGReset()->mMask : nullptr;
   newLayers = &StyleSVGReset()->mMask;
-  AddAndRemoveImageAssociations(this, oldLayers, newLayers);
+  AddAndRemoveImageAssociations(*imageLoader, this, oldLayers, newLayers);
 
   if (aOldComputedStyle) {
     // Detect style changes that should trigger a scroll anchor adjustment
@@ -1126,7 +1137,6 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {
     }
   }
 
-  ImageLoader* imageLoader = PresContext()->Document()->StyleImageLoader();
   imgIRequest* oldBorderImage =
       aOldComputedStyle
           ? aOldComputedStyle->StyleBorder()->GetBorderImageRequest()
@@ -1174,11 +1184,8 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {
   }
 
   // SVGObserverUtils::GetEffectProperties() asserts that we only invoke it with
-  // the first continuation so we need to check that in advance. Continuing text
-  // frame doesn't initialize its continuation pointer before reaching here for
-  // the first time, so we have to exclude text frames. This doesn't affect
-  // correctness because text nodes themselves shouldn't have effects applied.
-  if (!IsTextFrame() && !GetPrevContinuation()) {
+  // the first continuation so we need to check that in advance.
+  if (isNonTextFirstContinuation) {
     // Kick off loading of external SVG resources referenced from properties if
     // any. This currently includes filter, clip-path, and mask.
     SVGObserverUtils::InitiateResourceDocLoads(this);

layout/xul/nsTextBoxFrame.cpp

@@ -888,6 +888,8 @@ void nsTextBoxFrame::RecomputeTitle() {
 }
 
 void nsTextBoxFrame::DidSetComputedStyle(ComputedStyle* aOldComputedStyle) {
+  nsLeafBoxFrame::DidSetComputedStyle(aOldComputedStyle);
+
   if (!aOldComputedStyle) {
     // We're just being initialized
     return;

testing/web-platform/tests/css/css-pseudo/first-letter-background-image-dynamic.html

@@ -0,0 +1,24 @@
+<!doctype html>
+<title>CSS Test: ::first-letter correctly applies background-image dynamically</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#first-letter-styling">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1528451">
+<link rel="match" href="first-letter-background-image-ref.html">
+<style>
+  div::first-letter {
+    color: lime;
+  }
+  div.image::first-letter {
+    /* Lime background */
+    background-image: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M/wHwAEBgIApD5fRAAAAABJRU5ErkJggg==');
+  }
+</style>
+<div>
+  A letter
+</div>
+<script>
+  let div = document.querySelector("div");
+  getComputedStyle(div).color;
+  div.classList.add('image');
+</script>

testing/web-platform/tests/css/css-pseudo/first-letter-background-image-ref.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<style>
+  div::first-letter {
+    color: lime;
+    background-color: lime;
+  }
+</style>
+<div>
+  A letter
+</div>

testing/web-platform/tests/css/css-pseudo/first-letter-background-image.html

@@ -0,0 +1,17 @@
+<!doctype html>
+<title>CSS Test: ::first-letter correctly applies background-image</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#first-letter-styling">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1528451">
+<link rel="match" href="first-letter-background-image-ref.html">
+<style>
+  div::first-letter {
+    color: lime;
+    /* Lime background */
+    background-image: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M/wHwAEBgIApD5fRAAAAABJRU5ErkJggg==');
+  }
+</style>
+<div>
+  A letter
+</div>