Make column snapping optional when translating to flow thread coordinates.

third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h

Index: third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h
diff --git a/third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h b/third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h
index bb625deba6b8a4cb6937f3cabc59d3e5c4f364ee..5fdf64021d83428f969a5639e69a8bd2e3138ecf 100644
--- a/third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h
+++ b/third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h
@@ -79,8 +79,21 @@ class MultiColumnFragmentainerGroup {
                                            LayoutBox::PageBoundaryRule,
                                            CoordinateSpaceConversion) const;
   LayoutUnit columnLogicalTopForOffset(LayoutUnit offsetInFlowThread) const;
+
+  // If SnapToColumnPolicy is SnapToColumn, visualPointToFlowThreadPoint() won't
+  // return points that lie outside the bounds of the columns: Before converting
+  // to a flow thread position, if the block direction coordinate is outside the
+  // column, snap to the bounds of the column, and reset the inline direction
+  // coordinate to the start position in the column. The effect of this is that
+  // if the block position is before the column rectangle, we'll get to the
+  // beginning of this column, while if the block position is after the column
+  // rectangle, we'll get to the beginning of the next column. This is behavior
+  // that positionForPoint() depends on.
+  enum SnapToColumnPolicy { DontSnapToColumn, SnapToColumn };

[szager1, 2016/12/19 23:36:44]

nit: I prefer not to define an enum for what is effectively a boolean flag. 
Better, I think, to just use a boolean parameter with a self-documenting name. 
I don't feel strongly, so you can ignore this.

[mstensho (USE GERRIT), 2016/12/20 08:55:02]

A long time ago I learned that this was how to do things in Blink/WebKit.
Basically, we should never use bools in parameters, unless it's super-obvious
what it means (a bool setter, for instance). I've come to like this rule,
because it *is* hard to tell what it does when looking at the code from the call
site.

The requirement is still there (if the document is still valid). See item 10
here:
https://www.chromium.org/blink/coding-style#TOC-Names

   LayoutPoint visualPointToFlowThreadPoint(
-      const LayoutPoint& visualPoint) const;
+      const LayoutPoint& visualPoint,
+      SnapToColumnPolicy = DontSnapToColumn) const;

[szager1, 2016/12/19 23:36:44]

It's a bit strange that the default value doesn't preserve the previous
behavior.  Is this what you wanted?

[mstensho (USE GERRIT), 2016/12/20 08:55:02]

This is exactly what I wanted. The previous behavior is weird and non-obvious. I
wanted the default behavior to be straight-forward, boring and without
surprises.

+
   LayoutRect fragmentsBoundingBox(
       const LayoutRect& boundingBoxInFlowThread) const;