Clip visual rect with real overflow clip rect

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

 // Copyright 2015 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 "core/layout/LayoutObject.h"
 
 #include "core/layout/LayoutTestHelper.h"
 #include "core/layout/LayoutView.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ skipping 229 matching lines @@
     rect = targetOverflowRect;
     EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(container, rect));
     EXPECT_EQ(LayoutRect(100, 0, 140, 110), rect);
     rect = targetOverflowRect;
     EXPECT_TRUE(target->mapToVisualRectInAncestorSpace(&layoutView(), rect));
     EXPECT_EQ(LayoutRect(322, 111, 140, 110), rect);
 
     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