Untangle multicol coordinate space conversion from offsetFromContainer().

The various offsetFromContainer() implementations used to convert from flow
thread coordinates to visual coordinates if the container was a flow thread.
That works when mapping a position relatively to some ancestor, but not when
mapping a position relatively to some descendant. Put differently: It works
fine when walking upwards in a tree, but not so fine when walking it downwards
(we need the opposite operation in that case; convert from visual to flow
thread coordinates). That was the reason for some mess in mapAncestorToLocal(),
since we had to cancel out the shenanigans carried out by
offsetFromContainer().

So, instead, perform this flowthread-to-visual coordinate space conversion
where we need it, and don't cause trouble for those who don't need it.

No behavior changes intended. This is also why we're keeping this coordinate
space conversion in CaretBase for now, even if it's wrong (see bug 596070).
Simply removing that *now* wouldn't fix the bug anyway, just alter it (probably
for the better, but who knows -- still buggy). A proper fix will land shortly.

BUG=568492
R=leviw@chromium.org

Review URL: https://codereview.chromium.org/1820483002

Cr-Commit-Position: refs/heads/master@{#382153}

11 files changed, 44 insertions, 50 deletions

diff --git a/third_party/WebKit/Source/core/editing/CaretBase.cpp b/third_party/WebKit/Source/core/editing/CaretBase.cpp
index c448ac2..cbf5579 100644
--- a/third_party/WebKit/Source/core/editing/CaretBase.cpp
+++ b/third_party/WebKit/Source/core/editing/CaretBase.cpp
@@ -82,7 +82,13 @@ static void mapCaretRectToCaretPainter(LayoutItem caretLayoutItem, LayoutBlockIt
             unrooted = true;
             break;
         }
-        caretRect.move(caretLayoutItem.offsetFromContainer(containerItem, caretRect.location()));
+        caretRect.move(caretLayoutItem.offsetFromContainer(containerItem));
+
+        // TODO(mstensho): Fix crbug.com/596070 and get rid of this flowthread/multicol thing
+        // here. It was added to keep the same behavior as before flowthread-to-visual coordinate
+        // space conversion was moved out from offsetFromContainer() to mapLocalToAncestor().
+        caretRect.move(containerItem.columnOffset(caretRect.location()));
+
         caretLayoutItem = containerItem;
     }
 
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index 9fac7fe..fed747b 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -1707,7 +1707,7 @@ void LayoutBox::mapAncestorToLocal(const LayoutBoxModelObject* ancestor, Transfo
     LayoutBoxModelObject::mapAncestorToLocal(ancestor, transformState, mode);
 }
 
-LayoutSize LayoutBox::offsetFromContainer(const LayoutObject* o, const LayoutPoint& point, bool* offsetDependsOnPoint) const
+LayoutSize LayoutBox::offsetFromContainer(const LayoutObject* o) const
 {
     ASSERT(o == container());
 
@@ -1716,15 +1716,6 @@ LayoutSize LayoutBox::offsetFromContainer(const LayoutObject* o, const LayoutPoi
         offset += offsetForInFlowPosition();
 
     offset += topLeftLocationOffset();
-    if (o->isLayoutFlowThread()) {
-        // So far the point has been in flow thread coordinates (i.e. as if everything in
-        // the fragmentation context lived in one tall single column). Convert it to a
-        // visual point now.
-        LayoutPoint pointInContainer = point + offset;
-        offset += o->columnOffset(pointInContainer);
-        if (offsetDependsOnPoint)
-            *offsetDependsOnPoint = true;
-    }
 
     if (o->hasOverflowClip())
         offset -= toLayoutBox(o)->scrolledContentOffset();
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.h b/third_party/WebKit/Source/core/layout/LayoutBox.h
index 9f2bd0b..007845e 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.h
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.h
@@ -531,7 +531,7 @@ public:
     void setExtraBlockOffset(LayoutUnit blockOffest);
     void clearExtraInlineAndBlockOffests();
 
-    LayoutSize offsetFromContainer(const LayoutObject*, const LayoutPoint&, bool* offsetDependsOnPoint = nullptr) const override;
+    LayoutSize offsetFromContainer(const LayoutObject*) const override;
 
     LayoutUnit adjustBorderBoxLogicalWidthForBoxSizing(float width) const;
     LayoutUnit adjustBorderBoxLogicalHeightForBoxSizing(float height) const;
diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
index 155ea06..0042a5e 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
@@ -864,8 +864,14 @@ const LayoutObject* LayoutBoxModelObject::pushMappingToContainer(const LayoutBox
         adjustmentForSkippedAncestor = -ancestorToStopAt->offsetFromAncestorContainer(container);
     }
 
-    bool offsetDependsOnPoint = false;
-    LayoutSize containerOffset = offsetFromContainer(container, LayoutPoint(), &offsetDependsOnPoint);
+    LayoutSize containerOffset = offsetFromContainer(container);
+    bool offsetDependsOnPoint;
+    if (container->isLayoutFlowThread()) {
+        containerOffset += container->columnOffset(toLayoutPoint(containerOffset));
+        offsetDependsOnPoint = true;
+    } else {
+        offsetDependsOnPoint = container->style()->isFlippedBlocksWritingMode() && container->isBox();
+    }
 
     bool preserve3D = container->style()->preserves3D() || style()->preserves3D();
     GeometryInfoFlags flags = 0;
diff --git a/third_party/WebKit/Source/core/layout/LayoutInline.cpp b/third_party/WebKit/Source/core/layout/LayoutInline.cpp
index cc24729..7ba75d8 100644
--- a/third_party/WebKit/Source/core/layout/LayoutInline.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutInline.cpp
@@ -1128,7 +1128,7 @@ void LayoutInline::mapToVisibleRectInAncestorSpace(const LayoutBoxModelObject* a
     container->mapToVisibleRectInAncestorSpace(ancestor, rect, paintInvalidationState);
 }
 
-LayoutSize LayoutInline::offsetFromContainer(const LayoutObject* container, const LayoutPoint& point, bool* offsetDependsOnPoint) const
+LayoutSize LayoutInline::offsetFromContainer(const LayoutObject* container) const
 {
     ASSERT(container == this->container());
 
@@ -1136,14 +1136,9 @@ LayoutSize LayoutInline::offsetFromContainer(const LayoutObject* container, cons
     if (isInFlowPositioned())
         offset += offsetForInFlowPosition();
 
-    offset += container->columnOffset(point);
-
     if (container->hasOverflowClip())
         offset -= toLayoutBox(container)->scrolledContentOffset();
 
-    if (offsetDependsOnPoint)
-        *offsetDependsOnPoint = (container->isBox() && container->style()->isFlippedBlocksWritingMode()) || container->isLayoutFlowThread();
-
     return offset;
 }
 
diff --git a/third_party/WebKit/Source/core/layout/LayoutInline.h b/third_party/WebKit/Source/core/layout/LayoutInline.h
index 1df25d8..11a367c 100644
--- a/third_party/WebKit/Source/core/layout/LayoutInline.h
+++ b/third_party/WebKit/Source/core/layout/LayoutInline.h
@@ -140,7 +140,7 @@ public:
     void absoluteRects(Vector<IntRect>&, const LayoutPoint& accumulatedOffset) const final;
     void absoluteQuads(Vector<FloatQuad>&, bool* wasFixed) const override;
 
-    LayoutSize offsetFromContainer(const LayoutObject*, const LayoutPoint&, bool* offsetDependsOnPoint = nullptr) const final;
+    LayoutSize offsetFromContainer(const LayoutObject*) const final;
 
     IntRect linesBoundingBox() const;
     LayoutRect visualOverflowRect() const final;
diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.cpp b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
index 1941a74..0517995 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutObject.cpp
@@ -2267,7 +2267,14 @@ void LayoutObject::mapLocalToAncestor(const LayoutBoxModelObject* ancestor, Tran
         }
     }
 
-    LayoutSize containerOffset = offsetFromContainer(o, roundedLayoutPoint(transformState.mappedPoint()));
+    LayoutSize containerOffset = offsetFromContainer(o);
+    if (o->isLayoutFlowThread()) {
+        // So far the point has been in flow thread coordinates (i.e. as if everything in
+        // the fragmentation context lived in one tall single column). Convert it to a
+        // visual point now.
+        LayoutPoint pointInContainer = roundedLayoutPoint(transformState.mappedPoint()) + containerOffset;
+        containerOffset += o->columnOffset(pointInContainer);
+    }
 
     // Text objects just copy their parent's computed style, so we need to ignore them.
     bool preserve3D = mode & UseTransforms && ((o->style()->preserves3D() && !o->isText()) || (style()->preserves3D() && !isText()));
@@ -2319,17 +2326,11 @@ void LayoutObject::mapAncestorToLocal(const LayoutBoxModelObject* ancestor, Tran
     if (!containerSkipped)
         o->mapAncestorToLocal(ancestor, transformState, mode);
 
-    LayoutSize containerOffset = offsetFromContainer(o, LayoutPoint());
+    LayoutSize containerOffset = offsetFromContainer(o);
     if (o->isLayoutFlowThread()) {
         // Descending into a flow thread. Convert to the local coordinate space, i.e. flow thread coordinates.
-        const LayoutFlowThread* flowThread = toLayoutFlowThread(o);
         LayoutPoint visualPoint = LayoutPoint(transformState.mappedPoint());
-        transformState.move(visualPoint - flowThread->visualPointToFlowThreadPoint(visualPoint));
-        // |containerOffset| is also in visual coordinates. Convert to flow thread coordinates.
-        // TODO(mstensho): Wouldn't it be better add a parameter to instruct offsetFromContainer()
-        // to return flowthread coordinates in the first place? We're effectively performing two
-        // conversions here, when in fact none is needed.
-        containerOffset = toLayoutSize(flowThread->visualPointToFlowThreadPoint(toLayoutPoint(containerOffset)));
+        transformState.move(visualPoint - toLayoutFlowThread(o)->visualPointToFlowThreadPoint(visualPoint));
     }
 
     bool preserve3D = mode & UseTransforms && (o->style()->preserves3D() || style()->preserves3D());
@@ -2442,19 +2443,10 @@ FloatPoint LayoutObject::localToInvalidationBackingPoint(const LayoutPoint& loca
     return containerPoint;
 }
 
-LayoutSize LayoutObject::offsetFromContainer(const LayoutObject* o, const LayoutPoint& point, bool* offsetDependsOnPoint) const
+LayoutSize LayoutObject::offsetFromContainer(const LayoutObject* o) const
 {
     ASSERT(o == container());
-
-    LayoutSize offset = o->columnOffset(point);
-
-    if (o->hasOverflowClip())
-        offset -= toLayoutBox(o)->scrolledContentOffset();
-
-    if (offsetDependsOnPoint)
-        *offsetDependsOnPoint = o->isLayoutFlowThread();
-
-    return offset;
+    return o->hasOverflowClip() ? LayoutSize(-toLayoutBox(o)->scrolledContentOffset()) : LayoutSize();
 }
 
 LayoutSize LayoutObject::offsetFromAncestorContainer(const LayoutObject* ancestorContainer) const
@@ -2471,7 +2463,7 @@ LayoutSize LayoutObject::offsetFromAncestorContainer(const LayoutObject* ancesto
         if (!nextContainer)
             break;
         ASSERT(!currContainer->hasTransformRelatedProperty());
-        LayoutSize currentOffset = currContainer->offsetFromContainer(nextContainer, referencePoint);
+        LayoutSize currentOffset = currContainer->offsetFromContainer(nextContainer);
         offset += currentOffset;
         referencePoint.move(currentOffset);
         currContainer = nextContainer;
diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.h b/third_party/WebKit/Source/core/layout/LayoutObject.h
index 28431c2..3870583 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObject.h
+++ b/third_party/WebKit/Source/core/layout/LayoutObject.h
@@ -1008,9 +1008,8 @@ public:
     // Convert a local point into the coordinate system of backing coordinates. Also returns the backing layer if needed.
     FloatPoint localToInvalidationBackingPoint(const LayoutPoint&, PaintLayer** backingLayer = nullptr);
 
-    // Return the offset from the container() layoutObject (excluding transforms). In multi-column layout,
-    // different offsets apply at different points, so return the offset that applies to the given point.
-    virtual LayoutSize offsetFromContainer(const LayoutObject*, const LayoutPoint&, bool* offsetDependsOnPoint = nullptr) const;
+    // Return the offset from the container() layoutObject (excluding transforms and multicol).
+    virtual LayoutSize offsetFromContainer(const LayoutObject*) const;
     // Return the offset from an object up the container() chain. Asserts that none of the intermediate objects have transforms.
     LayoutSize offsetFromAncestorContainer(const LayoutObject*) const;
 
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
index f3851f4..618c650 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
@@ -304,11 +304,11 @@ void LayoutTableCell::setOverrideLogicalContentHeightFromRowHeight(LayoutUnit ro
     setOverrideLogicalContentHeight((rowHeight - borderAndPaddingLogicalHeight()).clampNegativeToZero());
 }
 
-LayoutSize LayoutTableCell::offsetFromContainer(const LayoutObject* o, const LayoutPoint& point, bool* offsetDependsOnPoint) const
+LayoutSize LayoutTableCell::offsetFromContainer(const LayoutObject* o) const
 {
     ASSERT(o == container());
 
-    LayoutSize offset = LayoutBlockFlow::offsetFromContainer(o, point, offsetDependsOnPoint);
+    LayoutSize offset = LayoutBlockFlow::offsetFromContainer(o);
     if (parent())
         offset -= parentBox()->locationOffset();
 
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableCell.h b/third_party/WebKit/Source/core/layout/LayoutTableCell.h
index 881f3eb..8f36bf0 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCell.h
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCell.h
@@ -290,7 +290,7 @@ private:
 
     bool boxShadowShouldBeAppliedToBackground(BackgroundBleedAvoidance, const InlineFlowBox*) const override;
 
-    LayoutSize offsetFromContainer(const LayoutObject*, const LayoutPoint&, bool* offsetDependsOnPoint = nullptr) const override;
+    LayoutSize offsetFromContainer(const LayoutObject*) const override;
     LayoutRect clippedOverflowRectForPaintInvalidation(const LayoutBoxModelObject* paintInvalidationContainer, const PaintInvalidationState* = nullptr) const override;
     void mapToVisibleRectInAncestorSpace(const LayoutBoxModelObject* ancestor, LayoutRect&, const PaintInvalidationState*) const override;
 
diff --git a/third_party/WebKit/Source/core/layout/api/LayoutItem.h b/third_party/WebKit/Source/core/layout/api/LayoutItem.h
index 101afa1..d7857e9 100644
--- a/third_party/WebKit/Source/core/layout/api/LayoutItem.h
+++ b/third_party/WebKit/Source/core/layout/api/LayoutItem.h
@@ -144,9 +144,14 @@ public:
         return m_layoutObject->styleRef();
     }
 
-    LayoutSize offsetFromContainer(const LayoutItem& item, const LayoutPoint& point, bool* offsetDependsOnPoint = nullptr) const
+    LayoutSize offsetFromContainer(const LayoutItem& item) const
     {
-        return m_layoutObject->offsetFromContainer(item.layoutObject(), point, offsetDependsOnPoint);
+        return m_layoutObject->offsetFromContainer(item.layoutObject());
+    }
+
+    LayoutSize columnOffset(const LayoutPoint& point) const
+    {
+        return m_layoutObject->columnOffset(point);
     }
 
     FrameView* frameView() const