Bug 1625571 - Invalidate canvas background in ImageLoader rather than nsDisplayBackgroundImage. r=tnikkel

This is less error prone and also less code.

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

--HG--
extra : moz-landing-system : lando

layout/base/RestyleManager.cpp

@@ -1122,40 +1122,6 @@ static void SyncViewsAndInvalidateDescendants(nsIFrame* aFrame,
   }
 }
 
-static bool IsPrimaryFrameOfRootOrBodyElement(nsIFrame* aFrame) {
-  nsIContent* content = aFrame->GetContent();
-  if (!content) {
-    return false;
-  }
-
-  Document* document = content->OwnerDoc();
-  Element* root = document->GetRootElement();
-  if (!root) {
-    return false;
-  }
-  nsIFrame* rootFrame = root->GetPrimaryFrame();
-  if (!rootFrame) {
-    return false;
-  }
-  if (aFrame == rootFrame) {
-    return true;
-  }
-
-  Element* body = document->GetBodyElement();
-  if (!body) {
-    return false;
-  }
-  nsIFrame* bodyFrame = body->GetPrimaryFrame();
-  if (!bodyFrame) {
-    return false;
-  }
-  if (aFrame == bodyFrame) {
-    return true;
-  }
-
-  return false;
-}
-
 static void ApplyRenderingChangeToTree(PresShell* aPresShell, nsIFrame* aFrame,
                                        nsChangeHint aChange) {
   // We check StyleDisplay()->HasTransformStyle() in addition to checking
@@ -1185,7 +1151,7 @@ static void ApplyRenderingChangeToTree(PresShell* aPresShell, nsIFrame* aFrame,
     // the html element, we propagate the repaint change hint to the
     // viewport. This is necessary for background and scrollbar colors
     // propagation.
-    if (IsPrimaryFrameOfRootOrBodyElement(aFrame)) {
+    if (aFrame->IsPrimaryFrameOfRootOrBodyElement()) {
       nsIFrame* rootFrame = aPresShell->GetRootFrame();
       MOZ_ASSERT(rootFrame, "No root frame?");
       DoApplyRenderingChangeToTree(rootFrame, nsChangeHint_RepaintFrame);

layout/generic/nsFrame.cpp

@@ -114,6 +114,7 @@
 #include "mozilla/ServoStyleSet.h"
 #include "mozilla/ServoStyleSetInlines.h"
 #include "mozilla/css/ImageLoader.h"
+#include "mozilla/dom/HTMLBodyElement.h"
 #include "mozilla/dom/SVGPathData.h"
 #include "mozilla/dom/TouchEvent.h"
 #include "mozilla/gfx/Tools.h"
@@ -614,6 +615,16 @@ static void MaybeScheduleReflowSVGNonDisplayText(nsFrame* aFrame) {
   svgTextFrame->ScheduleReflowSVGNonDisplayText(IntrinsicDirty::StyleChange);
 }
 
+bool nsIFrame::IsPrimaryFrameOfRootOrBodyElement() const {
+  if (!IsPrimaryFrame()) {
+    return false;
+  }
+  nsIContent* content = GetContent();
+  Document* document = content->OwnerDoc();
+  return content == document->GetRootElement() ||
+         content == document->GetBodyElement();
+}
+
 void nsFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
                    nsIFrame* aPrevInFlow) {
   MOZ_ASSERT(nsQueryFrame::FrameIID(mClass) == GetFrameId());

layout/generic/nsIFrame.h

@@ -2129,6 +2129,8 @@ class nsIFrame : public nsQueryFrame {
 
   void SetIsPrimaryFrame(bool aIsPrimary) { mIsPrimaryFrame = aIsPrimary; }
 
+  bool IsPrimaryFrameOfRootOrBodyElement() const;
+
   /**
    * This call is invoked on the primary frame for a character data content
    * node, when it is changed in the content tree.

layout/painting/nsDisplayList.cpp

@@ -3941,19 +3941,16 @@ nsDisplayBackgroundImage::nsDisplayBackgroundImage(
       mLayer(aInitData.layer),
       mIsRasterImage(aInitData.isRasterImage),
       mShouldFixToViewport(aInitData.shouldFixToViewport),
-      mImageFlags(0),
-      mAssociatedImage(false) {
+      mImageFlags(0) {
   MOZ_COUNT_CTOR(nsDisplayBackgroundImage);
-
-  // If we have the same background style as our frame we know we're not going
-  // to need to associate our image to the frame. nsFrame::DidSetComputedStyle
-  // would've done that for us already.
-  //
-  // We need to check this here because the style frame pointer of the frame may
-  // change (and doesn't have to invalidate the display item unless background
-  // styles change).
-  mTriedToAssociateImage =
-      !(mBackgroundStyle && mBackgroundStyle != mFrame->Style());
+#ifdef DEBUG
+  if (mBackgroundStyle && mBackgroundStyle != mFrame->Style()) {
+    // If this changes, then you also need to adjust css::ImageLoader to
+    // invalidate mFrame as needed.
+    MOZ_ASSERT(mFrame->IsCanvasFrame() ||
+               mFrame->IsFrameOfType(nsIFrame::eTablePart));
+  }
+#endif
 
   mBounds = GetBoundsInternal(aInitData.builder, aFrameForBounds);
   if (mShouldFixToViewport) {
@@ -3970,25 +3967,8 @@ nsDisplayBackgroundImage::nsDisplayBackgroundImage(
   }
 }
 
-void nsDisplayBackgroundImage::DisassociateImage() {
-  MOZ_ASSERT(mAssociatedImage);
-  MOZ_ASSERT(mFrame);
-
-  if (mFrame->HasImageRequest()) {
-    // We need to check HasImageRequest because the frame may already have
-    // cleared all its requests in some other way before us.
-    auto& layer = mBackgroundStyle->StyleBackground()->mImage.mLayers[mLayer];
-    mFrame->DisassociateImage(layer.mImage);
-  }
-
-  mAssociatedImage = false;
-}
-
 nsDisplayBackgroundImage::~nsDisplayBackgroundImage() {
   MOZ_COUNT_DTOR(nsDisplayBackgroundImage);
-  if (mAssociatedImage) {
-    DisassociateImage();
-  }
   if (mDependentFrame) {
     mDependentFrame->RemoveDisplayItem(this);
   }
@@ -4537,8 +4517,6 @@ bool nsDisplayBackgroundImage::CreateWebRenderCommands(
     mozilla::wr::IpcResourceUpdateQueue& aResources,
     const StackingContextHelper& aSc, RenderRootStateManager* aManager,
     nsDisplayListBuilder* aDisplayListBuilder) {
-  AssociateImageIfNeeded();
-
   if (!CanBuildWebRenderDisplayItems(aManager->LayerManager(),
                                      aDisplayListBuilder)) {
     return false;
@@ -4684,17 +4662,6 @@ bool nsDisplayBackgroundImage::RenderingMightDependOnPositioningAreaSizeChange()
   return false;
 }
 
-void nsDisplayBackgroundImage::AssociateImageIfNeeded() {
-  if (mTriedToAssociateImage) {
-    return;
-  }
-  mTriedToAssociateImage = true;
-  auto& layer = mBackgroundStyle->StyleBackground()->mImage.mLayers[mLayer];
-  if (mFrame->AssociateImage(layer.mImage)) {
-    mAssociatedImage = true;
-  }
-}
-
 void nsDisplayBackgroundImage::Paint(nsDisplayListBuilder* aBuilder,
                                      gfxContext* aCtx) {
   PaintInternal(aBuilder, aCtx, GetPaintRect(), &mBounds);
@@ -4704,7 +4671,6 @@ void nsDisplayBackgroundImage::PaintInternal(nsDisplayListBuilder* aBuilder,
                                              gfxContext* aCtx,
                                              const nsRect& aBounds,
                                              nsRect* aClipRect) {
-  AssociateImageIfNeeded();
   gfxContext* ctx = aCtx;
   StyleGeometryBox clip =
       mBackgroundStyle->StyleBackground()->mImage.mLayers[mLayer].mClip;

layout/painting/nsDisplayList.h

@@ -4652,9 +4652,6 @@ class nsDisplayBackgroundImage : public nsDisplayImageContainer {
     if (aFrame == mDependentFrame) {
       mDependentFrame = nullptr;
     }
-    if (mAssociatedImage && aFrame == mFrame) {
-      DisassociateImage();
-    }
     nsDisplayImageContainer::RemoveFrame(aFrame);
   }
 
@@ -4667,9 +4664,6 @@ class nsDisplayBackgroundImage : public nsDisplayImageContainer {
   nsRect GetBoundsInternal(nsDisplayListBuilder* aBuilder,
                            nsIFrame* aFrameForBounds = nullptr);
 
-  void AssociateImageIfNeeded();
-  void DisassociateImage();
-
   void PaintInternal(nsDisplayListBuilder* aBuilder, gfxContext* aCtx,
                      const nsRect& aBounds, nsRect* aClipRect);
 
@@ -4698,8 +4692,6 @@ class nsDisplayBackgroundImage : public nsDisplayImageContainer {
   /* Whether the image should be treated as fixed to the viewport. */
   bool mShouldFixToViewport;
   uint32_t mImageFlags;
-  bool mAssociatedImage;
-  bool mTriedToAssociateImage;
 };
 
 enum class TableType : uint8_t {

layout/style/ImageLoader.cpp

@@ -16,6 +16,7 @@
 #include "nsContentUtils.h"
 #include "nsLayoutUtils.h"
 #include "nsError.h"
+#include "nsCanvasFrame.h"
 #include "nsDisplayList.h"
 #include "nsIFrameInlines.h"
 #include "FrameLayerBuilder.h"
@@ -495,8 +496,27 @@ static bool IsRenderNoImages(uint32_t aDisplayItemKey) {
   return flags & TYPE_RENDERS_NO_IMAGES;
 }
 
-static void InvalidateImages(nsIFrame* aFrame, imgIRequest* aRequest) {
-  bool invalidateFrame = false;
+static void InvalidateImages(nsIFrame* aFrame, imgIRequest* aRequest,
+                             bool aForcePaint) {
+  if (!aFrame->StyleVisibility()->IsVisible()) {
+    return;
+  }
+
+  if (aFrame->IsFrameOfType(nsIFrame::eTablePart)) {
+    // Tables don't necessarily build border/background display items
+    // for the individual table part frames, so IterateRetainedDataFor
+    // might not find the right display item.
+    return aFrame->InvalidateFrame();
+  }
+
+  if (aFrame->IsPrimaryFrameOfRootOrBodyElement()) {
+    // Try to invalidate the canvas too, in the probable case the background
+    // was propagated to it.
+    InvalidateImages(aFrame->PresShell()->GetCanvasFrame(), aRequest,
+                     aForcePaint);
+  }
+
+  bool invalidateFrame = aForcePaint;
   const SmallPointerArray<DisplayItemData>& array = aFrame->DisplayItemData();
   for (uint32_t i = 0; i < array.Length(); i++) {
     DisplayItemData* data =
@@ -541,6 +561,17 @@ static void InvalidateImages(nsIFrame* aFrame, imgIRequest* aRequest) {
     }
   }
 
+  // Update ancestor rendering observers (-moz-element etc)
+  //
+  // NOTE: We need to do this even if invalidateFrame is false, see bug 1114526.
+  {
+    nsIFrame* f = aFrame;
+    while (f && !f->HasAnyStateBits(NS_FRAME_DESCENDANT_NEEDS_PAINT)) {
+      SVGObserverUtils::InvalidateDirectRenderingObservers(f);
+      f = nsLayoutUtils::GetCrossDocParentFrame(f);
+    }
+  }
+
   if (invalidateFrame) {
     aFrame->SchedulePaint();
   }
@@ -553,28 +584,7 @@ void ImageLoader::RequestPaintIfNeeded(FrameSet* aFrameSet,
   NS_ASSERTION(mDocument, "Should have returned earlier!");
 
   for (FrameWithFlags& fwf : *aFrameSet) {
-    nsIFrame* frame = fwf.mFrame;
-    if (frame->StyleVisibility()->IsVisible()) {
-      if (frame->IsFrameOfType(nsIFrame::eTablePart)) {
-        // Tables don't necessarily build border/background display items
-        // for the individual table part frames, so IterateRetainedDataFor
-        // might not find the right display item.
-        frame->InvalidateFrame();
-      } else {
-        InvalidateImages(frame, aRequest);
-
-        // Update ancestor rendering observers (-moz-element etc)
-        nsIFrame* f = frame;
-        while (f && !f->HasAnyStateBits(NS_FRAME_DESCENDANT_NEEDS_PAINT)) {
-          SVGObserverUtils::InvalidateDirectRenderingObservers(f);
-          f = nsLayoutUtils::GetCrossDocParentFrame(f);
-        }
-
-        if (aForcePaint) {
-          frame->SchedulePaint();
-        }
-      }
-    }
+    InvalidateImages(fwf.mFrame, aRequest, aForcePaint);
   }
 }