Fix painting of fixed background images in composited layers

Source/core/rendering/RenderBox.cpp

Index: Source/core/rendering/RenderBox.cpp
diff --git a/Source/core/rendering/RenderBox.cpp b/Source/core/rendering/RenderBox.cpp
index 072e07b172ad1b335a07c8c6400973efd835c5f9..f884fb88dfaf42a3012df7c87b250164c6c0205a 100644
--- a/Source/core/rendering/RenderBox.cpp
+++ b/Source/core/rendering/RenderBox.cpp
@@ -1331,19 +1331,26 @@ void RenderBox::paintBackground(const PaintInfo& paintInfo, const LayoutRect& pa
     paintFillLayers(paintInfo, resolveColor(CSSPropertyBackgroundColor), style()->backgroundLayers(), paintRect, bleedAvoidance);
 }
 
-LayoutRect RenderBox::backgroundPaintedExtent() const
+bool RenderBox::getBackgroundPaintedExtent(LayoutRect& paintedExtent) const
 {
     ASSERT(hasBackground());
     LayoutRect backgroundRect = pixelSnappedIntRect(borderBoxRect());
 
     Color backgroundColor = resolveColor(CSSPropertyBackgroundColor);
-    if (backgroundColor.isValid() && backgroundColor.alpha())
-        return backgroundRect;
-    if (!style()->backgroundLayers()->image() || style()->backgroundLayers()->next())
-        return backgroundRect;
+    if (backgroundColor.isValid() && backgroundColor.alpha()) {
+        paintedExtent = backgroundRect;
+        return true;
+    }
+
+    if (!style()->backgroundLayers()->image() || style()->backgroundLayers()->next()) {
+        paintedExtent =  backgroundRect;
+        return true;
+    }
+
     BackgroundImageGeometry geometry;
-    const_cast<RenderBox*>(this)->calculateBackgroundImageGeometry(style()->backgroundLayers(), backgroundRect, geometry);
-    return geometry.destRect();
+    calculateBackgroundImageGeometry(0, style()->backgroundLayers(), backgroundRect, geometry);
+    paintedExtent = geometry.destRect();
+    return !geometry.hasNonLocalGeometry();

[esprehn, 2014/01/08 01:53:55]

Instead of assigning the values of the layout rect even if we're going to return
false, can we early return and avoid assigning the values we know are probably
wrong?

if (geometry.hasNonLocalGeometry())
  return false;
paintExtent = ...;
return true;

[Stephen White, 2014/03/27 15:07:12]

Done.

 }
 
 bool RenderBox::backgroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect) const
@@ -1453,7 +1460,9 @@ bool RenderBox::computeBackgroundIsKnownToBeObscured()
     // FIXME: box-shadow is painted while background painting.
     if (style()->boxShadow())
         return false;
-    LayoutRect backgroundRect = backgroundPaintedExtent();
+    LayoutRect backgroundRect;
+    if (!getBackgroundPaintedExtent(backgroundRect))
+        return false;
     return foregroundIsKnownToBeOpaqueInRect(backgroundRect, backgroundObscurationTestMaxDepth);
 }
 
@@ -1557,7 +1566,8 @@ LayoutRect RenderBox::maskClipRect()
     for (const FillLayer* maskLayer = style()->maskLayers(); maskLayer; maskLayer = maskLayer->next()) {
         if (maskLayer->image()) {
             BackgroundImageGeometry geometry;
-            calculateBackgroundImageGeometry(maskLayer, borderBox, geometry);
+            // Masks should never have fixed attachment, so it's OK for paintContainer to be null.
+            calculateBackgroundImageGeometry(0, maskLayer, borderBox, geometry);
             result.unite(geometry.destRect());
         }
     }
@@ -1646,8 +1656,7 @@ bool RenderBox::repaintLayerRectsForImage(WrappedImagePtr image, const FillLayer
 
     for (const FillLayer* curLayer = layers; curLayer; curLayer = curLayer->next()) {
         if (curLayer->image() && image == curLayer->image()->data() && curLayer->image()->canRender(this, style()->effectiveZoom())) {
-            // Now that we know this image is being used, compute the renderer and the rect
-            // if we haven't already
+            // Now that we know this image is being used, compute the renderer and the rect if we haven't already.
             if (!layerRenderer) {
                 bool drawingRootBackground = drawingBackground && (isRoot() || (isBody() && !document().documentElement()->renderer()->hasBackground()));
                 if (drawingRootBackground) {
@@ -1674,7 +1683,14 @@ bool RenderBox::repaintLayerRectsForImage(WrappedImagePtr image, const FillLayer
             }
 
             BackgroundImageGeometry geometry;
-            layerRenderer->calculateBackgroundImageGeometry(curLayer, rendererRect, geometry);
+            layerRenderer->calculateBackgroundImageGeometry(0, curLayer, rendererRect, geometry);
+            if (geometry.hasNonLocalGeometry()) {
+                // Rather than incur the costs of computing the paintContainer for renderers with fixed backgrounds
+                // in order to get the right destRect, just repaint the entire renderer.
+                layerRenderer->repaint();

[esprehn, 2014/01/08 01:53:55]

I suspect this will be far worse sometimes too, a huge raster on mobile is worse
I think?

[Stephen White, 2014/03/27 15:07:12]

I defer to someone more knowledgeable. I'm just cargo culting this patch because
it fixes my bug. If there's a better way, someone else should probably step in.

+                return true;
+            }
+
             layerRenderer->repaintRectangle(geometry.destRect());
             if (geometry.destRect() == rendererRect)
                 return true;