[SPInvalidation] Fix PrePaintTreeWalk for multicol spanner

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp

Index: third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
index d364b4aae8502a176c4cf733c207622263586046..c7320573a192ff180e61ce0ebedd22e508afd146 100644
--- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
@@ -71,6 +71,12 @@ class PaintPropertyTreeBuilderTest
     return frameView->scroll();
   }
 
+  LayoutPoint paintOffset(const LayoutObject* object) {
+    return object->objectPaintProperties()
+        ->localBorderBoxProperties()
+        ->paintOffset;
+  }
+
  private:
   void SetUp() override {
     Settings::setMockScrollbarsEnabled(true);
@@ -2739,4 +2745,58 @@ TEST_P(PaintPropertyTreeBuilderTest,
               MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
 }
 
+TEST_P(PaintPropertyTreeBuilderTest, PaintOffsetUnderMultiColumn) {

[chrishtr, 2016/10/05 00:48:17]

@mstensho could you please review this test and verify Xianzhu's calculations as
well as his comments
about some of them being incorrect?

+  setBodyInnerHTML(
+      "<style>"
+      "  body { margin: 0; }"
+      "  .space { height: 30px; }"
+      "  .abs { position: absolute; width: 20px; height: 20px; }"
+      "</style>"
+      "<div style='columns:2; width: 200px; column-gap: 0'>"
+      "  <div style='position: relative'>"
+      "    <div id=space1 class=space></div>"
+      "    <div id=space2 class=space></div>"
+      "    <div id=spanner style='column-span: all'>"
+      "      <div id=normal style='height: 50px'>"

[mstensho (USE GERRIT), 2016/10/05 08:44:09]

Missing </div> ?

[Xianzhu, 2016/10/05 17:29:40]

Done.

+      "      <div id=top-left class=abs style='top: 0; left: 0'></div>"
+      "      <div id=bottom-right class=abs style='bottom: 0; right: 0'></div>"
+      "    </div>"
+      "    <div id=space3 class=space></div>"

[mstensho (USE GERRIT), 2016/10/05 08:44:09]

I don't think you intended for this to be inside the spanner? At least that's
what the indentation suggests.

[Xianzhu, 2016/10/05 17:29:40]

Right.

+      "    <div id=space4 class=space></div>"
+      "  </div>"
+      "</div>");
+
+  // Above the spanner.
+  // Column 1.
+  EXPECT_EQ(LayoutPoint(), paintOffset(getLayoutObjectByElementId("space1")));
+  // Column 2. TODO(crbug.com/648274): This is incorrect. Should be (50, 0).

[mstensho (USE GERRIT), 2016/10/05 08:44:09]

What should it be relative to? LayoutPoint(0, 30) suggests that it's relative to
the flow thread. As far as I can read from the documentation, it should be
relative to "the propertyTreeState's transform space". Does that mean the
viewport in this case, since we have no transforms?

If it should be something else, I don't think (50, 0) is right. Perhaps (100,
0). Column width is 100px, after all.

[Xianzhu, 2016/10/05 17:29:40]

Changed the comment to (100, 0).

Made the mistake (as well as other should-be's) using 50px column width.

+  EXPECT_EQ(LayoutPoint(0, 30),
+            paintOffset(getLayoutObjectByElementId("space2")));
+
+  // The spanner's normal flow.
+  EXPECT_EQ(LayoutPoint(0, 30),
+            paintOffset(getLayoutObjectByElementId("spanner")));
+  EXPECT_EQ(LayoutPoint(0, 30),
+            paintOffset(getLayoutObjectByElementId("normal")));
+
+  // Below the spanner.
+  // Column 1.
+  EXPECT_EQ(LayoutPoint(0, 80),
+            paintOffset(getLayoutObjectByElementId("space3")));
+  // Column 2. TODO(crbug.com/648274): This is incorrect. Should be (50, 80).

[mstensho (USE GERRIT), 2016/10/05 08:44:09]

(100, 80), perhaps?

[Xianzhu, 2016/10/05 17:29:40]

Done.

+  EXPECT_EQ(LayoutPoint(0, 110),
+            paintOffset(getLayoutObjectByElementId("space4")));
+
+  // Out-of-flow positioned descendants of the spanner. They are laid out in
+  // the relative-position container.
+  // This should be aligned to the top-left corner of space1.
+  EXPECT_EQ(LayoutPoint(0, 30),

[mstensho (USE GERRIT), 2016/10/05 08:44:09]

Shouldn't this be (0,0) then?

+            paintOffset(getLayoutObjectByElementId("top-left")));
+
+  // This should be aligned to the bottom-right corner of space4.
+  // TODO(crbug.com/648274): This is incorrect. Should be (80, 90).

[mstensho (USE GERRIT), 2016/10/05 08:44:09]

Probably (180, 90) then.

[Xianzhu, 2016/10/05 17:29:40]

Done.

+  EXPECT_EQ(LayoutPoint(80, 40),
+            paintOffset(getLayoutObjectByElementId("bottom-right")));
+}
+
 }  // namespace blink