Remove multicol special-cases from invalidation and the pre-paint tree walk

Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow the layout tree in DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM ancestors.

With this approach, the pre-paint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
for multicolumn during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2615203002
Cr-Commit-Position: refs/heads/master@{#442149}
Committed: https://chromium.googlesource.com/chromium/src/+/78357e64419e215213610a27cd89a94da49e6bbe

[pdr., 2017-01-06 04:40:55]

Description was changed from

==========
Remove multicolumn special-case from the pre-paint tree walk traversal

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.
==========

to

==========
Remove multicolumn special-case from the pre-paint tree walk traversal

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2017-01-06 04:41:26]

pdr@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[pdr., 2017-01-06 04:51:24]

(not ready for a real review)

This new approach simplifies the treewalk, but it has a bug because the prepaint
walk is layout-tree order but paint invalidation follows the paint-order of
spanners, jumping from the spanner to its placeholder. This can lead to skipping
invalidation, for example:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

If we mark the spanner as needing paint invalidation, we'll get the following
tree:
LayoutBlockFlow DIV style="columns: 2" needs_paint_invalidation
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner" needs_paint_invalidation
  LayoutMultiColumnSpannerPlaceholder (anonymous) needs_paint_invalidation

During paint invalidation, we will then early-out at the
LayoutMultiColumnFlowThread instead of continuing to the spanner. For this to
work, I think we will need to change paint invalidation to follow layout tree
order too. This may make fixing https://crbug.com/648274 and
https://crbug.com/672989 easier. Do you think this is a reasonable approach?

[Xianzhu, 2017-01-06 16:58:23]

On 2017/01/06 04:51:24, pdr. wrote:
> (not ready for a real review)
> 
> This new approach simplifies the treewalk, but it has a bug because the
prepaint
> walk is layout-tree order but paint invalidation follows the paint-order of
> spanners, jumping from the spanner to its placeholder. This can lead to
skipping
> invalidation, for example:
> LayoutBlockFlow DIV style="columns: 2"
>   LayoutMultiColumnFlowThread (anonymous)
>     LayoutBlockFlow (column spanner) 	DIV id="spanner"
>   LayoutMultiColumnSpannerPlaceholder (anonymous)
> 
> If we mark the spanner as needing paint invalidation, we'll get the following
> tree:
> LayoutBlockFlow DIV style="columns: 2" needs_paint_invalidation
>   LayoutMultiColumnFlowThread (anonymous)
>     LayoutBlockFlow (column spanner) 	DIV id="spanner"
needs_paint_invalidation
>   LayoutMultiColumnSpannerPlaceholder (anonymous) needs_paint_invalidation
> 
> During paint invalidation, we will then early-out at the
> LayoutMultiColumnFlowThread instead of continuing to the spanner. For this to
> work, I think we will need to change paint invalidation to follow layout tree
> order too. This may make fixing https://crbug.com/648274 and
> https://crbug.com/672989 easier. Do you think this is a reasonable approach?

Yes, I think it is reasonable.

[pdr., 2017-01-06 23:28:13]

Description was changed from

==========
Remove multicolumn special-case from the pre-paint tree walk traversal

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation to follow layout-tree
order.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2017-01-06 23:29:01]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 23:29:32]

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, 2017-01-06 23:32:27]

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

[commit-bot: I haz the power, 2017-01-06 23:32:28]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[pdr., 2017-01-06 23:58:23]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 23:58:43]

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

[pdr., 2017-01-07 00:25:25]

Description was changed from

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation to follow layout-tree
order.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation to follow layout-tree
order.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2017-01-07 01:05:38]

Woohoo, ready for review

[pdr., 2017-01-07 01:06:27]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-07 01:07:11]

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

[Xianzhu, 2017-01-07 01:12:08]

lgtm with one comment.

This is great!

https://codereview.chromium.org/2615203002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):

https://codereview.chromium.org/2615203002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:204: // See
LayoutObject::paintingLayer() for the special-cases of out-of-flow
s/out-of-flow floating/floating under inline/.

[pdr., 2017-01-07 01:37:55]

Description was changed from

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree order. This patch
switches the pre-paint tree walk to be entirely layout-tree order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation to follow layout-tree
order.

With this approach, only the paint offset needs special adjustment
during the pre-paint tree walk.

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM order.

With this approach, the prepaint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

layout tree DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2017-01-07 01:39:50]

Description was changed from

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow layout-tree DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM order.

With this approach, the prepaint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

layout tree DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow the layout tree in DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM ancestors.

With this approach, the pre-paint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
for multicolumn during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

layout tree DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2017-01-07 01:41:38]

Description was changed from

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow the layout tree in DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM ancestors.

With this approach, the pre-paint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
for multicolumn during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

layout tree DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow the layout tree in DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM ancestors.

With this approach, the pre-paint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
for multicolumn during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2017-01-07 02:03:54]

The CQ bit was checked by pdr@chromium.org

[pdr., 2017-01-07 02:03:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2615203002/#ps100001
(title: "Cleanup comment")

[commit-bot: I haz the power, 2017-01-07 02:04:08]

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, 2017-01-07 03:18:53]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1483754634358750,
"parent_rev": "ce807707f40a88316d193941c31b196f15e138ba", "commit_rev":
"78357e64419e215213610a27cd89a94da49e6bbe"}

[commit-bot: I haz the power, 2017-01-07 03:19:21]

Description was changed from

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow the layout tree in DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM ancestors.

With this approach, the pre-paint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
for multicolumn during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove multicol special-cases from invalidation and the pre-paint tree walk

Multicolumn spanners paint out-of-flow through placeholder LayoutObjects
but their paint properties still follow the layout tree in DOM order[1].
This patch switches the pre-paint tree walk to be entirely DOM order
instead of having a special-case for the paint-order multicolumn
spanner. This patch also changes paint invalidation of multicolumn
to follow DOM ancestors.

With this approach, the pre-paint tree walk and paint invalidation
can be simplified, and only paint offset needs special adjustment
for multicolumn during the pre-paint tree walk.

[1] For example, the following layout tree:
LayoutBlockFlow DIV style="columns: 2"
  LayoutMultiColumnFlowThread (anonymous)
    LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

paint order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutMultiColumnSpannerPlaceholder (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"

DOM order:
  LayoutBlockFlow DIV
  LayoutMultiColumnFlowThread (anonymous)
  LayoutBlockFlow (column spanner) 	DIV id="spanner"
  LayoutMultiColumnSpannerPlaceholder (anonymous)

BUG=648274

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2615203002
Cr-Commit-Position: refs/heads/master@{#442149}
Committed:
https://chromium.googlesource.com/chromium/src/+/78357e64419e215213610a27cd89...
==========

[commit-bot: I haz the power, 2017-01-07 03:19:22]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/78357e64419e215213610a27cd89...