Bug 510856. ComputeRepaintRegionForCopy should not look outside the update rect for moving and non-moving visible content. r=dbaron

layout/base/nsLayoutUtils.cpp

@@ -1256,28 +1256,28 @@ nsLayoutUtils::ComputeRepaintRegionForCopy(nsIFrame* aRootFrame,
   // up nsDisplayClip items, in particular see ApplyAbsPosClipping.
   // XXX but currently a non-moving clip item can incorrectly clip
   // moving items! See bug 428156.
-  nsRect rect;
-  rect.UnionRect(aUpdateRect, aUpdateRect - aDelta);
   nsDisplayListBuilder builder(aRootFrame, PR_FALSE, PR_TRUE);
-  // Retrieve the area of the moving content that's visible. This is the
+  // Retrieve the area of the moving content (considered in both its
+  // before- and after-movement positions) that's visible. This is the
   // only area that needs to be blitted or repainted.
   nsRegion visibleRegionOfMovingContent;
   builder.SetMovingFrame(aMovingFrame, aDelta, &visibleRegionOfMovingContent);
   nsDisplayList list;
 
-  builder.EnterPresShell(aRootFrame, rect);
+  builder.EnterPresShell(aRootFrame, aUpdateRect);
 
   nsresult rv =
-    aRootFrame->BuildDisplayListForStackingContext(&builder, rect, &list);
+    aRootFrame->BuildDisplayListForStackingContext(&builder, aUpdateRect, &list);
 
-  builder.LeavePresShell(aRootFrame, rect);
+  builder.LeavePresShell(aRootFrame, aUpdateRect);
   NS_ENSURE_SUCCESS(rv, rv);
 
 #ifdef DEBUG
   if (gDumpRepaintRegionForCopy) {
     fprintf(stderr,
             "Repaint region for copy --- before optimization (area %d,%d,%d,%d, frame %p):\n",
-            rect.x, rect.y, rect.width, rect.height, (void*)aMovingFrame);
+            aUpdateRect.x, aUpdateRect.y, aUpdateRect.width, aUpdateRect.height,
+            (void*)aMovingFrame);
     nsFrame::PrintDisplayList(&builder, list);
   }
 #endif
@@ -1285,7 +1285,6 @@ nsLayoutUtils::ComputeRepaintRegionForCopy(nsIFrame* aRootFrame,
   // Optimize for visibility, but frames under aMovingFrame will not be
   // considered opaque, so they don't cover non-moving frames.
   nsRegion visibleRegion(aUpdateRect);
-  visibleRegion.Or(visibleRegion, aUpdateRect - aDelta);
   list.OptimizeVisibility(&builder, &visibleRegion);
 
 #ifdef DEBUG
@@ -1295,6 +1294,17 @@ nsLayoutUtils::ComputeRepaintRegionForCopy(nsIFrame* aRootFrame,
   }
 #endif
 
+  // It's possible that there was moving content which was visible but
+  // has now been scrolled out of view so it does not intersect aUpdateRect,
+  // so it's not in our display list. So compute the region that that content
+  // could have occupied --- the complete region that has been scrolled out
+  // of view --- and add it to visibleRegionOfMovingContent.
+  // Note that aRepaintRegion does not depend on moving content which has
+  // been scrolled out of view.
+  nsRegion scrolledOutOfView;
+  scrolledOutOfView.Sub(aUpdateRect, aUpdateRect - aDelta);
+  visibleRegionOfMovingContent.Or(visibleRegionOfMovingContent, scrolledOutOfView);
+
   aRepaintRegion->SetEmpty();
   // Any visible non-moving display items get added to the repaint region
   // a) at their current location and b) offset by -aPt (their position in
@@ -1305,7 +1315,7 @@ nsLayoutUtils::ComputeRepaintRegionForCopy(nsIFrame* aRootFrame,
   // with the moving items taken into account, either on the before-list
   // or the after-list, or even both if we cloned the display lists ... but
   // it's probably not worth it.
-  AddItemsToRegion(&builder, &list, aUpdateRect, rect, aDelta, aRepaintRegion);
+  AddItemsToRegion(&builder, &list, aUpdateRect, aUpdateRect, aDelta, aRepaintRegion);
   // Flush the list so we don't trigger the IsEmpty-on-destruction assertion
   list.DeleteAll();
 

layout/base/nsLayoutUtils.h

@@ -527,9 +527,9 @@ public:
    * efficient), so we use some unfortunately tricky techniques to get by
    * with just the after-list.
    * 
-   * We compute the "visible moving area": aUpdateRect minus any opaque
-   * areas of non-moving content that are above all moving content in
-   * z-order.
+   * We compute the "visible moving area", a region that contains all
+   * moving content that is visible, either before or after scrolling,
+   * intersected with aUpdateRect.
    *
    * The aRepaintRegion region consists of the visible moving area
    * intersected with the union of the following areas:

layout/base/tests/scrolling_helper.html

@@ -58,6 +58,10 @@ body > div {
   <p>Hello
 </div>
 
+<div style="border:1px solid black;" id="testBorder">
+  <div style="height:300px; background:-moz-linear-gradient(top, bottom, from(red), to(black));"></div>
+</div>
+
 <script>
 var tests = document.querySelectorAll("body>div");
 var currentTest = -1;
@@ -111,6 +115,15 @@ function testFixedPosOverlay(blitRegion, paintRegion) {
      "paint region should not intersect blittable area");
 }
 
+// Check that scrolling an element with moving content in it does
+// the obvious thing, even if the element has a border.
+function testBorder(blitRegion, paintRegion) {
+  ok(blitRegion.equalsRegion(new Region([[1,1,201,181]])),
+     "Should blit everything that was already visible");
+  ok(paintRegion.equalsRegion(new Region([[1,181,201,201]])),
+     "Should repaint area that was scrolled into view");
+}
+
 function clientRectToRect(cr)
 {
   return [cr.left, cr.top, cr.right, cr.bottom];