Clip visual rect with real overflow clip rect

third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp

Index: third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp b/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp
index f56793a13f7f90d59823aaf3442f7c279eaaa899..cc10e038e5df380ae3372875dbebbccbd477aa25 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp
@@ -247,11 +247,88 @@ TEST_F(LayoutObjectTest, OverflowRectMappingWithContainerFlippedWritingMode)
     LayoutRect containerOverflowRect = container->localOverflowRectForPaintInvalidation();
     EXPECT_EQ(LayoutRect(-40, 0, 240, 110), containerOverflowRect);
     rect = containerOverflowRect;
-    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(container, rect));
+    EXPECT_TRUE(container->mapToVisualRectInAncestorSpace(container, rect));

[chrishtr, 2016/04/04 16:33:16]

Why did you change two call sites in this test?

[Xianzhu, 2016/04/04 17:46:58]

The previous code was incorrect. We should never map container's overflow rect
by calling target's mapToVisualRectInAncestorSpace(). The result happened to be
correct because container and target have the same origin.

     EXPECT_EQ(LayoutRect(0, 0, 240, 110), rect);
     rect = containerOverflowRect;
-    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(&layoutView(), rect));
+    EXPECT_TRUE(container->mapToVisualRectInAncestorSpace(&layoutView(), rect));
     EXPECT_EQ(LayoutRect(222, 111, 240, 110), rect);
 }
 
+TEST_F(LayoutObjectTest, OverflowRectMappingWithContainerOverflowClip)
+{
+    setBodyInnerHTML(
+        "<div id='container' style='position: absolute; top: 111px; left: 222px;"
+        "    border: 10px solid red; overflow: hidden; width: 50px; height: 80px;'>"
+        "    <div id='target' style='box-shadow: 40px 20px black; width: 100px; height: 90px'></div>"
+        "</div>");
+
+    LayoutBlock* container = toLayoutBlock(getLayoutObjectByElementId("container"));
+    EXPECT_EQ(LayoutUnit(0), container->scrollTop());
+    EXPECT_EQ(LayoutUnit(0), container->scrollLeft());
+    container->setScrollTop(LayoutUnit(7));
+    container->setScrollLeft(LayoutUnit(8));
+
+    LayoutBlock* target = toLayoutBlock(getLayoutObjectByElementId("target"));
+    LayoutRect targetOverflowRect = target->localOverflowRectForPaintInvalidation();
+    LayoutRect rect = targetOverflowRect;

[chrishtr, 2016/04/04 16:33:16]

Add an assert about what targetOverflowRect is equal to before mapping.

[Xianzhu, 2016/04/04 17:46:58]

Done.

+    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(container, rect));
+    EXPECT_EQ(LayoutRect(2, 3, 140, 110), rect);

[chrishtr, 2016/04/04 16:33:15]

Why 2,3?

Add a quick note that this is 140x110 because it includes the box-shadow
overflow, and is
 not clipped by the clip rect of 'container'

[Xianzhu, 2016/04/04 17:46:58]

Done.

+    rect = targetOverflowRect;
+    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(&layoutView(), rect));
+    EXPECT_EQ(LayoutRect(232, 121, 50, 80), rect);

[chrishtr, 2016/04/04 16:33:16]

Add a quick note that this adjusts for the top/left of 'container' and applies
its clip of
50x80.

[Xianzhu, 2016/04/04 17:46:58]

Done.

+
+    LayoutRect containerOverflowRect = container->localOverflowRectForPaintInvalidation();
+    EXPECT_EQ(LayoutRect(0, 0, 70, 100), containerOverflowRect);

[chrishtr, 2016/04/04 16:33:16]

Why 70x100? Doesn't it include the overflow from 'target'?

[Xianzhu, 2016/04/04 17:46:58]

No. The container has overflow clip, so its visual overflow doesn't include
overflow from descendants.

+    rect = containerOverflowRect;
+    EXPECT_TRUE(container->mapToVisualRectInAncestorSpace(container, rect));
+    EXPECT_EQ(LayoutRect(0, 0, 70, 100), rect);
+    rect = containerOverflowRect;
+    EXPECT_TRUE(container->mapToVisualRectInAncestorSpace(&layoutView(), rect));
+    EXPECT_EQ(LayoutRect(222, 111, 70, 100), rect);

[chrishtr, 2016/04/04 16:33:15]

No clip applied?

[Xianzhu, 2016/04/04 17:46:58]

Right. We should not apply container's overflow clip on container's own overflow
rect, otherwise we would clip off its border.

+}
+
+TEST_F(LayoutObjectTest, OverflowRectMappingWithContainerFlippedWritingModeAndOverflowClip)
+{
+    setBodyInnerHTML(
+        "<div id='container' style='writing-mode: vertical-rl; position: absolute; top: 111px; left: 222px;"
+        "    border: solid red; border-width: 10px 20px 30px 40px;"
+        "    overflow: hidden; width: 50px; height: 80px'>"
+        "    <div id='target' style='box-shadow: 40px 20px black; width: 100px; height: 90px'></div>"
+        "    <div style='width: 100px; height: 100px'></div>"
+        "</div>");
+
+    LayoutBlock* container = toLayoutBlock(getLayoutObjectByElementId("container"));
+    EXPECT_EQ(LayoutUnit(0), container->scrollTop());
+    EXPECT_EQ(LayoutUnit(150), container->scrollLeft());
+    container->setScrollTop(LayoutUnit(7));
+    container->setScrollLeft(container->scrollLeft() - 8);
+
+    LayoutBlock* target = toLayoutBlock(getLayoutObjectByElementId("target"));
+    LayoutRect targetOverflowRect = target->localOverflowRectForPaintInvalidation();
+    EXPECT_EQ(LayoutRect(-40, 0, 140, 110), targetOverflowRect);
+    LayoutRect rect = targetOverflowRect;
+    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(target, rect));
+    EXPECT_EQ(LayoutRect(0, 0, 140, 110), rect);
+
+    rect = targetOverflowRect;
+    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(container, rect));
+    EXPECT_EQ(LayoutRect(-2, 3, 140, 110), rect);
+    rect = targetOverflowRect;
+    EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(&layoutView(), rect));
+    // TODO(crbug.com/600039): rect.x() should be 262 (left + border-left), but is offset
+    // by extra horizontal border-widths because of layout error.
+    EXPECT_EQ(LayoutRect(322, 121, 50, 80), rect);
+
+    LayoutRect containerOverflowRect = container->localOverflowRectForPaintInvalidation();
+    EXPECT_EQ(LayoutRect(0, 0, 110, 120), containerOverflowRect);
+    rect = containerOverflowRect;
+    EXPECT_TRUE(container->mapToVisualRectInAncestorSpace(container, rect));
+    EXPECT_EQ(LayoutRect(0, 0, 110, 120), rect);
+    rect = containerOverflowRect;
+    EXPECT_TRUE(container->mapToVisualRectInAncestorSpace(&layoutView(), rect));
+    // TODO(crbug.com/600039): rect.x() should be 222 (left), but is offset by extra horizontal
+    // border-widths because of layout error.
+    EXPECT_EQ(LayoutRect(282, 111, 110, 120), rect);
+}
+
 } // namespace blink