[SPInvaldation] Update subtree paint properties when a layout subtree is inserted

[SPInvaldation] Update subtree paint properties when a layout subtree is inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

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

Committed: https://crrev.com/f70b47902bd95e111862265afa513926d777fd20
Cr-Commit-Position: refs/heads/master@{#439193}

[Xianzhu, 2016-12-16 00:54:11]

Description was changed from

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

BTW, removed FrameView::shouldInvalidateAllPaintAndPaintProperties() and
use the new flag on LayoutView instead.

BUG=645667
==========

to

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

BTW, removed FrameView::shouldInvalidateAllPaintAndPaintProperties() and
use the new flag on LayoutView instead.

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

[Xianzhu, 2016-12-16 01:00:03]

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

[Xianzhu, 2016-12-16 01:00:04]

Haven't found another way to deal with this. Some descendant of a moved subtree
may change without any paint property change of an ancestor in the subtree, e.g.
the descendant can get a containing block or stacking context above the root of
the subtree.

https://codereview.chromium.org/2585603002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameView.h (left):

https://codereview.chromium.org/2585603002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameView.h:763: 
Some changes in this CL are actually revert of
https://codereview.chromium.org/2575653002/. Now use the new flag for the
functionality (invalidating all paint properties before/after printing).

[Xianzhu, 2016-12-16 01:38:11]

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

[commit-bot: I haz the power, 2016-12-16 01:38:58]

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

[Xianzhu, 2016-12-16 01:42:05]

Description was changed from

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

BTW, removed FrameView::shouldInvalidateAllPaintAndPaintProperties() and
use the new flag on LayoutView instead.

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

to

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

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

[pdr., 2016-12-16 02:17:28]

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

https://codereview.chromium.org/2585603002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObjectChildList.cpp:187:
newChild->setSubtreeNeedsPaintPropertyUpdate();
WDYT of making this more explicit by using a flag (or by putting the reason in
the function name instead of the update type)? I think this will help make it
clearer to folks outside the paint team which call is appropriate.

I'm thinking something like:
enum PaintPropertyUpdate {
  StyleOrLayoutChange
  InsertedIntoLayoutTree
}
newChild->setNeedsPaintPropertyUpdate(InsertedIntoLayoutTree);

We could then hide the details in LayoutObject::setNeedsPaintPropertyUpdate.

[commit-bot: I haz the power, 2016-12-16 03:40:40]

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

[commit-bot: I haz the power, 2016-12-16 03:40:41]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Xianzhu, 2016-12-16 17:24:57]

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

https://codereview.chromium.org/2585603002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObjectChildList.cpp:187:
newChild->setSubtreeNeedsPaintPropertyUpdate();
On 2016/12/16 02:17:28, pdr. wrote:
> WDYT of making this more explicit by using a flag (or by putting the reason in
> the function name instead of the update type)? I think this will help make it
> clearer to folks outside the paint team which call is appropriate.
> 
> I'm thinking something like:
> enum PaintPropertyUpdate {
>   StyleOrLayoutChange
>   InsertedIntoLayoutTree
> }
> newChild->setNeedsPaintPropertyUpdate(InsertedIntoLayoutTree);
> 
> We could then hide the details in LayoutObject::setNeedsPaintPropertyUpdate.

I would still prefer subtree update flag because
- I'm planning to replace
FrameView::setShouldInvalidateAllPaintAndPaintProperties with this new flag
(https://codereview.chromium.org/2583733003/). Using the enum we would need
another value;
- The semantics of subtree update for callers in the two cases (insert child,
printing) seem all clear;
- I feel we might have other cases needing explicit subtree update semantics.

[chrishtr, 2016-12-16 18:40:38]

Please get pdr's LGTM also.

I think this approach is ok for the reasons stated in the latest comment.

[chrishtr, 2016-12-16 18:40:40]

lgtm

[pdr., 2016-12-16 21:14:56]

On 2016/12/16 at 18:40:40, chrishtr wrote:
> lgtm

lgtm

[Xianzhu, 2016-12-16 21:22:56]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-12-16 21:24:19]

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-12-16 21:39:05]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481923376939480,
"parent_rev": "0f0da2909ed681949f575f6dfd4b8f710509e606", "commit_rev":
"7d6dedad1ed835bacbe0c04d9ca9c3de84e0130b"}

[commit-bot: I haz the power, 2016-12-16 21:39:44]

Description was changed from

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

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

to

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

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

Review-Url: https://codereview.chromium.org/2585603002
==========

[commit-bot: I haz the power, 2016-12-16 21:39:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-16 21:42:11]

Description was changed from

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

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

Review-Url: https://codereview.chromium.org/2585603002
==========

to

==========
[SPInvaldation] Update subtree paint properties when a layout subtree is
inserted

Add LayoutObject::subtreeNeedsPaintPropertyUpdate() and set it in
LayoutObjectChildList::addChild() to force paint property update of the
whole subtree.

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

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

[commit-bot: I haz the power, 2016-12-16 21:42:12]

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