Don't mark a positioned object for layout when we've just laid it out.

Don't mark a positioned object for layout when we've just laid it out.

When we mark positioned objects for layout while laying out lines or block flows we
are assuming that any object we mark will get laid out later in the same layout.
This is fine when we're walking a tree but the positioned objects list is not a tree
and it's possible for elements in it to be out of DOM order. So if we layout the
descendants of positioned object A and mark a positioned object while doing so we
cannot assume that it will get laid out later; it may have already had its layout
in this round.

So to remove this assumption start bringing all the marking of positioned objects for layout
back into layoutPositionedObjects. This CL starts by bringing in the objects from
line layout.

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

[rhogan, 2015-05-12 18:41:11]

robhogan@gmail.com changed reviewers:
+ leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-05-18 22:30:10]

Sorry for the delay. I've been traveling/attending BlinkOn. I'll get to this
tomorrow when I can take the time to fully grok it. Thanks for your patience,
sir!

[leviw_travelin_and_unemployed, 2015-05-19 18:32:13]

A couple nits, but great job on this tricky stuff! LGTM with nits addressed.

https://codereview.chromium.org/1128713004/diff/1/LayoutTests/fast/block/posi...
File
LayoutTests/fast/block/positioning/setting-layout-on-posobjs-while-laying-them-out.html
(right):

https://codereview.chromium.org/1128713004/diff/1/LayoutTests/fast/block/posi...
LayoutTests/fast/block/positioning/setting-layout-on-posobjs-while-laying-them-out.html:1:
<style>
Nit: doctype :p

https://codereview.chromium.org/1128713004/diff/1/LayoutTests/fast/block/posi...
LayoutTests/fast/block/positioning/setting-layout-on-posobjs-while-laying-them-out.html:4:
}
Nit: Un-indent this brace

https://codereview.chromium.org/1128713004/diff/1/Source/core/layout/LayoutBl...
File Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1128713004/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlock.cpp:1690: // *after* we've laid it out in
layoutPositionedObjects.
We should assert this. Can you file a bug and link it here?

[rhogan, 2015-05-20 20:15:06]

https://codereview.chromium.org/1128713004/diff/1/Source/core/layout/LayoutBl...
File Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1128713004/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlock.cpp:1690: // *after* we've laid it out in
layoutPositionedObjects.
On 2015/05/19 at 18:32:12, leviw wrote:
> We should assert this. Can you file a bug and link it here?

Added the bug, but couldn't think of a way of asserting this without writing an
elaborate isBeforeInDom function. There doesn't seem to be one already.

[rhogan, 2015-05-20 20:15:15]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2015-05-20 20:15:15]

The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org
Link to the patchset: https://codereview.chromium.org/1128713004/#ps20001
(title: "Updated")

[commit-bot: I haz the power, 2015-05-20 20:15:58]

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

[commit-bot: I haz the power, 2015-05-20 20:56:23]

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

[commit-bot: I haz the power, 2015-05-20 20:56:24]

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

[leviw_travelin_and_unemployed, 2015-05-20 21:19:38]

On 2015/05/20 at 20:15:06, robhogan wrote:
>
https://codereview.chromium.org/1128713004/diff/1/Source/core/layout/LayoutBl...
> File Source/core/layout/LayoutBlock.cpp (right):
> 
>
https://codereview.chromium.org/1128713004/diff/1/Source/core/layout/LayoutBl...
> Source/core/layout/LayoutBlock.cpp:1690: // *after* we've laid it out in
layoutPositionedObjects.
> On 2015/05/19 at 18:32:12, leviw wrote:
> > We should assert this. Can you file a bug and link it here?
> 
> Added the bug, but couldn't think of a way of asserting this without writing
an elaborate isBeforeInDom function. There doesn't seem to be one already.

It shouldn't need to be elaborate, right? You should just be able to walk
backwards in pre-order, staying inside the block, no?

Anyways, LGTM.

[rhogan, 2015-05-22 18:43:51]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2015-05-22 18:43:52]

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

[commit-bot: I haz the power, 2015-05-22 18:44:52]

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

[rhogan, 2015-05-22 20:19:31]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2015-05-22 20:19:31]

The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org
Link to the patchset: https://codereview.chromium.org/1128713004/#ps60001
(title: "Updated")

[commit-bot: I haz the power, 2015-05-22 20:19:44]

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

[commit-bot: I haz the power, 2015-05-22 21:57:19]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195813