Fix graphics corruption that can occur in the PDF viewer when scrolling

pdf/paint_aggregator.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "pdf/paint_aggregator.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 
+namespace {
+
+// We don't use std::sign here to avoid any issues with negative 0.

[Sam McNally, 2015/06/02 03:29:26]

Don't pp:Points contain ints?

+int32_t sign(int32_t num) {

[Sam McNally, 2015/06/02 03:29:26]

How about
bool IsNegative(int32_t num) { return num < 0; }
(or something similar) instead?

[raymes, 2015/06/02 06:57:08]

Done.

+  return num < 0 ? -1 : 1;
+}
+
+}  // namespace
+
 // ----------------------------------------------------------------------------
 // ALGORITHM NOTES
 //
 // 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.
 //
 // 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.
@@ skipping 117 matching lines @@
   }
 
   // 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 ((amount.x() && update_.scroll_delta.y()) ||
       (amount.y() && update_.scroll_delta.x())) {
     InvalidateRect(clip_rect);
     return;
   }
 
+  // If we scroll in a reverse direction to the direction we originally scrolled
+  // and there were invalidations that happened in-between we may end up
+  // incorrectly clipping the invalidated rects (see crbug.com/488390). This bug
+  // doesn't exist in the original implementation
+  // (ppapi/utility/graphics/paint_aggregator.cc) which uses a different method
+  // of handling invalidations that occur after a scroll. The problem is that
+  // when we scroll the invalidated region, we clip it to the scroll rect. This
+  // can cause us to lose information about what the invalidated region was if
+  // it gets scrolled back into view. We either need to not do this clipping or
+  // disallow combining scrolls that occur in different directions with
+  // invalidations that happen in-between. This code really needs some tests...
+  if (!update_.paint_rects.empty()) {
+    if (sign(amount.x()) != sign(update_.scroll_delta.x()) ||
+        sign(amount.y()) != sign(update_.scroll_delta.y())) {
+      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 += amount;
 
   // We might have just wiped out a pre-existing scroll.
   if (update_.scroll_delta == pp::Point()) {
     update_.scroll_rect = pp::Rect();
     return;
   }
@@ skipping 96 matching lines @@
   }
 
   // If the new paint overlaps with a scroll, then also invalidate the rect in
   // its new position.
   if (check_scroll &&
       !update_.scroll_rect.IsEmpty() &&
       update_.scroll_rect.Intersects(rect)) {
     InvalidateRectInternal(ScrollPaintRect(rect, update_.scroll_delta), false);
   }
 }