[PE] Avoid integer overflows in EnclosingIntRect(const FloatRect&)
The size computation could potentially overflow, yielding an "empty"
result, which could have different consequences depending on the user.
In CullRect::UpdateCullRect for instance, the MapRect call could make
the cull rectangle appear empty causing visible elements to be culled.
Rather than using integers for the computing the enclosing rect, use
floats, and then cast that to an IntRect at the end. (The
IntRect(const FloatRect&) constructor clamps the input using clampTo.)
Adjust EnclosedIntRect similarly because it shares the same deficiency.
Bug: 793676
Change-Id: I63bcff3747dea7f29dac39a05d9cf179b0770030
Reviewed-on: https://chromium-review.googlesource.com/839521
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525812}diff --git a/third_party/WebKit/Source/platform/geometry/FloatRect.cpp b/third_party/WebKit/Source/platform/geometry/FloatRect.cpp
index 0617c83c..e3b6002 100644
--- a/third_party/WebKit/Source/platform/geometry/FloatRect.cpp
+++ b/third_party/WebKit/Source/platform/geometry/FloatRect.cpp
@@ -210,13 +210,24 @@
return result;
}
-IntRect EnclosedIntRect(const FloatRect& rect) {
- IntPoint location = CeiledIntPoint(rect.MinXMinYCorner());
- IntPoint max_point = FlooredIntPoint(rect.MaxXMaxYCorner());
- IntSize size = max_point - location;
- size.ClampNegativeToZero();
+IntRect EnclosingIntRect(const FloatRect& rect) {
+ // Compute the enclosing rect using float types directly rather than
+ // FlooredIntPoint(...) et.c to avoid triggering integer overflows.
+ FloatPoint location(floorf(rect.X()), floorf(rect.Y()));
+ FloatPoint max_point(ceilf(rect.MaxX()), ceilf(rect.MaxY()));
+ FloatRect enclosing_rect(location, max_point - location);
+ return IntRect(enclosing_rect);
+}
- return IntRect(location, size);
+IntRect EnclosedIntRect(const FloatRect& rect) {
+ // Compute the enclosed rect using float types directly rather than
+ // FlooredIntPoint(...) et.c to avoid triggering integer overflows.
+ FloatPoint location(ceilf(rect.X()), ceilf(rect.Y()));
+ FloatPoint max_point(floorf(rect.MaxX()), floorf(rect.MaxY()));
+ FloatSize size = max_point - location;
+ size.ClampNegativeToZero();
+ FloatRect enclosed_rect(location, size);
+ return IntRect(enclosed_rect);
}
IntRect RoundedIntRect(const FloatRect& rect) {
diff --git a/third_party/WebKit/Source/platform/geometry/FloatRect.h b/third_party/WebKit/Source/platform/geometry/FloatRect.h
index 0be6fa1..1bf5932 100644
--- a/third_party/WebKit/Source/platform/geometry/FloatRect.h
+++ b/third_party/WebKit/Source/platform/geometry/FloatRect.h
@@ -236,12 +236,8 @@
return a.Location() != b.Location() || a.Size() != b.Size();
}
-inline IntRect EnclosingIntRect(const FloatRect& rect) {
- IntPoint location = FlooredIntPoint(rect.MinXMinYCorner());
- IntPoint max_point = CeiledIntPoint(rect.MaxXMaxYCorner());
-
- return IntRect(location, max_point - location);
-}
+// Returns a IntRect containing the given FloatRect.
+PLATFORM_EXPORT IntRect EnclosingIntRect(const FloatRect&);
// Returns a valid IntRect contained within the given FloatRect.
PLATFORM_EXPORT IntRect EnclosedIntRect(const FloatRect&);
diff --git a/third_party/WebKit/Source/platform/geometry/FloatRectTest.cpp b/third_party/WebKit/Source/platform/geometry/FloatRectTest.cpp
index e9a45aa9..64c5bee18 100644
--- a/third_party/WebKit/Source/platform/geometry/FloatRectTest.cpp
+++ b/third_party/WebKit/Source/platform/geometry/FloatRectTest.cpp
@@ -161,4 +161,100 @@
EXPECT_EQ("0,0 nanxnan", nan_rect.ToString());
}
+TEST(FloatRectTest, EnclosingIntRect) {
+ FloatRect small_dimensions_rect(42.5f, 84.5f,
+ std::numeric_limits<float>::epsilon(),
+ std::numeric_limits<float>::epsilon());
+ EXPECT_EQ(IntRect(42, 84, 1, 1), EnclosingIntRect(small_dimensions_rect));
+
+ FloatRect integral_rect(100, 150, 200, 350);
+ EXPECT_EQ(IntRect(100, 150, 200, 350), EnclosingIntRect(integral_rect));
+
+ FloatRect fractional_pos_rect(100.6f, 150.8f, 200, 350);
+ EXPECT_EQ(IntRect(100, 150, 201, 351), EnclosingIntRect(fractional_pos_rect));
+
+ FloatRect fractional_dimensions_rect(100, 150, 200.6f, 350.4f);
+ EXPECT_EQ(IntRect(100, 150, 201, 351),
+ EnclosingIntRect(fractional_dimensions_rect));
+
+ FloatRect fractional_both_rect1(100.6f, 150.8f, 200.4f, 350.2f);
+ EXPECT_EQ(IntRect(100, 150, 201, 351),
+ EnclosingIntRect(fractional_both_rect1));
+
+ FloatRect fractional_both_rect2(100.6f, 150.8f, 200.3f, 350.3f);
+ EXPECT_EQ(IntRect(100, 150, 201, 352),
+ EnclosingIntRect(fractional_both_rect2));
+
+ FloatRect fractional_both_rect3(100.6f, 150.8f, 200.5f, 350.3f);
+ EXPECT_EQ(IntRect(100, 150, 202, 352),
+ EnclosingIntRect(fractional_both_rect3));
+
+ FloatRect fractional_negpos_rect1(-100.4f, -150.8f, 200, 350);
+ EXPECT_EQ(IntRect(-101, -151, 201, 351),
+ EnclosingIntRect(fractional_negpos_rect1));
+
+ FloatRect fractional_negpos_rect2(-100.4f, -150.8f, 199.4f, 350.3f);
+ EXPECT_EQ(IntRect(-101, -151, 200, 351),
+ EnclosingIntRect(fractional_negpos_rect2));
+
+ FloatRect fractional_negpos_rect3(-100.6f, -150.8f, 199.6f, 350.3f);
+ EXPECT_EQ(IntRect(-101, -151, 201, 351),
+ EnclosingIntRect(fractional_negpos_rect3));
+
+ FloatRect max_rect(-std::numeric_limits<float>::max() / 2,
+ -std::numeric_limits<float>::max() / 2,
+ std::numeric_limits<float>::max(),
+ std::numeric_limits<float>::max());
+ EXPECT_EQ(IntRect(INT_MIN, INT_MIN, INT_MAX, INT_MAX),
+ EnclosingIntRect(max_rect));
+}
+
+TEST(FloatRectTest, EnclosedIntRect) {
+ FloatRect small_dimensions_rect(42.5f, 84.5f,
+ std::numeric_limits<float>::epsilon(),
+ std::numeric_limits<float>::epsilon());
+ EXPECT_EQ(IntRect(43, 85, 0, 0), EnclosedIntRect(small_dimensions_rect));
+
+ FloatRect integral_rect(100, 150, 200, 350);
+ EXPECT_EQ(IntRect(100, 150, 200, 350), EnclosedIntRect(integral_rect));
+
+ FloatRect fractional_pos_rect(100.6f, 150.8f, 200, 350);
+ EXPECT_EQ(IntRect(101, 151, 199, 349), EnclosedIntRect(fractional_pos_rect));
+
+ FloatRect fractional_dimensions_rect(100, 150, 200.6f, 350.4f);
+ EXPECT_EQ(IntRect(100, 150, 200, 350),
+ EnclosedIntRect(fractional_dimensions_rect));
+
+ FloatRect fractional_both_rect1(100.6f, 150.8f, 200.4f, 350.2f);
+ EXPECT_EQ(IntRect(101, 151, 200, 350),
+ EnclosedIntRect(fractional_both_rect1));
+
+ FloatRect fractional_both_rect2(100.6f, 150.8f, 200.3f, 350.3f);
+ EXPECT_EQ(IntRect(101, 151, 199, 350),
+ EnclosedIntRect(fractional_both_rect2));
+
+ FloatRect fractional_both_rect3(100.6f, 150.8f, 200.5f, 350.3f);
+ EXPECT_EQ(IntRect(101, 151, 200, 350),
+ EnclosedIntRect(fractional_both_rect3));
+
+ FloatRect fractional_negpos_rect1(-100.4f, -150.8f, 200, 350);
+ EXPECT_EQ(IntRect(-100, -150, 199, 349),
+ EnclosedIntRect(fractional_negpos_rect1));
+
+ FloatRect fractional_negpos_rect2(-100.4f, -150.8f, 199.5f, 350.3f);
+ EXPECT_EQ(IntRect(-100, -150, 199, 349),
+ EnclosedIntRect(fractional_negpos_rect2));
+
+ FloatRect fractional_negpos_rect3(-100.6f, -150.8f, 199.6f, 350.3f);
+ EXPECT_EQ(IntRect(-100, -150, 199, 349),
+ EnclosedIntRect(fractional_negpos_rect3));
+
+ FloatRect max_rect(-std::numeric_limits<float>::max() / 2,
+ -std::numeric_limits<float>::max() / 2,
+ std::numeric_limits<float>::max(),
+ std::numeric_limits<float>::max());
+ EXPECT_EQ(IntRect(INT_MIN, INT_MIN, INT_MAX, INT_MAX),
+ EnclosedIntRect(max_rect));
+}
+
} // namespace blink
diff --git a/third_party/WebKit/Source/platform/geometry/FloatSize.h b/third_party/WebKit/Source/platform/geometry/FloatSize.h
index 3e4466c13..5b1880b5 100644
--- a/third_party/WebKit/Source/platform/geometry/FloatSize.h
+++ b/third_party/WebKit/Source/platform/geometry/FloatSize.h
@@ -102,6 +102,8 @@
height_ < other.height_ ? height_ : other.height_);
}
+ void ClampNegativeToZero() { *this = ExpandedTo(FloatSize()); }
+
float DiagonalLength() const;
float DiagonalLengthSquared() const {
return width_ * width_ + height_ * height_;
