May need to insert or remove column sets when out-of-flow objects get their containing block change…

May need to insert or remove column sets when out-of-flow objects get their containing block changed.

Out-of-flow descendants may become or cease to be a part of a multicol
container due to changes on an ancestor. An ancestor may become or cease to be
a containing block for out-of-flow descendants.

Transforms also establish containing blocks for absolutely and fixed positioned
descendants, so we need to check for that too.

BUG=509463
R=dsinclair@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198940

[mstensho (USE GERRIT), 2015-07-14 07:38:50]

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

[dsinclair, 2015-07-14 13:47:44]

dsinclair@chromium.org changed reviewers:
+ leviw@chromium.org

[dsinclair, 2015-07-14 13:47:44]

lgtm

[mstensho (USE GERRIT), 2015-07-14 15:10:50]

Levi: does this compute?

[leviw_travelin_and_unemployed, 2015-07-14 19:43:42]

Code change makes sense to me. One nit about the comment.

https://codereview.chromium.org/1229983004/diff/20001/Source/core/layout/Layo...
File Source/core/layout/LayoutMultiColumnFlowThread.cpp (right):

https://codereview.chromium.org/1229983004/diff/20001/Source/core/layout/Layo...
Source/core/layout/LayoutMultiColumnFlowThread.cpp:759: // If we've become (or
are about to become) a container for absolutely positioned descendants,
This comment seems to imply we only care if we're about to become a container
for abs pos descendants, but the code checks if we become or stop being a
container for abs pos.

[mstensho (USE GERRIT), 2015-07-14 19:49:25]

https://codereview.chromium.org/1229983004/diff/20001/Source/core/layout/Layo...
File Source/core/layout/LayoutMultiColumnFlowThread.cpp (right):

https://codereview.chromium.org/1229983004/diff/20001/Source/core/layout/Layo...
Source/core/layout/LayoutMultiColumnFlowThread.cpp:759: // If we've become (or
are about to become) a container for absolutely positioned descendants,
On 2015/07/14 19:43:42, leviw wrote:
> This comment seems to imply we only care if we're about to become a container
> for abs pos descendants, but the code checks if we become or stop being a
> container for abs pos.

Done.

The code was right, the comment was inaccurate.

[leviw_travelin_and_unemployed, 2015-07-14 20:08:23]

On 2015/07/14 at 19:49:25, mstensho wrote:
>
https://codereview.chromium.org/1229983004/diff/20001/Source/core/layout/Layo...
> File Source/core/layout/LayoutMultiColumnFlowThread.cpp (right):
> 
>
https://codereview.chromium.org/1229983004/diff/20001/Source/core/layout/Layo...
> Source/core/layout/LayoutMultiColumnFlowThread.cpp:759: // If we've become (or
are about to become) a container for absolutely positioned descendants,
> On 2015/07/14 19:43:42, leviw wrote:
> > This comment seems to imply we only care if we're about to become a
container
> > for abs pos descendants, but the code checks if we become or stop being a
> > container for abs pos.
> 
> Done.
> 
> The code was right, the comment was inaccurate.

That was my presumption :)

LGTM!

[mstensho (USE GERRIT), 2015-07-14 20:11:32]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2015-07-14 20:11:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/1229983004/#ps40001
(title: "code review")

[commit-bot: I haz the power, 2015-07-14 20:11:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1229983004/40001

[commit-bot: I haz the power, 2015-07-15 00:50:16]

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

[commit-bot: I haz the power, 2015-07-15 00:50:17]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/70452)

[mstensho (USE GERRIT), 2015-07-15 06:59:21]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2015-07-15 06:59:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1229983004/40001

[commit-bot: I haz the power, 2015-07-15 08:33:43]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198940