Commit aa23d5d0 authored by enne's avatar enne Committed by Commit bot

Fix scrollbar overflow with ScaleToEnclosingRectSafe

ScaleToEnclosingRect DCHECKS that the scale that's being done won't
overflow, however this can't be known because the scrollbar's
internal scale is determined by page content.  Clamp these values
safely instead of DCHECKing.

R=danakj@chromium.org
BUG=652604
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2384063007
Cr-Commit-Position: refs/heads/master@{#422987}
parent 58011218
...@@ -133,7 +133,7 @@ gfx::Rect PaintedScrollbarLayer::ScrollbarLayerRectToContentRect( ...@@ -133,7 +133,7 @@ gfx::Rect PaintedScrollbarLayer::ScrollbarLayerRectToContentRect(
const gfx::Rect& layer_rect) const { const gfx::Rect& layer_rect) const {
// Don't intersect with the bounds as in LayerRectToContentRect() because // Don't intersect with the bounds as in LayerRectToContentRect() because
// layer_rect here might be in coordinates of the containing layer. // layer_rect here might be in coordinates of the containing layer.
gfx::Rect expanded_rect = gfx::ScaleToEnclosingRect( gfx::Rect expanded_rect = gfx::ScaleToEnclosingRectSafe(
layer_rect, internal_contents_scale_, internal_contents_scale_); layer_rect, internal_contents_scale_, internal_contents_scale_);
// We should never return a rect bigger than the content bounds. // We should never return a rect bigger than the content bounds.
gfx::Size clamped_size = expanded_rect.size(); gfx::Size clamped_size = expanded_rect.size();
......
...@@ -254,6 +254,9 @@ GFX_EXPORT Rect SubtractRects(const Rect& a, const Rect& b); ...@@ -254,6 +254,9 @@ GFX_EXPORT Rect SubtractRects(const Rect& a, const Rect& b);
// contained within the rect, because they will appear on one of these edges. // contained within the rect, because they will appear on one of these edges.
GFX_EXPORT Rect BoundingRect(const Point& p1, const Point& p2); GFX_EXPORT Rect BoundingRect(const Point& p1, const Point& p2);
// Scales the rect and returns the enclosing rect. Use this only the inputs are
// known to not overflow. Use ScaleToEnclosingRectSafe if the inputs are
// unknown and need to use saturated math.
inline Rect ScaleToEnclosingRect(const Rect& rect, inline Rect ScaleToEnclosingRect(const Rect& rect,
float x_scale, float x_scale,
float y_scale) { float y_scale) {
...@@ -284,6 +287,24 @@ inline Rect ScaleToEnclosingRect(const Rect& rect, float scale) { ...@@ -284,6 +287,24 @@ inline Rect ScaleToEnclosingRect(const Rect& rect, float scale) {
return ScaleToEnclosingRect(rect, scale, scale); return ScaleToEnclosingRect(rect, scale, scale);
} }
// ScaleToEnclosingRect but clamping instead of asserting if the resulting rect
// would overflow.
inline Rect ScaleToEnclosingRectSafe(const Rect& rect,
float x_scale,
float y_scale) {
if (x_scale == 1.f && y_scale == 1.f)
return rect;
int x = base::saturated_cast<int>(std::floor(rect.x() * x_scale));
int y = base::saturated_cast<int>(std::floor(rect.y() * y_scale));
int w = base::saturated_cast<int>(std::ceil(rect.width() * x_scale));
int h = base::saturated_cast<int>(std::ceil(rect.height() * y_scale));
return Rect(x, y, w, h);
}
inline Rect ScaleToEnclosingRectSafe(const Rect& rect, float scale) {
return ScaleToEnclosingRectSafe(rect, scale, scale);
}
inline Rect ScaleToEnclosedRect(const Rect& rect, inline Rect ScaleToEnclosedRect(const Rect& rect,
float x_scale, float x_scale,
float y_scale) { float y_scale) {
......
...@@ -683,9 +683,12 @@ TEST(RectTest, ScaleToEnclosingRect) { ...@@ -683,9 +683,12 @@ TEST(RectTest, ScaleToEnclosingRect) {
}; };
for (size_t i = 0; i < arraysize(tests); ++i) { for (size_t i = 0; i < arraysize(tests); ++i) {
Rect result = ScaleToEnclosingRect(tests[i].input_rect, Rect result =
tests[i].input_scale); ScaleToEnclosingRect(tests[i].input_rect, tests[i].input_scale);
EXPECT_EQ(tests[i].expected_rect, result); EXPECT_EQ(tests[i].expected_rect, result);
Rect result_safe =
ScaleToEnclosingRectSafe(tests[i].input_rect, tests[i].input_scale);
EXPECT_EQ(tests[i].expected_rect, result_safe);
} }
} }
...@@ -983,4 +986,44 @@ TEST(RectTest, IntegerOverflow) { ...@@ -983,4 +986,44 @@ TEST(RectTest, IntegerOverflow) {
EXPECT_EQ(expected_size, operator_overflow.size()); EXPECT_EQ(expected_size, operator_overflow.size());
} }
TEST(RectTest, ScaleToEnclosingRectSafe) {
const int max_int = std::numeric_limits<int>::max();
const int min_int = std::numeric_limits<int>::min();
Rect xy_underflow(-100000, -123456, 10, 20);
EXPECT_EQ(ScaleToEnclosingRectSafe(xy_underflow, 100000, 100000),
Rect(min_int, min_int, 1000000, 2000000));
// A location overflow means that width/right and bottom/top also
// overflow so need to be clamped.
Rect xy_overflow(100000, 123456, 10, 20);
EXPECT_EQ(ScaleToEnclosingRectSafe(xy_overflow, 100000, 100000),
Rect(max_int, max_int, 0, 0));
// In practice all rects are clamped to 0 width / 0 height so
// negative sizes don't matter, but try this for the sake of testing.
Rect size_underflow(-1, -2, 100000, 100000);
EXPECT_EQ(ScaleToEnclosingRectSafe(size_underflow, -100000, -100000),
Rect(100000, 200000, 0, 0));
Rect size_overflow(-1, -2, 123456, 234567);
EXPECT_EQ(ScaleToEnclosingRectSafe(size_overflow, 100000, 100000),
Rect(-100000, -200000, max_int, max_int));
// Verify width/right gets clamped properly too if x/y positive.
Rect size_overflow2(1, 2, 123456, 234567);
EXPECT_EQ(ScaleToEnclosingRectSafe(size_overflow2, 100000, 100000),
Rect(100000, 200000, max_int - 100000, max_int - 200000));
Rect max_rect(max_int, max_int, max_int, max_int);
EXPECT_EQ(ScaleToEnclosingRectSafe(max_rect, max_int, max_int),
Rect(max_int, max_int, 0, 0));
Rect min_rect(min_int, min_int, max_int, max_int);
// Min rect can't be scaled up any further in any dimension.
EXPECT_EQ(ScaleToEnclosingRectSafe(min_rect, 2, 3.5), min_rect);
EXPECT_EQ(ScaleToEnclosingRectSafe(min_rect, max_int, max_int), min_rect);
// Min rect scaled by min is an empty rect at (max, max)
EXPECT_EQ(ScaleToEnclosingRectSafe(min_rect, min_int, min_int), max_rect);
}
} // namespace gfx } // namespace gfx
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment