Handle auto-positioned out-of-flow objects inside multicol containers correctly.

Handle auto-positioned out-of-flow objects inside multicol containers correctly.

We used to get it all wrong for out-of-flow children of multicol containers
whose containing block were on the outside of the multicol container. Those
do not live in the flow thread coordinate space, so we have to convert their
location to visual coordinates.

BUG=291616
Committed: https://crrev.com/57b66b0901dfcd97ed669a3635c55fbb6cc01fc9
Cr-Commit-Position: refs/heads/master@{#412375}

[mstensho (USE GERRIT), 2016-08-16 16:42:06]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 16:42:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 18:17:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-16 18:17:23]

Dry run: This issue passed the CQ dry run.

[mstensho (USE GERRIT), 2016-08-16 18:19:12]

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

[eae, 2016-08-16 18:23:45]

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:3604: LayoutBox& box =
*toLayoutBox(curr);
const?

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp (right):

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp:195: position =
flipForWritingMode(position);
Instead of:

position = flipForWritingMode(position);
position.move(columnOffset(position));
position = flipForWritingMode(position);

why not do
position.move(columnOffset(flipForWritingMode(position)));
? or would that not have the same effect? The repeated updates of position makes
this code pretty hard to follow.

[mstensho (USE GERRIT), 2016-08-16 18:55:27]

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:3604: LayoutBox& box =
*toLayoutBox(curr);
On 2016/08/16 18:23:45, eae wrote:
> const? 

Done.

flowThreadToVisualLogicalPosition() didn't like that, though, due to lazy me
forgetting to const it in the first round.

I also renamed flowThreadToVisualLogicalPosition() to
flowThreadToContainingCoordinateSpace(), since they will not become visual
coordinates if the flowthread is nested inside another multicol container (see
the CoordinateSpaceConversion::Containing enum value for more details). I meant
to do this *before* filing the CL, but I obviously forgot it. Sorry about that.

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp (right):

https://codereview.chromium.org/2250713002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp:195: position =
flipForWritingMode(position);
On 2016/08/16 18:23:45, eae wrote:
> Instead of:
> 
> position = flipForWritingMode(position);
> position.move(columnOffset(position));
> position = flipForWritingMode(position);
> 
> why not do
> position.move(columnOffset(flipForWritingMode(position)));
> ? or would that not have the same effect? The repeated updates of position
makes
> this code pretty hard to follow.

columnOffset() works on physical coordinates, while
flowThreadToVisualLogicalPosition() and its callers are purely logical. So we
have to flip back and forth. I'll add some comments instead. See if that works
better.

[eae, 2016-08-16 19:26:00]

Thanks, that's a bit clearer!

LGTM

[mstensho (USE GERRIT), 2016-08-16 19:52:22]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2016-08-16 19:52:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 19:59:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-16 19:59:21]

Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mstensho (USE GERRIT), 2016-08-16 20:45:51]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2016-08-16 20:46:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 23:35:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-16 23:37:31]

Description was changed from

==========
Handle auto-positioned out-of-flow objects inside multicol containers correctly.

We used to get it all wrong for out-of-flow children of multicol containers
whose containing block were on the outside of the multicol container. Those
do not live in the flow thread coordinate space, so we have to convert their
location to visual coordinates.

BUG=291616
==========

to

==========
Handle auto-positioned out-of-flow objects inside multicol containers correctly.

We used to get it all wrong for out-of-flow children of multicol containers
whose containing block were on the outside of the multicol container. Those
do not live in the flow thread coordinate space, so we have to convert their
location to visual coordinates.

BUG=291616

Committed: https://crrev.com/57b66b0901dfcd97ed669a3635c55fbb6cc01fc9
Cr-Commit-Position: refs/heads/master@{#412375}
==========

[commit-bot: I haz the power, 2016-08-16 23:37:32]

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