Bug 810470. Part 2: Change nsDisplayBackground invalidation to store and compare the background positioning rect. r=mattwoodrow
authorRobert O'Callahan <robert@ocallahan.org>
Thu, 08 Nov 2012 10:05:32 -0500
changeset 113428 76e6feedd0d6ab359ba04ccc94b8356ef646d22a
parent 113427 a3ea6aa33de6c18651aca7633221dcf7373eb18b
child 113429 85793c93543aabf86d2003139a6fb1b19eb1d8a4
push id18151
push userrocallahan@mozilla.com
push dateFri, 16 Nov 2012 00:03:18 +0000
treeherdermozilla-inbound@e8c599817e97 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs810470
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 810470. Part 2: Change nsDisplayBackground invalidation to store and compare the background positioning rect. r=mattwoodrow
layout/base/nsCSSRendering.cpp
layout/base/nsCSSRendering.h
layout/base/nsDisplayList.cpp
layout/base/nsDisplayList.h
layout/base/nsDisplayListInvalidation.cpp
layout/base/nsDisplayListInvalidation.h
layout/generic/nsCanvasFrame.cpp
layout/generic/nsCanvasFrame.h
layout/generic/nsIFrame.h
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/base/nsCSSRendering.cpp
+++ b/layout/base/nsCSSRendering.cpp
@@ -2731,16 +2731,113 @@ IsTransformed(nsIFrame* aForFrame, nsIFr
   for (nsIFrame* f = aForFrame; f != aTopFrame; f = f->GetParent()) {
     if (f->IsTransformed()) {
       return true;
     }
   }
   return false;
 }
 
+nsRect
+nsCSSRendering::ComputeBackgroundPositioningArea(nsPresContext* aPresContext,
+                                                 nsIFrame* aForFrame,
+                                                 const nsRect& aBorderArea,
+                                                 const nsStyleBackground& aBackground,
+                                                 const nsStyleBackground::Layer& aLayer,
+                                                 nsIFrame** aAttachedToFrame)
+{
+  // Compute background origin area relative to aBorderArea now as we may need
+  // it to compute the effective image size for a CSS gradient.
+  nsRect bgPositioningArea(0, 0, 0, 0);
+
+  nsIAtom* frameType = aForFrame->GetType();
+  nsIFrame* geometryFrame = aForFrame;
+  if (frameType == nsGkAtoms::inlineFrame) {
+    // XXXjwalden Strictly speaking this is not quite faithful to how
+    // background-break is supposed to interact with background-origin values,
+    // but it's a non-trivial amount of work to make it fully conformant, and
+    // until the specification is more finalized (and assuming background-break
+    // even makes the cut) it doesn't make sense to hammer out exact behavior.
+    switch (aBackground.mBackgroundInlinePolicy) {
+    case NS_STYLE_BG_INLINE_POLICY_EACH_BOX:
+      bgPositioningArea = nsRect(nsPoint(0,0), aBorderArea.Size());
+      break;
+    case NS_STYLE_BG_INLINE_POLICY_BOUNDING_BOX:
+      bgPositioningArea = gInlineBGData->GetBoundingRect(aForFrame);
+      break;
+    default:
+      NS_ERROR("Unknown background-inline-policy value!  "
+               "Please, teach me what to do.");
+    case NS_STYLE_BG_INLINE_POLICY_CONTINUOUS:
+      bgPositioningArea = gInlineBGData->GetContinuousRect(aForFrame);
+      break;
+    }
+  } else if (frameType == nsGkAtoms::canvasFrame) {
+    geometryFrame = aForFrame->GetFirstPrincipalChild();
+    // geometryFrame might be null if this canvas is a page created
+    // as an overflow container (e.g. the in-flow content has already
+    // finished and this page only displays the continuations of
+    // absolutely positioned content).
+    if (geometryFrame) {
+      bgPositioningArea = geometryFrame->GetRect();
+    }
+  } else {
+    bgPositioningArea = nsRect(nsPoint(0,0), aBorderArea.Size());
+  }
+
+  // Background images are tiled over the 'background-clip' area
+  // but the origin of the tiling is based on the 'background-origin' area
+  if (aLayer.mOrigin != NS_STYLE_BG_ORIGIN_BORDER && geometryFrame) {
+    nsMargin border = geometryFrame->GetUsedBorder();
+    if (aLayer.mOrigin != NS_STYLE_BG_ORIGIN_PADDING) {
+      border += geometryFrame->GetUsedPadding();
+      NS_ASSERTION(aLayer.mOrigin == NS_STYLE_BG_ORIGIN_CONTENT,
+                   "unknown background-origin value");
+    }
+    geometryFrame->ApplySkipSides(border);
+    bgPositioningArea.Deflate(border);
+  }
+
+  nsIFrame* attachedToFrame = aForFrame;
+  if (NS_STYLE_BG_ATTACHMENT_FIXED == aLayer.mAttachment) {
+    // If it's a fixed background attachment, then the image is placed
+    // relative to the viewport, which is the area of the root frame
+    // in a screen context or the page content frame in a print context.
+    attachedToFrame = aPresContext->PresShell()->FrameManager()->GetRootFrame();
+    NS_ASSERTION(attachedToFrame, "no root frame");
+    nsIFrame* pageContentFrame = nullptr;
+    if (aPresContext->IsPaginated()) {
+      pageContentFrame =
+        nsLayoutUtils::GetClosestFrameOfType(aForFrame, nsGkAtoms::pageContentFrame);
+      if (pageContentFrame) {
+        attachedToFrame = pageContentFrame;
+      }
+      // else this is an embedded shell and its root frame is what we want
+    }
+
+    // Set the background positioning area to the viewport's area
+    // (relative to aForFrame)
+    bgPositioningArea =
+      nsRect(-aForFrame->GetOffsetTo(attachedToFrame), attachedToFrame->GetSize());
+
+    if (!pageContentFrame) {
+      // Subtract the size of scrollbars.
+      nsIScrollableFrame* scrollableFrame =
+        aPresContext->PresShell()->GetRootScrollFrameAsScrollable();
+      if (scrollableFrame) {
+        nsMargin scrollbars = scrollableFrame->GetActualScrollbarSizes();
+        bgPositioningArea.Deflate(scrollbars);
+      }
+    }
+  }
+  *aAttachedToFrame = attachedToFrame;
+
+  return bgPositioningArea;
+}
+
 nsBackgroundLayerState
 nsCSSRendering::PrepareBackgroundLayer(nsPresContext* aPresContext,
                                        nsIFrame* aForFrame,
                                        uint32_t aFlags,
                                        const nsRect& aBorderArea,
                                        const nsRect& aBGClipRect,
                                        const nsStyleBackground& aBackground,
                                        const nsStyleBackground::Layer& aLayer)
@@ -2806,112 +2903,38 @@ nsCSSRendering::PrepareBackgroundLayer(n
   }
 
   nsBackgroundLayerState state(aForFrame, &aLayer.mImage, irFlags);
   if (!state.mImageRenderer.PrepareImage()) {
     // There's no image or it's not ready to be painted.
     return state;
   }
 
+  // The frame to which the background is attached
+  nsIFrame* attachedToFrame = aForFrame;
   // Compute background origin area relative to aBorderArea now as we may need
   // it to compute the effective image size for a CSS gradient.
-  nsRect bgPositioningArea(0, 0, 0, 0);
-
-  nsIAtom* frameType = aForFrame->GetType();
-  nsIFrame* geometryFrame = aForFrame;
-  if (frameType == nsGkAtoms::inlineFrame) {
-    // XXXjwalden Strictly speaking this is not quite faithful to how
-    // background-break is supposed to interact with background-origin values,
-    // but it's a non-trivial amount of work to make it fully conformant, and
-    // until the specification is more finalized (and assuming background-break
-    // even makes the cut) it doesn't make sense to hammer out exact behavior.
-    switch (aBackground.mBackgroundInlinePolicy) {
-    case NS_STYLE_BG_INLINE_POLICY_EACH_BOX:
-      bgPositioningArea = nsRect(nsPoint(0,0), aBorderArea.Size());
-      break;
-    case NS_STYLE_BG_INLINE_POLICY_BOUNDING_BOX:
-      bgPositioningArea = gInlineBGData->GetBoundingRect(aForFrame);
-      break;
-    default:
-      NS_ERROR("Unknown background-inline-policy value!  "
-               "Please, teach me what to do.");
-    case NS_STYLE_BG_INLINE_POLICY_CONTINUOUS:
-      bgPositioningArea = gInlineBGData->GetContinuousRect(aForFrame);
-      break;
-    }
-  } else if (frameType == nsGkAtoms::canvasFrame) {
-    geometryFrame = aForFrame->GetFirstPrincipalChild();
-    // geometryFrame might be null if this canvas is a page created
-    // as an overflow container (e.g. the in-flow content has already
-    // finished and this page only displays the continuations of
-    // absolutely positioned content).
-    if (geometryFrame) {
-      bgPositioningArea = geometryFrame->GetRect();
-    }
-  } else {
-    bgPositioningArea = nsRect(nsPoint(0,0), aBorderArea.Size());
-  }
-
-  // Background images are tiled over the 'background-clip' area
-  // but the origin of the tiling is based on the 'background-origin' area
-  if (aLayer.mOrigin != NS_STYLE_BG_ORIGIN_BORDER && geometryFrame) {
-    nsMargin border = geometryFrame->GetUsedBorder();
-    if (aLayer.mOrigin != NS_STYLE_BG_ORIGIN_PADDING) {
-      border += geometryFrame->GetUsedPadding();
-      NS_ASSERTION(aLayer.mOrigin == NS_STYLE_BG_ORIGIN_CONTENT,
-                   "unknown background-origin value");
-    }
-    geometryFrame->ApplySkipSides(border);
-    bgPositioningArea.Deflate(border);
-  }
+  nsRect bgPositioningArea =
+    ComputeBackgroundPositioningArea(aPresContext, aForFrame, aBorderArea,
+                                     aBackground, aLayer, &attachedToFrame);
 
   // For background-attachment:fixed backgrounds, we'll limit the area
   // where the background can be drawn to the viewport.
   nsRect bgClipRect = aBGClipRect;
 
   // Compute the anchor point.
   //
   // relative to aBorderArea.TopLeft() (which is where the top-left
   // of aForFrame's border-box will be rendered)
   nsPoint imageTopLeft;
   if (NS_STYLE_BG_ATTACHMENT_FIXED == aLayer.mAttachment) {
     aPresContext->SetHasFixedBackgroundFrame();
 
-    // If it's a fixed background attachment, then the image is placed
-    // relative to the viewport, which is the area of the root frame
-    // in a screen context or the page content frame in a print context.
-    nsIFrame* topFrame =
-      aPresContext->PresShell()->FrameManager()->GetRootFrame();
-    NS_ASSERTION(topFrame, "no root frame");
-    nsIFrame* pageContentFrame = nullptr;
-    if (aPresContext->IsPaginated()) {
-      pageContentFrame =
-        nsLayoutUtils::GetClosestFrameOfType(aForFrame, nsGkAtoms::pageContentFrame);
-      if (pageContentFrame) {
-        topFrame = pageContentFrame;
-      }
-      // else this is an embedded shell and its root frame is what we want
-    }
-
-    // Set the background positioning area to the viewport's area
-    // (relative to aForFrame)
-    bgPositioningArea = nsRect(-aForFrame->GetOffsetTo(topFrame), topFrame->GetSize());
-
-    if (!pageContentFrame) {
-      // Subtract the size of scrollbars.
-      nsIScrollableFrame* scrollableFrame =
-        aPresContext->PresShell()->GetRootScrollFrameAsScrollable();
-      if (scrollableFrame) {
-        nsMargin scrollbars = scrollableFrame->GetActualScrollbarSizes();
-        bgPositioningArea.Deflate(scrollbars);
-      }
-    }
-
-    if (aFlags & nsCSSRendering::PAINTBG_TO_WINDOW &&
-        !IsTransformed(aForFrame, topFrame)) {
+    if ((aFlags & nsCSSRendering::PAINTBG_TO_WINDOW) &&
+        !IsTransformed(aForFrame, attachedToFrame)) {
       // Clip background-attachment:fixed backgrounds to the viewport, if we're
       // painting to the screen and not transformed. This avoids triggering
       // tiling in common cases, without affecting output since drawing is
       // always clipped to the viewport when we draw to the screen. (But it's
       // not a pure optimization since it can affect the values of pixels at the
       // edge of the viewport --- whether they're sampled from a putative "next
       // tile" or not.)
       bgClipRect.IntersectRect(bgClipRect, bgPositioningArea + aBorderArea.TopLeft());
--- a/layout/base/nsCSSRendering.h
+++ b/layout/base/nsCSSRendering.h
@@ -295,16 +295,24 @@ struct nsCSSRendering {
    */
   static nscolor
   DetermineBackgroundColor(nsPresContext* aPresContext,
                            nsStyleContext* aStyleContext,
                            nsIFrame* aFrame,
                            bool& aDrawBackgroundImage,
                            bool& aDrawBackgroundColor);
 
+  static nsRect
+  ComputeBackgroundPositioningArea(nsPresContext* aPresContext,
+                                   nsIFrame* aForFrame,
+                                   const nsRect& aBorderArea,
+                                   const nsStyleBackground& aBackground,
+                                   const nsStyleBackground::Layer& aLayer,
+                                   nsIFrame** aAttachedToFrame);
+
   static nsBackgroundLayerState
   PrepareBackgroundLayer(nsPresContext* aPresContext,
                          nsIFrame* aForFrame,
                          uint32_t aFlags,
                          const nsRect& aBorderArea,
                          const nsRect& aBGClipRect,
                          const nsStyleBackground& aBackground,
                          const nsStyleBackground::Layer& aLayer);
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -2029,34 +2029,49 @@ nsDisplayBackground::IsVaryingRelativeTo
   // If aFrame is mFrame or an ancestor in this document, and aFrame is
   // not the viewport frame, then moving aFrame will move mFrame
   // relative to the viewport, so our fixed-pos background will change.
   return aFrame->GetParent() &&
     (aFrame == mFrame ||
      nsLayoutUtils::IsProperAncestorFrame(aFrame, mFrame));
 }
 
+nsRect
+nsDisplayBackground::GetPositioningArea()
+{
+  if (!mBackgroundStyle) {
+    return nsRect();
+  }
+  nsIFrame* attachedToFrame;
+  return nsCSSRendering::ComputeBackgroundPositioningArea(
+      mFrame->PresContext(), mFrame,
+      nsRect(ToReferenceFrame(), mFrame->GetSize()),
+      *mBackgroundStyle, mBackgroundStyle->mLayers[mLayer],
+      &attachedToFrame) + ToReferenceFrame();
+}
+
 bool
-nsDisplayBackground::RenderingMightDependOnFrameSize()
+nsDisplayBackground::RenderingMightDependOnPositioningAreaSizeChange()
 {
   // theme background overrides any other background and we don't know what to do here
   if (mIsThemed)
     return true;
-  
-  // We could be smarter with rounded corners and only invalidate the new area + the piece that was previously
-  // clipped out.
-  nscoord radii[8];
-  if (mFrame->GetBorderRadii(radii))
-    return true;
 
   if (!mBackgroundStyle)
     return false;
 
+  nscoord radii[8];
+  if (mFrame->GetBorderRadii(radii)) {
+    // A change in the size of the positioning area might change the position
+    // of the rounded corners.
+    return true;
+  }
+
   const nsStyleBackground::Layer &layer = mBackgroundStyle->mLayers[mLayer];
-  if (layer.RenderingMightDependOnFrameSize()) {
+  if (layer.RenderingMightDependOnPositioningAreaSizeChange()) {
     return true;
   }
   return false;
 }
 
 bool
 nsDisplayBackground::ShouldFixToViewport(nsDisplayListBuilder* aBuilder)
 {
@@ -2088,31 +2103,37 @@ nsDisplayBackground::Paint(nsDisplayList
                                   nsRect(offset, mFrame->GetSize()),
                                   flags, nullptr, mLayer);
 }
 
 void nsDisplayBackground::ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
                                                     const nsDisplayItemGeometry* aGeometry,
                                                     nsRegion* aInvalidRegion)
 {
-  const nsDisplayBackgroundGeometry* geometry = static_cast<const nsDisplayBackgroundGeometry*>(aGeometry);
-  if (ShouldFixToViewport(aBuilder)) {
-    // This is incorrect, We definitely need to check more things here. 
+  if (!mBackgroundStyle) {
     return;
   }
 
+  const nsDisplayBackgroundGeometry* geometry = static_cast<const nsDisplayBackgroundGeometry*>(aGeometry);
+
   bool snap;
-  if (!geometry->mBounds.IsEqualInterior(GetBounds(aBuilder, &snap)) ||
-      !geometry->mPaddingRect.IsEqualInterior(GetPaddingRect()) ||
-      !geometry->mContentRect.IsEqualInterior(GetContentRect())) {
-    if (!RenderingMightDependOnFrameSize() && geometry->mBounds.TopLeft() == GetBounds(aBuilder, &snap).TopLeft()) {
-      aInvalidRegion->Xor(GetBounds(aBuilder, &snap), geometry->mBounds);
-    } else {
-      aInvalidRegion->Or(GetBounds(aBuilder, &snap), geometry->mBounds);
-    }
+  nsRect bounds = GetBounds(aBuilder, &snap);
+  nsRect positioningArea = GetPositioningArea();
+  if (positioningArea.TopLeft() != geometry->mPositioningArea.TopLeft() ||
+      (positioningArea.Size() != geometry->mPositioningArea.Size() &&
+       RenderingMightDependOnPositioningAreaSizeChange())) {
+    // Positioning area changed in a way that could cause everything to change,
+    // so invalidate everything (both old and new painting areas).
+    aInvalidRegion->Or(bounds, geometry->mBounds);
+    return;
+  }
+  if (!bounds.IsEqualInterior(geometry->mBounds)) {
+    // Positioning area is unchanged, so invalidate just the change in the
+    // painting area.
+    aInvalidRegion->Xor(bounds, geometry->mBounds);
   }
 }
 
 nsRect
 nsDisplayBackground::GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) {
   *aSnap = true;
   nsPresContext* presContext = mFrame->PresContext();
 
--- a/layout/base/nsDisplayList.h
+++ b/layout/base/nsDisplayList.h
@@ -1821,30 +1821,42 @@ public:
                                    nsRegion* aVisibleRegion,
                                    const nsRect& aAllowVisibleRegionExpansion) MOZ_OVERRIDE;
   virtual nsRegion GetOpaqueRegion(nsDisplayListBuilder* aBuilder,
                                    bool* aSnap) MOZ_OVERRIDE;
   virtual bool IsVaryingRelativeToMovingFrame(nsDisplayListBuilder* aBuilder,
                                                 nsIFrame* aFrame) MOZ_OVERRIDE;
   virtual bool IsUniform(nsDisplayListBuilder* aBuilder, nscolor* aColor) MOZ_OVERRIDE;
   virtual bool ShouldFixToViewport(nsDisplayListBuilder* aBuilder) MOZ_OVERRIDE;
+  /**
+   * GetBounds() returns the background painting area.
+   */
   virtual nsRect GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) MOZ_OVERRIDE;
   virtual void Paint(nsDisplayListBuilder* aBuilder, nsRenderingContext* aCtx) MOZ_OVERRIDE;
   virtual uint32_t GetPerFrameKey() MOZ_OVERRIDE;
   NS_DISPLAY_DECL_NAME("Background", TYPE_BACKGROUND)
   // Returns the value of GetUnderlyingFrame()->IsThemed(), but cached
   bool IsThemed() { return mIsThemed; }
 
   /**
+   * Return the background positioning area.
+   * (GetBounds() returns the background painting area.)
+   * Can be called only when mBackgroundStyle is non-null.
+   */
+  nsRect GetPositioningArea();
+
+  /**
    * Returns true if existing rendered pixels of this display item may need
-   * to be redrawn if the frame size changes.
-   * If false, only the changed area needs to be redrawn.
+   * to be redrawn if the positioning area size changes but its position does
+   * not.
+   * If false, only the changed painting area needs to be redrawn when the
+   * positioning area size changes but its position does not.
    */
-  bool RenderingMightDependOnFrameSize();
-  
+  bool RenderingMightDependOnPositioningAreaSizeChange();
+
   virtual nsDisplayItemGeometry* AllocateGeometry(nsDisplayListBuilder* aBuilder)
   {
     return new nsDisplayBackgroundGeometry(this, aBuilder);
   }
 
   virtual void ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
                                          const nsDisplayItemGeometry* aGeometry,
                                          nsRegion* aInvalidRegion);
--- a/layout/base/nsDisplayListInvalidation.cpp
+++ b/layout/base/nsDisplayListInvalidation.cpp
@@ -37,28 +37,27 @@ nsDisplayBorderGeometry::nsDisplayBorder
 
 void
 nsDisplayBorderGeometry::MoveBy(const nsPoint& aOffset)
 {
   mBounds.MoveBy(aOffset);
   mContentRect.MoveBy(aOffset);
 }
 
-nsDisplayBackgroundGeometry::nsDisplayBackgroundGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder)
+nsDisplayBackgroundGeometry::nsDisplayBackgroundGeometry(nsDisplayBackground* aItem,
+                                                         nsDisplayListBuilder* aBuilder)
   : nsDisplayItemGeometry(aItem, aBuilder)
-  , mPaddingRect(aItem->GetPaddingRect())
-  , mContentRect(aItem->GetContentRect())
+  , mPositioningArea(aItem->GetPositioningArea())
 {}
 
 void
 nsDisplayBackgroundGeometry::MoveBy(const nsPoint& aOffset)
 {
   mBounds.MoveBy(aOffset);
-  mPaddingRect.MoveBy(aOffset);
-  mContentRect.MoveBy(aOffset);
+  mPositioningArea.MoveBy(aOffset);
 }
 
 nsDisplayBoxShadowInnerGeometry::nsDisplayBoxShadowInnerGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder)
   : nsDisplayItemGeometry(aItem, aBuilder)
   , mPaddingRect(aItem->GetPaddingRect())
 {}
 
 void
--- a/layout/base/nsDisplayListInvalidation.h
+++ b/layout/base/nsDisplayListInvalidation.h
@@ -5,16 +5,17 @@
 
 #ifndef NSDISPLAYLISTINVALIDATION_H_
 #define NSDISPLAYLISTINVALIDATION_H_
 
 #include "nsRect.h"
 
 class nsDisplayItem;
 class nsDisplayListBuilder;
+class nsDisplayBackground;
 
 /**
  * This stores the geometry of an nsDisplayItem, and the area
  * that will be affected when painting the item.
  *
  * It is used to retain information about display items so they
  * can be compared against new display items in the next paint.
  */
@@ -70,22 +71,21 @@ public:
   virtual void MoveBy(const nsPoint& aOffset);
 
   nsRect mContentRect;
 };
 
 class nsDisplayBackgroundGeometry : public nsDisplayItemGeometry
 {
 public:
-  nsDisplayBackgroundGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder);
+  nsDisplayBackgroundGeometry(nsDisplayBackground* aItem, nsDisplayListBuilder* aBuilder);
 
   virtual void MoveBy(const nsPoint& aOffset);
 
-  nsRect mPaddingRect;
-  nsRect mContentRect;
+  nsRect mPositioningArea;
 };
 
 class nsDisplayBoxShadowInnerGeometry : public nsDisplayItemGeometry
 {
 public:
   nsDisplayBoxShadowInnerGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder);
   
   virtual void MoveBy(const nsPoint& aOffset);
--- a/layout/generic/nsCanvasFrame.cpp
+++ b/layout/generic/nsCanvasFrame.cpp
@@ -513,49 +513,16 @@ nsCanvasFrame::Reflow(nsPresContext*    
         kidReflowState.mComputedMargin.TopBottom();
     } else {
       aDesiredSize.height = aReflowState.ComputedHeight();
     }
 
     aDesiredSize.SetOverflowAreasToDesiredBounds();
     aDesiredSize.mOverflowAreas.UnionWith(
       kidDesiredSize.mOverflowAreas + kidPt);
-
-    // Handle invalidating fixed-attachment backgrounds propagated to the
-    // canvas when the canvas size (and therefore the background positioning
-    // area's size) changes.  Such backgrounds are not invalidated in the
-    // normal manner because the size of the original frame for that background
-    // may not have changed.
-    //
-    // This isn't the right fix for this issue, taken more generally.  In
-    // particular, this doesn't handle fixed-attachment backgrounds that are *not*
-    // propagated.  If a layer with the characteristics tested for below exists
-    // in a non-propagated background, we should invalidate the "corresponding"
-    // frame (which subsumes this special case if defined broadly).  For now,
-    // however, this addresses the most common case.  Given that this behavior has
-    // long been broken (non-zero percent background-size may be a new instance,
-    // but non-zero percent background-position is longstanding), we defer a
-    // fully correct fix until later.
-    if (nsSize(aDesiredSize.width, aDesiredSize.height) != GetSize()) {
-      nsIFrame* rootElementFrame =
-        aPresContext->PresShell()->FrameConstructor()->GetRootElementStyleFrame();
-      nsStyleContext* bgSC =
-        nsCSSRendering::FindCanvasBackground(this, rootElementFrame);
-      const nsStyleBackground* bg = bgSC->GetStyleBackground();
-      if (!bg->IsTransparent()) {
-        NS_FOR_VISIBLE_BACKGROUND_LAYERS_BACK_TO_FRONT(i, bg) {
-          const nsStyleBackground::Layer& layer = bg->mLayers[i];
-          if (layer.mAttachment == NS_STYLE_BG_ATTACHMENT_FIXED &&
-              layer.RenderingMightDependOnFrameSize()) {
-            InvalidateFrame();
-            break;
-          }
-        }
-      }
-    }
   }
 
   if (prevCanvasFrame) {
     ReflowOverflowContainerChildren(aPresContext, aReflowState,
                                     aDesiredSize.mOverflowAreas, 0,
                                     aStatus);
   }
 
--- a/layout/generic/nsCanvasFrame.h
+++ b/layout/generic/nsCanvasFrame.h
@@ -111,38 +111,16 @@ public:
 protected:
   virtual int GetSkipSides() const;
 
   // Data members
   bool                      mDoPaintFocus;
   bool                      mAddedScrollPositionListener;
 };
 
-class nsDisplayCanvasBackgroundGeometry : public nsDisplayItemGeometry
-{
-public:
-  nsDisplayCanvasBackgroundGeometry(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder, const nsRect& aChildBorder)
-    : nsDisplayItemGeometry(aItem, aBuilder)
-    , mChildBorder(aChildBorder)
-    , mPaddingRect(aItem->GetPaddingRect())
-    , mContentRect(aItem->GetContentRect())
-  {}
-
-  virtual void MoveBy(const nsPoint& aOffset)
-  {
-    mBounds.MoveBy(aOffset);
-    mPaddingRect.MoveBy(aOffset);
-    mContentRect.MoveBy(aOffset);
-  }
-
-  nsRect mChildBorder;
-  nsRect mPaddingRect;
-  nsRect mContentRect;
-};
-
 /**
  * Override nsDisplayBackground methods so that we pass aBGClipRect to
  * PaintBackground, covering the whole overflow area.
  * We can also paint an "extra background color" behind the normal
  * background.
  */
 class nsDisplayCanvasBackground : public nsDisplayBackground {
 public:
@@ -190,48 +168,16 @@ public:
   }
   virtual void HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect,
                        HitTestState* aState, nsTArray<nsIFrame*> *aOutFrames) MOZ_OVERRIDE
   {
     // We need to override so we don't consider border-radius.
     aOutFrames->AppendElement(mFrame);
   }
   
-  virtual nsDisplayItemGeometry* AllocateGeometry(nsDisplayListBuilder* aBuilder)
-  {
-    nsIFrame *child = mFrame->GetFirstPrincipalChild();
-    return new nsDisplayCanvasBackgroundGeometry(this, aBuilder, 
-                                                 child ? child->GetRect() : nsRect());;
-  }
-
-  virtual void ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
-                                         const nsDisplayItemGeometry* aGeometry,
-                                         nsRegion* aInvalidRegion)
-  {
-    const nsDisplayCanvasBackgroundGeometry* geometry = static_cast<const nsDisplayCanvasBackgroundGeometry*>(aGeometry);
-    if (ShouldFixToViewport(aBuilder)) {
-      // This is incorrect, We definitely need to check more things here. 
-      return;
-    }
-
-    nsIFrame *child = mFrame->GetFirstPrincipalChild();
-
-    bool snap;
-    if (!geometry->mBounds.IsEqualInterior(GetBounds(aBuilder, &snap)) ||
-        (child && !geometry->mChildBorder.IsEqualInterior(child->GetRect())) ||
-        !geometry->mPaddingRect.IsEqualInterior(GetPaddingRect()) ||
-        !geometry->mContentRect.IsEqualInterior(GetContentRect())) {
-      if (!RenderingMightDependOnFrameSize() && geometry->mBounds.TopLeft() == GetBounds(aBuilder, &snap).TopLeft()) {
-        aInvalidRegion->Xor(GetBounds(aBuilder, &snap), geometry->mBounds);
-      } else {
-        aInvalidRegion->Or(GetBounds(aBuilder, &snap), geometry->mBounds);
-      }
-    }
-  }
-  
   virtual void Paint(nsDisplayListBuilder* aBuilder,
                      nsRenderingContext* aCtx) MOZ_OVERRIDE;
 
   void SetExtraBackgroundColor(nscolor aColor)
   {
     mExtraBackgroundColor = aColor;
   }
 
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -2231,17 +2231,16 @@ public:
    * if this frame painted a display item of that type during the 
    * previous paint. SVG rendering observers are always notified.
    */
   void InvalidateFrameSubtree(uint32_t aDisplayItemKey = 0);
 
   /**
    * Called when a frame is about to be removed and needs to be invalidated.
    * Normally does nothing since DLBI handles removed frames.
-   * 
    */
   virtual void InvalidateFrameForRemoval() {}
 
   /**
    * When HasUserData(frame->LayerIsPrerenderedDataKey()), then the
    * entire overflow area of this frame has been rendered in its
    * layer(s).
    */
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -1828,17 +1828,17 @@ nsStyleBackground::Position::SetInitialV
   mXPosition.mLength = 0;
   mXPosition.mHasPercent = true;
   mYPosition.mPercent = 0.0f;
   mYPosition.mLength = 0;
   mYPosition.mHasPercent = true;
 }
 
 bool
-nsStyleBackground::Size::DependsOnFrameSize(const nsStyleImage& aImage) const
+nsStyleBackground::Size::DependsOnPositioningAreaSize(const nsStyleImage& aImage) const
 {
   NS_ABORT_IF_FALSE(aImage.GetType() != eStyleImageType_Null,
                     "caller should have handled this");
 
   // If either dimension contains a non-zero percentage, rendering for that
   // dimension straightforwardly depends on frame size.
   if ((mWidthType == eLengthPercentage && mWidth.mPercent != 0.0f) ||
       (mHeightType == eLengthPercentage && mHeight.mPercent != 0.0f)) {
@@ -1958,24 +1958,25 @@ nsStyleBackground::Layer::SetInitialValu
   mOrigin = NS_STYLE_BG_ORIGIN_PADDING;
   mRepeat.SetInitialValues();
   mPosition.SetInitialValues();
   mSize.SetInitialValues();
   mImage.SetNull();
 }
 
 bool
-nsStyleBackground::Layer::RenderingMightDependOnFrameSize() const
+nsStyleBackground::Layer::RenderingMightDependOnPositioningAreaSizeChange() const
 {
   // Do we even have an image?
   if (mImage.IsEmpty()) {
     return false;
   }
 
-  return mPosition.DependsOnFrameSize() || mSize.DependsOnFrameSize(mImage);
+  return mPosition.DependsOnPositioningAreaSize() ||
+      mSize.DependsOnPositioningAreaSize(mImage);
 }
 
 bool
 nsStyleBackground::Layer::operator==(const Layer& aOther) const
 {
   return mAttachment == aOther.mAttachment &&
          mClip == aOther.mClip &&
          mOrigin == aOther.mOrigin &&
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -325,17 +325,17 @@ struct nsStyleBackground {
     // Initialize nothing
     Position() {}
 
     // Initialize to initial values
     void SetInitialValues();
 
     // True if the effective background image position described by this depends
     // on the size of the corresponding frame.
-    bool DependsOnFrameSize() const {
+    bool DependsOnPositioningAreaSize() const {
       return mXPosition.mPercent != 0.0f || mYPosition.mPercent != 0.0f;
     }
 
     bool operator==(const Position& aOther) const {
       return mXPosition == aOther.mXPosition &&
              mYPosition == aOther.mYPosition;
     }
     bool operator!=(const Position& aOther) const {
@@ -366,33 +366,33 @@ struct nsStyleBackground {
       NS_ABORT_IF_FALSE(mHeightType == eLengthPercentage,
                         "resolving non-length/percent dimension!");
       return mHeight.ResolveLengthPercentage(aBgPositioningArea.height);
     }
 
     // Except for eLengthPercentage, Dimension types which might change
     // how a layer is painted when the corresponding frame's dimensions
     // change *must* precede all dimension types which are agnostic to
-    // frame size; see DependsOnFrameSize.
+    // frame size; see DependsOnDependsOnPositioningAreaSizeSize.
     enum DimensionType {
       // If one of mWidth and mHeight is eContain or eCover, then both are.
       // Also, these two values must equal the corresponding values in
       // kBackgroundSizeKTable.
       eContain, eCover,
 
       eAuto,
       eLengthPercentage,
       eDimensionType_COUNT
     };
     uint8_t mWidthType, mHeightType;
 
     // True if the effective image size described by this depends on the size of
     // the corresponding frame, when aImage (which must not have null type) is
     // the background image.
-    bool DependsOnFrameSize(const nsStyleImage& aImage) const;
+    bool DependsOnPositioningAreaSize(const nsStyleImage& aImage) const;
 
     // Initialize nothing
     Size() {}
 
     // Initialize to initial values
     void SetInitialValues();
 
     bool operator==(const Size& aOther) const;
@@ -445,21 +445,21 @@ struct nsStyleBackground {
     void UntrackImages(nsPresContext* aContext) {
       if (mImage.GetType() == eStyleImageType_Image)
         mImage.UntrackImage(aContext);
     }
 
     void SetInitialValues();
 
     // True if the rendering of this layer might change when the size
-    // of the corresponding frame changes.  This is true for any
+    // of the background positioning area changes.  This is true for any
     // non-solid-color background whose position or size depends on
-    // the frame size.  It's also true for SVG images whose root <svg>
-    // node has a viewBox.
-    bool RenderingMightDependOnFrameSize() const;
+    // the size of the positioning area.  It's also true for SVG images
+    // whose root <svg> node has a viewBox.
+    bool RenderingMightDependOnPositioningAreaSizeChange() const;
 
     // An equality operator that compares the images using URL-equality
     // rather than pointer-equality.
     bool operator==(const Layer& aOther) const;
     bool operator!=(const Layer& aOther) const {
       return !(*this == aOther);
     }
   };