Commit 65216b2c authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

Geometry: Ensure to check for infinite "other" clip rect in FloatClipRect.

Since the infinite float clip rect is actually pretty small
(it's on the order of 10s of millions), due to fixed point math in layout
unit, it can cause clips on other rects that are computed elsewhere,
such as IntersectionObserver. This can cause bugs (see referenced bug)
where unclipped elements are treated as clipped by the intersection
observer.

R=chrishtr@chromium.org, szager@chromium.org

Bug: 1004196
Change-Id: Ia3e5d483cded6153ff546289fbc762b6e8922bc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825579Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700259}
parent 83351cbc
...@@ -44,6 +44,9 @@ class PLATFORM_EXPORT FloatClipRect { ...@@ -44,6 +44,9 @@ class PLATFORM_EXPORT FloatClipRect {
} }
void Intersect(const FloatClipRect& other) { void Intersect(const FloatClipRect& other) {
if (other.is_infinite_)
return;
if (is_infinite_) { if (is_infinite_) {
is_infinite_ = other.is_infinite_; is_infinite_ = other.is_infinite_;
rect_ = other.rect_; rect_ = other.rect_;
...@@ -56,7 +59,12 @@ class PLATFORM_EXPORT FloatClipRect { ...@@ -56,7 +59,12 @@ class PLATFORM_EXPORT FloatClipRect {
ClearIsTight(); ClearIsTight();
} }
// See FloatRect::InclusiveIntersect for description of the return value.
// TL;DR, this returns true if rects actually intersect.
bool InclusiveIntersect(const FloatClipRect& other) { bool InclusiveIntersect(const FloatClipRect& other) {
if (other.is_infinite_)
return true;
bool retval = true; bool retval = true;
if (is_infinite_) { if (is_infinite_) {
is_infinite_ = other.is_infinite_; is_infinite_ = other.is_infinite_;
......
...@@ -13,7 +13,7 @@ class FloatClipRectTest : public testing::Test { ...@@ -13,7 +13,7 @@ class FloatClipRectTest : public testing::Test {
public: public:
}; };
TEST_F(FloatClipRectTest, InfinitRect) { TEST_F(FloatClipRectTest, InfiniteRect) {
FloatClipRect rect; FloatClipRect rect;
EXPECT_TRUE(rect.IsInfinite()); EXPECT_TRUE(rect.IsInfinite());
EXPECT_FALSE(rect.HasRadius()); EXPECT_FALSE(rect.HasRadius());
...@@ -60,6 +60,28 @@ TEST_F(FloatClipRectTest, Intersect) { ...@@ -60,6 +60,28 @@ TEST_F(FloatClipRectTest, Intersect) {
EXPECT_FALSE(rect.IsTight()); EXPECT_FALSE(rect.IsTight());
} }
TEST_F(FloatClipRectTest, IntersectWithInfinite) {
FloatClipRect infinite;
FloatRect large(0, 0, std::numeric_limits<int>::max(),
std::numeric_limits<int>::max());
FloatClipRect unclipped(large);
unclipped.Intersect(infinite);
EXPECT_FALSE(unclipped.IsInfinite());
EXPECT_EQ(large, unclipped.Rect());
}
TEST_F(FloatClipRectTest, InclusiveIntersectWithInfinite) {
FloatClipRect infinite;
FloatRect large(0, 0, std::numeric_limits<int>::max(),
std::numeric_limits<int>::max());
FloatClipRect unclipped(large);
ASSERT_TRUE(unclipped.InclusiveIntersect(infinite));
EXPECT_FALSE(unclipped.IsInfinite());
EXPECT_EQ(large, unclipped.Rect());
}
TEST_F(FloatClipRectTest, SetHasRadius) { TEST_F(FloatClipRectTest, SetHasRadius) {
FloatClipRect rect; FloatClipRect rect;
rect.SetHasRadius(); rect.SetHasRadius();
......
...@@ -799,6 +799,8 @@ TEST_P(GeometryMapperTest, InvertedClip) { ...@@ -799,6 +799,8 @@ TEST_P(GeometryMapperTest, InvertedClip) {
PropertyTreeState dest(t0(), *clip, e0()); PropertyTreeState dest(t0(), *clip, e0());
FloatClipRect visual_rect(FloatRect(0, 0, 10, 200)); FloatClipRect visual_rect(FloatRect(0, 0, 10, 200));
EXPECT_TRUE(visual_rect.IsTight());
GeometryMapper::LocalToAncestorVisualRect(PropertyTreeState::Root(), dest, GeometryMapper::LocalToAncestorVisualRect(PropertyTreeState::Root(), dest,
visual_rect); visual_rect);
...@@ -806,7 +808,7 @@ TEST_P(GeometryMapperTest, InvertedClip) { ...@@ -806,7 +808,7 @@ TEST_P(GeometryMapperTest, InvertedClip) {
// LocalToAncestorVisualRect must fall back to the original rect, mapped // LocalToAncestorVisualRect must fall back to the original rect, mapped
// into the root space. // into the root space.
EXPECT_EQ(FloatRect(0, 0, 10, 200), visual_rect.Rect()); EXPECT_EQ(FloatRect(0, 0, 10, 200), visual_rect.Rect());
EXPECT_FALSE(visual_rect.IsTight()); EXPECT_TRUE(visual_rect.IsTight());
} }
TEST_P(GeometryMapperTest, Precision) { TEST_P(GeometryMapperTest, Precision) {
......
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