Translate flow thread coords to the nearest enclosing coord space when appropriate.

Translate flow thread coords to the nearest enclosing coord space when appropriate.

We used to always convert to the visual coordinate space, meaning that we
walked all enclosing fragmentation contexts. However, only the PaintLayer code
wants this behavior, while everyone else typically wants to do one
fragmentation context at a time, e.g. when walking the ancestry with
LayoutObject::mapLocalToAncestor().

For nested multicol, this caused invalidation bugs, problems with
getClientRects(), and maybe more.

Added an enum CoordinateSpaceConversion (with values "Containing" and "Visual")
for flowThreadTranslationAtOffset() to use to determine which conversion to
perform. The old behavior was to always do CoordinateSpaceConversion::Visual.

BUG=604883
Committed: https://crrev.com/e60405b884e8e5ac517a27e03a955a0f3a4f2c98
Cr-Commit-Position: refs/heads/master@{#388692}

[mstensho (USE GERRIT), 2016-04-20 16:44:29]

mstensho@opera.com changed reviewers:
+ leviw@chromium.org

[wkorman, 2016-04-20 19:58:34]

wkorman@chromium.org changed reviewers:
+ wangxianzhu@chromium.org, wkorman@chromium.org

[wkorman, 2016-04-20 19:58:35]

lgtm

+wangxianzhu as he may be able to look at this before leviw. Thanks again for
putting this together so quickly.

[Xianzhu, 2016-04-20 20:50:13]

https://codereview.chromium.org/1907443003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
(right):

https://codereview.chromium.org/1907443003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp:336:
LayoutUnit blockOffset = isHorizontalWritingMode() ? flippedPoint.y() :
flippedPoint.x();
This CL also changes how we handle writing-mode in several methods. Is this
covered by any test? What's the rule of coordinate spaces of the results from
these methods?

[mstensho (USE GERRIT), 2016-04-20 21:02:24]

https://codereview.chromium.org/1907443003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
(right):

https://codereview.chromium.org/1907443003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp:336:
LayoutUnit blockOffset = isHorizontalWritingMode() ? flippedPoint.y() :
flippedPoint.x();
On 2016/04/20 20:50:12, Xianzhu wrote:
> This CL also changes how we handle writing-mode in several methods. Is this
> covered by any test? What's the rule of coordinate spaces of the results from
> these methods?

Yes, we have tests for multicol + writing modes, but this CL shouldn't change
how we handle writing modes. This is code that used to sit in the columnOffset()
implementation. Just moved it here instead.

There was one caller of columnOffset() that wanted to translate to the visual
coordinate space, while all other callers just want the nearest containing
coordinate space. So now columnOffset() always does what all the other callers
wanted, while the one that used to call columnOffset() and wanted the other
behavior (that's flowThreadPointToVisualPoint() in LayoutFlowThread()) now calls
directly into flowThreadTranslationAtPoint() instead, so that it can specify
that it wants it converted to the visual coordinate space.

[Xianzhu, 2016-04-20 21:04:34]

lgtm

[mstensho (USE GERRIT), 2016-04-21 04:19:29]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2016-04-21 04:19:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1907443003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1907443003/20001

[commit-bot: I haz the power, 2016-04-21 04:24:46]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-22 19:31:30]

Description was changed from

==========
Translate flow thread coords to the nearest enclosing coord space when
appropriate.

We used to always convert to the visual coordinate space, meaning that we
walked all enclosing fragmentation contexts. However, only the PaintLayer code
wants this behavior, while everyone else typically wants to do one
fragmentation context at a time, e.g. when walking the ancestry with
LayoutObject::mapLocalToAncestor().

For nested multicol, this caused invalidation bugs, problems with
getClientRects(), and maybe more.

Added an enum CoordinateSpaceConversion (with values "Containing" and "Visual")
for flowThreadTranslationAtOffset() to use to determine which conversion to
perform. The old behavior was to always do CoordinateSpaceConversion::Visual.

BUG=604883
==========

to

==========
Translate flow thread coords to the nearest enclosing coord space when
appropriate.

We used to always convert to the visual coordinate space, meaning that we
walked all enclosing fragmentation contexts. However, only the PaintLayer code
wants this behavior, while everyone else typically wants to do one
fragmentation context at a time, e.g. when walking the ancestry with
LayoutObject::mapLocalToAncestor().

For nested multicol, this caused invalidation bugs, problems with
getClientRects(), and maybe more.

Added an enum CoordinateSpaceConversion (with values "Containing" and "Visual")
for flowThreadTranslationAtOffset() to use to determine which conversion to
perform. The old behavior was to always do CoordinateSpaceConversion::Visual.

BUG=604883

Committed: https://crrev.com/e60405b884e8e5ac517a27e03a955a0f3a4f2c98
Cr-Commit-Position: refs/heads/master@{#388692}
==========

[commit-bot: I haz the power, 2016-04-22 19:31:32]

Patchset 2 (id:??) landed as
https://crrev.com/e60405b884e8e5ac517a27e03a955a0f3a4f2c98
Cr-Commit-Position: refs/heads/master@{#388692}