Preserve optimized scrolling in the presence of multiple animating rects.

Change PlatformCanvas so that it only fills with "sea foam green" when bitmap data is not externally supplied.  Modifying the interface to make this an option bloated the CL too much.  I may do this as a follow-up.

Adds a new --show-paint-rects command line flag that will render a border around paint rects to help debug and study WebKit painting issues.

R=brettw
BUG=25905
TEST=none

Review URL: http://codereview.chromium.org/414016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33861 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 457 insertions, 89 deletions

diff --git a/chrome/renderer/paint_aggregator.cc b/chrome/renderer/paint_aggregator.cc
index ca4841b..af543af 100644
--- a/chrome/renderer/paint_aggregator.cc
+++ b/chrome/renderer/paint_aggregator.cc
@@ -6,14 +6,26 @@
 
 #include "base/logging.h"
 
-// We implement a very simple algorithm:
+// ----------------------------------------------------------------------------
+// ALGORITHM NOTES
 //
-// - Multiple repaints are unioned to form the smallest bounding box.
-// - If a scroll intersects a repaint, then the scroll is downgraded
-//   to a repaint and then unioned with the existing repaint.
+// We attempt to maintain a scroll rect in the presence of invalidations that
+// are contained within the scroll rect.  If an invalidation crosses a scroll
+// rect, then we just treat the scroll rect as an invalidation rect.
 //
-// This allows for a scroll to exist in parallel to a repaint provided the two
-// do not intersect.
+// For invalidations performed prior to scrolling and contained within the
+// scroll rect, we offset the invalidation rects to account for the fact that
+// the consumer will perform scrolling before painting.
+//
+// We only support scrolling along one axis at a time.  A diagonal scroll will
+// therefore be treated as an invalidation.
+// ----------------------------------------------------------------------------
+
+// If the combined area of paint rects contained within the scroll rect grows
+// too large, then we might as well just treat the scroll rect as a paint rect.
+// This constant sets the max ratio of paint rect area to scroll rect area that
+// we will tolerate before downgrading the scroll into a repaint.
+static const float kMaxRedundantPaintToScrollArea = 0.8f;
 
 gfx::Rect PaintAggregator::PendingUpdate::GetScrollDamage() const {
   // Should only be scrolling in one direction at a time.
@@ -51,8 +63,15 @@ gfx::Rect PaintAggregator::PendingUpdate::GetScrollDamage() const {
   return scroll_rect.Intersect(damaged_rect);
 }
 
+gfx::Rect PaintAggregator::PendingUpdate::GetPaintBounds() const {
+  gfx::Rect bounds;
+  for (size_t i = 0; i < paint_rects.size(); ++i)
+    bounds = bounds.Union(paint_rects[i]);
+  return bounds;
+}
+
 bool PaintAggregator::HasPendingUpdate() const {
-  return !update_.scroll_rect.IsEmpty() || !update_.paint_rect.IsEmpty();
+  return !update_.scroll_rect.IsEmpty() || !update_.paint_rects.empty();
 }
 
 void PaintAggregator::ClearPendingUpdate() {
@@ -60,46 +79,125 @@ void PaintAggregator::ClearPendingUpdate() {
 }
 
 void PaintAggregator::InvalidateRect(const gfx::Rect& rect) {
-  // If this invalidate overlaps with a pending scroll, then we have to
-  // downgrade to invalidating the scroll rect.
-  if (rect.Intersects(update_.scroll_rect)) {
-    update_.paint_rect = update_.paint_rect.Union(update_.scroll_rect);
-    update_.scroll_rect = gfx::Rect();
-    update_.scroll_delta = gfx::Point();
+  // Combine overlapping paints using smallest bounding box.
+  for (size_t i = 0; i < update_.paint_rects.size(); ++i) {
+    gfx::Rect r = update_.paint_rects[i];
+    if (rect.Intersects(r)) {
+      if (!r.Contains(rect)) {  // Optimize for redundant paint.
+        update_.paint_rects.erase(update_.paint_rects.begin() + i);
+        InvalidateRect(rect.Union(r));
+      }
+      return;
+    }
   }
 
-  update_.paint_rect = update_.paint_rect.Union(rect);
+  // Add a non-overlapping paint.
+  // TODO(darin): Limit the size of this vector?
+  // TODO(darin): Coalesce adjacent rects.
+  update_.paint_rects.push_back(rect);
+
+  // If the new paint overlaps with a scroll, then it forces an invalidation of
+  // the scroll.  If the new paint is contained by a scroll, then trim off the
+  // scroll damage to avoid redundant painting.
+  if (!update_.scroll_rect.IsEmpty()) {
+    if (ShouldInvalidateScrollRect(rect)) {
+      InvalidateScrollRect();
+    } else if (update_.scroll_rect.Contains(rect)) {
+      update_.paint_rects[update_.paint_rects.size() - 1] =
+          rect.Subtract(update_.GetScrollDamage());
+      if (update_.paint_rects[update_.paint_rects.size() - 1].IsEmpty())
+        update_.paint_rects.erase(update_.paint_rects.end() - 1);
+    }
+  }
 }
 
 void PaintAggregator::ScrollRect(int dx, int dy, const gfx::Rect& clip_rect) {
+  // We only support scrolling along one axis at a time.
   if (dx != 0 && dy != 0) {
-    // We only support scrolling along one axis at a time.
-    ScrollRect(0, dy, clip_rect);
-    dy = 0;
+    InvalidateRect(clip_rect);
+    return;
   }
 
-  bool intersects_with_painting = update_.paint_rect.Intersects(clip_rect);
+  // We can only scroll one rect at a time.
+  if (!update_.scroll_rect.IsEmpty() &&
+      !update_.scroll_rect.Equals(clip_rect)) {
+    InvalidateRect(clip_rect);
+    return;
+  }
 
-  // If we already have a pending scroll operation or if this scroll operation
-  // intersects the existing paint region, then just failover to invalidating.
-  if (!update_.scroll_rect.IsEmpty() || intersects_with_painting) {
-    if (!intersects_with_painting && update_.scroll_rect == clip_rect) {
-      // OK, we can just update the scroll delta (requires same scrolling axis)
-      if (!dx && !update_.scroll_delta.x()) {
-        update_.scroll_delta.set_y(update_.scroll_delta.y() + dy);
-        return;
-      }
-      if (!dy && !update_.scroll_delta.y()) {
-        update_.scroll_delta.set_x(update_.scroll_delta.x() + dx);
-        return;
-      }
-    }
-    InvalidateRect(update_.scroll_rect);
-    DCHECK(update_.scroll_rect.IsEmpty());
+  // Again, we only support scrolling along one axis at a time.  Make sure this
+  // update doesn't scroll on a different axis than any existing one.
+  if ((dx && update_.scroll_delta.y()) || (dy && update_.scroll_delta.x())) {
     InvalidateRect(clip_rect);
     return;
   }
 
+  // The scroll rect is new or isn't changing (though the scroll amount may
+  // be changing).
   update_.scroll_rect = clip_rect;
-  update_.scroll_delta = gfx::Point(dx, dy);
+  update_.scroll_delta.Offset(dx, dy);
+
+  // Adjust any contained paint rects and check for any overlapping paints.
+  for (size_t i = 0; i < update_.paint_rects.size(); ++i) {
+    if (update_.scroll_rect.Contains(update_.paint_rects[i])) {
+      update_.paint_rects[i] = ScrollPaintRect(update_.paint_rects[i], dx, dy);
+      // The rect may have been scrolled out of view.
+      if (update_.paint_rects[i].IsEmpty()) {
+        update_.paint_rects.erase(update_.paint_rects.begin() + i);
+        i--;
+      }
+    } else if (update_.scroll_rect.Intersects(update_.paint_rects[i])) {
+      InvalidateScrollRect();
+      return;
+    }
+  }
+
+  // If the new scroll overlaps too much with contained paint rects, then force
+  // an invalidation of the scroll.
+  if (ShouldInvalidateScrollRect(gfx::Rect()))
+    InvalidateScrollRect();
+}
+
+gfx::Rect PaintAggregator::ScrollPaintRect(const gfx::Rect& paint_rect,
+                                           int dx, int dy) const {
+  gfx::Rect result = paint_rect;
+
+  result.Offset(dx, dy);
+  result = update_.scroll_rect.Intersect(result);
+
+  // Subtract out the scroll damage rect to avoid redundant painting.
+  return result.Subtract(update_.GetScrollDamage());
+}
+
+bool PaintAggregator::ShouldInvalidateScrollRect(const gfx::Rect& rect) const {
+  if (!rect.IsEmpty()) {
+    if (!update_.scroll_rect.Intersects(rect))
+      return false;
+
+    if (!update_.scroll_rect.Contains(rect))
+      return true;
+  }
+
+  // Check if the combined area of all contained paint rects plus this new
+  // rect comes too close to the area of the scroll_rect.  If so, then we
+  // might as well invalidate the scroll rect.
+
+  int paint_area = rect.width() * rect.height();
+  for (size_t i = 0; i < update_.paint_rects.size(); ++i) {
+    const gfx::Rect& r = update_.paint_rects[i];
+    if (update_.scroll_rect.Contains(r))
+      paint_area += r.width() * r.height();
+  }
+  int scroll_area = update_.scroll_rect.width() * update_.scroll_rect.height();
+  if (float(paint_area) / float(scroll_area) > kMaxRedundantPaintToScrollArea)
+    return true;
+
+  return false;
+}
+
+void PaintAggregator::InvalidateScrollRect() {
+  gfx::Rect scroll_rect = update_.scroll_rect;
+  update_.scroll_rect = gfx::Rect();
+  update_.scroll_delta = gfx::Point();
+  InvalidateRect(scroll_rect);
 }
diff --git a/chrome/renderer/paint_aggregator.h b/chrome/renderer/paint_aggregator.h
index b3ecf4d..d1af49b 100644
--- a/chrome/renderer/paint_aggregator.h
+++ b/chrome/renderer/paint_aggregator.h
@@ -5,28 +5,32 @@
 #ifndef CHROME_RENDERER_PAINT_AGGREGATOR_H_
 #define CHROME_RENDERER_PAINT_AGGREGATOR_H_
 
+#include <vector>
+
 #include "base/gfx/rect.h"
 
 // This class is responsible for aggregating multiple invalidation and scroll
-// commands to produce a single scroll and repaint.
+// commands to produce a scroll and repaint sequence.
 class PaintAggregator {
  public:
-
   // This structure describes an aggregation of InvalidateRect and ScrollRect
   // calls.  If |scroll_rect| is non-empty, then that rect should be scrolled
-  // by the amount specified by |scroll_delta|.  If |paint_rect| is non-empty,
-  // then that rect should be repainted.  If |scroll_rect| and |paint_rect| are
-  // non-empty, then scrolling should be performed before repainting.
+  // by the amount specified by |scroll_delta|.  If |paint_rects| is non-empty,
+  // then those rects should be repainted.  If |scroll_rect| and |paint_rects|
+  // are non-empty, then scrolling should be performed before repainting.
   // |scroll_delta| can only specify scrolling in one direction (i.e., the x
   // and y members cannot both be non-zero).
   struct PendingUpdate {
     gfx::Point scroll_delta;
     gfx::Rect scroll_rect;
-    gfx::Rect paint_rect;
+    std::vector<gfx::Rect> paint_rects;
 
     // Returns the rect damaged by scrolling within |scroll_rect| by
     // |scroll_delta|.  This rect must be repainted.
     gfx::Rect GetScrollDamage() const;
+
+    // Returns the smallest rect containing all paint rects.
+    gfx::Rect GetPaintBounds() const;
   };
 
   // There is a PendingUpdate if InvalidateRect or ScrollRect were called and
@@ -43,6 +47,10 @@ class PaintAggregator {
   void ScrollRect(int dx, int dy, const gfx::Rect& clip_rect);
 
  private:
+  gfx::Rect ScrollPaintRect(const gfx::Rect& paint_rect, int dx, int dy) const;
+  bool ShouldInvalidateScrollRect(const gfx::Rect& rect) const;
+  void InvalidateScrollRect();
+
   PendingUpdate update_;
 };
 
diff --git a/chrome/renderer/paint_aggregator_unittest.cc b/chrome/renderer/paint_aggregator_unittest.cc
index f490a2d..be7f72a 100644
--- a/chrome/renderer/paint_aggregator_unittest.cc
+++ b/chrome/renderer/paint_aggregator_unittest.cc
@@ -18,12 +18,9 @@ TEST(PaintAggregator, SingleInvalidation) {
 
   EXPECT_TRUE(greg.HasPendingUpdate());
   EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
-  EXPECT_FALSE(greg.GetPendingUpdate().paint_rect.IsEmpty());
+  ASSERT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
 
-  EXPECT_EQ(rect.x(), greg.GetPendingUpdate().paint_rect.x());
-  EXPECT_EQ(rect.y(), greg.GetPendingUpdate().paint_rect.y());
-  EXPECT_EQ(rect.width(), greg.GetPendingUpdate().paint_rect.width());
-  EXPECT_EQ(rect.height(), greg.GetPendingUpdate().paint_rect.height());
+  EXPECT_EQ(rect, greg.GetPendingUpdate().paint_rects[0]);
 }
 
 TEST(PaintAggregator, DoubleDisjointInvalidation) {
@@ -35,16 +32,13 @@ TEST(PaintAggregator, DoubleDisjointInvalidation) {
   greg.InvalidateRect(r1);
   greg.InvalidateRect(r2);
 
-  gfx::Rect expected = r1.Union(r2);
+  gfx::Rect expected_bounds = r1.Union(r2);
 
   EXPECT_TRUE(greg.HasPendingUpdate());
   EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
-  EXPECT_FALSE(greg.GetPendingUpdate().paint_rect.IsEmpty());
+  ASSERT_EQ(2U, greg.GetPendingUpdate().paint_rects.size());
 
-  EXPECT_EQ(expected.x(), greg.GetPendingUpdate().paint_rect.x());
-  EXPECT_EQ(expected.y(), greg.GetPendingUpdate().paint_rect.y());
-  EXPECT_EQ(expected.width(), greg.GetPendingUpdate().paint_rect.width());
-  EXPECT_EQ(expected.height(), greg.GetPendingUpdate().paint_rect.height());
+  EXPECT_EQ(expected_bounds, greg.GetPendingUpdate().GetPaintBounds());
 }
 
 TEST(PaintAggregator, SingleScroll) {
@@ -55,23 +49,17 @@ TEST(PaintAggregator, SingleScroll) {
   greg.ScrollRect(delta.x(), delta.y(), rect);
 
   EXPECT_TRUE(greg.HasPendingUpdate());
-  EXPECT_TRUE(greg.GetPendingUpdate().paint_rect.IsEmpty());
+  EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty());
   EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
 
-  EXPECT_EQ(rect.x(), greg.GetPendingUpdate().scroll_rect.x());
-  EXPECT_EQ(rect.y(), greg.GetPendingUpdate().scroll_rect.y());
-  EXPECT_EQ(rect.width(), greg.GetPendingUpdate().scroll_rect.width());
-  EXPECT_EQ(rect.height(), greg.GetPendingUpdate().scroll_rect.height());
+  EXPECT_EQ(rect, greg.GetPendingUpdate().scroll_rect);
 
   EXPECT_EQ(delta.x(), greg.GetPendingUpdate().scroll_delta.x());
   EXPECT_EQ(delta.y(), greg.GetPendingUpdate().scroll_delta.y());
 
   gfx::Rect resulting_damage = greg.GetPendingUpdate().GetScrollDamage();
   gfx::Rect expected_damage(1, 2, 1, 4);
-  EXPECT_EQ(expected_damage.x(), resulting_damage.x());
-  EXPECT_EQ(expected_damage.y(), resulting_damage.y());
-  EXPECT_EQ(expected_damage.width(), resulting_damage.width());
-  EXPECT_EQ(expected_damage.height(), resulting_damage.height());
+  EXPECT_EQ(expected_damage, resulting_damage);
 }
 
 TEST(PaintAggregator, DoubleOverlappingScroll) {
@@ -84,13 +72,10 @@ TEST(PaintAggregator, DoubleOverlappingScroll) {
   greg.ScrollRect(delta2.x(), delta2.y(), rect);
 
   EXPECT_TRUE(greg.HasPendingUpdate());
-  EXPECT_TRUE(greg.GetPendingUpdate().paint_rect.IsEmpty());
+  EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty());
   EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
 
-  EXPECT_EQ(rect.x(), greg.GetPendingUpdate().scroll_rect.x());
-  EXPECT_EQ(rect.y(), greg.GetPendingUpdate().scroll_rect.y());
-  EXPECT_EQ(rect.width(), greg.GetPendingUpdate().scroll_rect.width());
-  EXPECT_EQ(rect.height(), greg.GetPendingUpdate().scroll_rect.height());
+  EXPECT_EQ(rect, greg.GetPendingUpdate().scroll_rect);
 
   gfx::Point expected_delta(delta1.x() + delta2.x(),
                             delta1.y() + delta2.y());
@@ -99,10 +84,7 @@ TEST(PaintAggregator, DoubleOverlappingScroll) {
 
   gfx::Rect resulting_damage = greg.GetPendingUpdate().GetScrollDamage();
   gfx::Rect expected_damage(1, 2, 2, 4);
-  EXPECT_EQ(expected_damage.x(), resulting_damage.x());
-  EXPECT_EQ(expected_damage.y(), resulting_damage.y());
-  EXPECT_EQ(expected_damage.width(), resulting_damage.width());
-  EXPECT_EQ(expected_damage.height(), resulting_damage.height());
+  EXPECT_EQ(expected_damage, resulting_damage);
 }
 
 TEST(PaintAggregator, DiagonalScroll) {
@@ -117,12 +99,259 @@ TEST(PaintAggregator, DiagonalScroll) {
 
   EXPECT_TRUE(greg.HasPendingUpdate());
   EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
-  EXPECT_FALSE(greg.GetPendingUpdate().paint_rect.IsEmpty());
+  ASSERT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
 
-  EXPECT_EQ(rect.x(), greg.GetPendingUpdate().paint_rect.x());
-  EXPECT_EQ(rect.y(), greg.GetPendingUpdate().paint_rect.y());
-  EXPECT_EQ(rect.width(), greg.GetPendingUpdate().paint_rect.width());
-  EXPECT_EQ(rect.height(), greg.GetPendingUpdate().paint_rect.height());
+  EXPECT_EQ(rect, greg.GetPendingUpdate().paint_rects[0]);
 }
 
-// TODO(darin): Add tests for mixed scrolling and invalidation
+TEST(PaintAggregator, ContainedPaintAfterScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  gfx::Rect paint_rect(4, 4, 2, 2);
+  greg.InvalidateRect(paint_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  // expecting a paint rect inside the scroll rect
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+  EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, ContainedPaintBeforeScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect(4, 4, 2, 2);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  // Expecting a paint rect inside the scroll rect
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  paint_rect.Offset(2, 0);
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+  EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, ContainedPaintsBeforeAndAfterScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect1(4, 4, 2, 2);
+  greg.InvalidateRect(paint_rect1);
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  gfx::Rect paint_rect2(6, 4, 2, 2);
+  greg.InvalidateRect(paint_rect2);
+
+  gfx::Rect expected_paint_rect = paint_rect2;
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  // Expecting a paint rect inside the scroll rect
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+  EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, LargeContainedPaintAfterScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(0, 1, scroll_rect);
+
+  gfx::Rect paint_rect(0, 0, 10, 9);  // Repaint 90%
+  greg.InvalidateRect(paint_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, LargeContainedPaintBeforeScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect(0, 0, 10, 9);  // Repaint 90%
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(0, 1, scroll_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, OverlappingPaintBeforeScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect(4, 4, 10, 2);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  gfx::Rect expected_paint_rect = scroll_rect.Union(paint_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, OverlappingPaintAfterScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  gfx::Rect paint_rect(4, 4, 10, 2);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect expected_paint_rect = scroll_rect.Union(paint_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, DisjointPaintBeforeScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect(4, 4, 10, 2);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect scroll_rect(0, 0, 2, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+}
+
+TEST(PaintAggregator, DisjointPaintAfterScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect scroll_rect(0, 0, 2, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  gfx::Rect paint_rect(4, 4, 10, 2);
+  greg.InvalidateRect(paint_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+}
+
+TEST(PaintAggregator, ContainedPaintTrimmedByScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect(4, 4, 6, 6);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(2, 0, scroll_rect);
+
+  // The paint rect should have become narrower.
+  gfx::Rect expected_paint_rect(6, 4, 4, 6);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+}
+
+TEST(PaintAggregator, ContainedPaintEliminatedByScroll) {
+  PaintAggregator greg;
+
+  gfx::Rect paint_rect(4, 4, 6, 6);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(6, 0, scroll_rect);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+}
+
+TEST(PaintAggregator, ContainedPaintAfterScrollTrimmedByScrollDamage) {
+  PaintAggregator greg;
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(4, 0, scroll_rect);
+
+  gfx::Rect paint_rect(2, 0, 4, 10);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect expected_scroll_damage(0, 0, 4, 10);
+  gfx::Rect expected_paint_rect(4, 0, 2, 10);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+  EXPECT_EQ(expected_scroll_damage, greg.GetPendingUpdate().GetScrollDamage());
+  EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]);
+}
+
+TEST(PaintAggregator, ContainedPaintAfterScrollEliminatedByScrollDamage) {
+  PaintAggregator greg;
+
+  gfx::Rect scroll_rect(0, 0, 10, 10);
+  greg.ScrollRect(4, 0, scroll_rect);
+
+  gfx::Rect paint_rect(2, 0, 2, 10);
+  greg.InvalidateRect(paint_rect);
+
+  gfx::Rect expected_scroll_damage(0, 0, 4, 10);
+
+  EXPECT_TRUE(greg.HasPendingUpdate());
+
+  EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty());
+  EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty());
+
+  EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect);
+  EXPECT_EQ(expected_scroll_damage, greg.GetPendingUpdate().GetScrollDamage());
+}
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 8a9df7a..c164f63 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2111,7 +2111,7 @@ void RenderView::didStartProvisionalLoad(WebFrame* frame) {
     completed_client_redirect_src_ = GURL();
   } else if (frame->parent()->isLoading()) {
     // Take note of AUTO_SUBFRAME loads here, so that we can know how to
-    // load an error page.  See DidFailProvisionalLoadWithError.
+    // load an error page.  See didFailProvisionalLoad.
     navigation_state->set_transition_type(PageTransition::AUTO_SUBFRAME);
   }
 
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 34dee52..714fbe6 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -4,12 +4,14 @@
 
 #include "chrome/renderer/render_widget.h"
 
+#include "base/command_line.h"
 #include "base/gfx/point.h"
 #include "base/gfx/size.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/scoped_ptr.h"
 #include "build/build_config.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/transport_dib.h"
 #include "chrome/renderer/render_process.h"
@@ -247,7 +249,7 @@ void RenderWidget::OnWasRestored(bool needs_repainting) {
     return;
   needs_repainting_on_restore_ = false;
 
-  // Tag the next paint as a restore ack, which is picked up by DoDeferredPaint
+  // Tag the next paint as a restore ack, which is picked up by DoDeferredUpdate
   // when it sends out the next PaintRect message.
   set_next_paint_is_restore_ack();
 
@@ -353,11 +355,13 @@ void RenderWidget::ClearFocus() {
 }
 
 void RenderWidget::PaintRect(const gfx::Rect& rect,
+                             const gfx::Point& canvas_origin,
                              skia::PlatformCanvas* canvas) {
+  canvas->save();
 
   // Bring the canvas into the coordinate system of the paint rect.
-  canvas->translate(static_cast<SkScalar>(-rect.x()),
-                    static_cast<SkScalar>(-rect.y()));
+  canvas->translate(static_cast<SkScalar>(-canvas_origin.x()),
+                    static_cast<SkScalar>(-canvas_origin.y()));
 
   // If there is a custom background, tile it.
   if (!background_.empty()) {
@@ -372,8 +376,29 @@ void RenderWidget::PaintRect(const gfx::Rect& rect,
 
   webwidget_->paint(webkit_glue::ToWebCanvas(canvas), rect);
 
+  PaintDebugBorder(rect, canvas);
+
   // Flush to underlying bitmap.  TODO(darin): is this needed?
   canvas->getTopPlatformDevice().accessBitmap(false);
+
+  canvas->restore();
+}
+
+void RenderWidget::PaintDebugBorder(const gfx::Rect& rect,
+                                    skia::PlatformCanvas* canvas) {
+  static bool kPaintBorder =
+      CommandLine::ForCurrentProcess()->HasSwitch(switches::kShowPaintRects);
+  if (!kPaintBorder)
+    return;
+
+  SkPaint paint;
+  paint.setStyle(SkPaint::kStroke_Style);
+  paint.setColor(SkColorSetARGB(0x7F, 0xFF, 0, 0));
+  paint.setStrokeWidth(1);
+
+  SkIRect irect;
+  irect.set(rect.x(), rect.y(), rect.right() - 1, rect.bottom() - 1);
+  canvas->drawIRect(irect, paint);
 }
 
 void RenderWidget::CallDoDeferredUpdate() {
@@ -437,32 +462,36 @@ void RenderWidget::DoDeferredUpdate() {
 
     plugin_window_moves_.clear();
 
-    PaintRect(damaged_rect, canvas.get());
+    PaintRect(damaged_rect, damaged_rect.origin(), canvas.get());
     Send(new ViewHostMsg_ScrollRect(routing_id_, params));
   }
 
-  if (!update.paint_rect.IsEmpty()) {
+  if (!update.paint_rects.empty()) {
     // Normal painting
 
-    gfx::Rect damaged_rect = update.paint_rect;
+    gfx::Rect bounds = update.GetPaintBounds();
 
     // Compute a buffer for painting and cache it.
     scoped_ptr<skia::PlatformCanvas> canvas(
         RenderProcess::current()->GetDrawingCanvas(&current_paint_buf_,
-                                                   damaged_rect));
+                                                   bounds));
     if (!canvas.get()) {
       NOTREACHED();
       return;
     }
 
     // We may get back a smaller canvas than we asked for.
-    damaged_rect.set_width(canvas->getDevice()->width());
-    damaged_rect.set_height(canvas->getDevice()->height());
+    bounds.set_width(canvas->getDevice()->width());
+    bounds.set_height(canvas->getDevice()->height());
+
+    HISTOGRAM_COUNTS_100("MPArch.RW_PaintRectCount", update.paint_rects.size());
 
-    PaintRect(damaged_rect, canvas.get());
+    for (size_t i = 0; i < update.paint_rects.size(); ++i)
+      PaintRect(update.paint_rects[i], bounds.origin(), canvas.get());
 
     ViewHostMsg_PaintRect_Params params;
-    params.bitmap_rect = damaged_rect;
+    params.bitmap_rect = bounds;
+    params.update_rects = update.paint_rects;  // XXX clip to bounds?
     params.view_size = size_;
     params.plugin_window_moves = plugin_window_moves_;
     params.flags = next_paint_flags_;
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index 31eee61..0dcc98a 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -119,7 +119,11 @@ class RenderWidget : public IPC::Channel::Listener,
   // Paints the given rectangular region of the WebWidget into canvas (a
   // shared memory segment returned by AllocPaintBuf on Windows). The caller
   // must ensure that the given rect fits within the bounds of the WebWidget.
-  void PaintRect(const gfx::Rect& rect, skia::PlatformCanvas* canvas);
+  void PaintRect(const gfx::Rect& rect, const gfx::Point& canvas_origin,
+                 skia::PlatformCanvas* canvas);
+
+  // Paints a border at the given rect for debugging purposes.
+  void PaintDebugBorder(const gfx::Rect& rect, skia::PlatformCanvas* canvas);
 
   void CallDoDeferredUpdate();
   void DoDeferredUpdate();