Don't traverse non-stacked layer under composited child in traverseNonCompositingDescendants

Don't traverse non-stacked layer under composited child in traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
TBR=nednguyen (for enabling the system health smoke test)

Committed: https://crrev.com/6f716f1637685a30fd65cdec3a6b130690d56dbf
Cr-Commit-Position: refs/heads/master@{#410070}

[Xianzhu, 2016-08-04 22:40:09]

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

[commit-bot: I haz the power, 2016-08-04 22:40:34]

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

[Xianzhu, 2016-08-04 22:43:07]

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

[commit-bot: I haz the power, 2016-08-04 22:43:11]

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

[Xianzhu, 2016-08-04 22:43:28]

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

[chrishtr, 2016-08-04 23:10:30]

lgtm

[Xianzhu, 2016-08-04 23:31:07]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-08-04 23:31:12]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-08-04 23:31:38]

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

[trchen, 2016-08-04 23:34:55]

Also, it is not your fault (actually my fault), the four cases are meant to
never fall through or overlap, so we should change them to "else if".

https://codereview.chromium.org/2214253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2214253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3505: if
(!descendant->hasLayer()) {
How about this:

// Case 1: If the descendant is not stacked, keep searching until a stacked one
is found.
if (!descendant->styleRef().isStacked()) {

https://codereview.chromium.org/2214253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3510: if
(!descendant->isPaintInvalidationContainer()) {
// Case 2: The descendant is stacked but not composited.
if (!descendant->isPaintInvalidationContainer()) {

[commit-bot: I haz the power, 2016-08-04 23:38:41]

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

[commit-bot: I haz the power, 2016-08-04 23:38:43]

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

[Xianzhu, 2016-08-05 00:27:07]

Also changed the if () { ... continue; } statements to if () { } else if ...
which looks clearer.

https://codereview.chromium.org/2214253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2214253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3505: if
(!descendant->hasLayer()) {
On 2016/08/04 23:34:55, trchen wrote:
> How about this:
> 
> // Case 1: If the descendant is not stacked, keep searching until a stacked
one
> is found.
> if (!descendant->styleRef().isStacked()) {

Done.

https://codereview.chromium.org/2214253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3510: if
(!descendant->isPaintInvalidationContainer()) {
On 2016/08/04 23:34:55, trchen wrote:
> // Case 2: The descendant is stacked but not composited.
> if (!descendant->isPaintInvalidationContainer()) {

Done.

[Xianzhu, 2016-08-05 00:29:39]

Description was changed from

==========
Don't traverse non-stacked layer under composited child in
traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
==========

to

==========
Don't traverse non-stacked layer under composited child in
traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
TBR=nednguyen (for enabling the system health smoke test)
==========

[trchen, 2016-08-05 00:34:10]

Thank you for fixing this! lgtm

[Xianzhu, 2016-08-05 00:34:31]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-08-05 00:34:31]

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

[commit-bot: I haz the power, 2016-08-05 00:34:50]

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

[Xianzhu, 2016-08-05 00:39:20]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-08-05 00:46:45]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-08-05 00:46:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, trchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2214253002/#ps60001
(title: "Update comments to be more accurate")

[commit-bot: I haz the power, 2016-08-05 00:47:05]

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-05 01:02:38]

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

[commit-bot: I haz the power, 2016-08-05 01:02:39]

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

[Xianzhu, 2016-08-05 03:34:19]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-08-05 03:34:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, trchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2214253002/#ps80001
(title: "!hasLayer || !isStacked")

[commit-bot: I haz the power, 2016-08-05 03:34:29]

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-05 05:04:16]

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

[commit-bot: I haz the power, 2016-08-05 05:04:18]

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

[Xianzhu, 2016-08-05 15:29:10]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-08-05 15:29:22]

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-05 15:59:56]

Description was changed from

==========
Don't traverse non-stacked layer under composited child in
traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
TBR=nednguyen (for enabling the system health smoke test)
==========

to

==========
Don't traverse non-stacked layer under composited child in
traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
TBR=nednguyen (for enabling the system health smoke test)
==========

[commit-bot: I haz the power, 2016-08-05 15:59:57]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-05 16:01:18]

Description was changed from

==========
Don't traverse non-stacked layer under composited child in
traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
TBR=nednguyen (for enabling the system health smoke test)
==========

to

==========
Don't traverse non-stacked layer under composited child in
traverseNonCompositingDescendants

Non-stacked layers under a composited object belong to the object, so
we should not traverse them in traverseNonCompositingDescendants.

BUG=634389
TEST=LayoutObjectTest.TraverseNonCompositingDescendantsInPaintOrder
TBR=nednguyen (for enabling the system health smoke test)

Committed: https://crrev.com/6f716f1637685a30fd65cdec3a6b130690d56dbf
Cr-Commit-Position: refs/heads/master@{#410070}
==========

[commit-bot: I haz the power, 2016-08-05 16:01:19]

Patchset 5 (id:??) landed as
https://crrev.com/6f716f1637685a30fd65cdec3a6b130690d56dbf
Cr-Commit-Position: refs/heads/master@{#410070}

[anthonyvd, 2016-08-05 16:12:33]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2219903002/ by anthonyvd@chromium.org.

The reason for reverting is: Reverting this CL because it appears to be breaking
the following bot:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder%....

[findit-for-me, 2016-08-05 16:20:31]

FYI: Findit try jobs (rerunning failed compile or tests) identified this CL
at revision 410070 as the culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...

[iclelland, 2016-08-05 17:10:26]

iclelland@chromium.org changed reviewers:
+ iclelland@chromium.org

[iclelland, 2016-08-05 17:10:27]

https://codereview.chromium.org/2214253002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp (right):

https://codereview.chromium.org/2214253002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:153:
RefPtr<JSONArray> invalidations =
document().view()->trackedObjectPaintInvalidationsAsJSON();
Drive-by note, because of the revert. This needs to be
std::unique_ptr<JSONArray> now (as of https://crrev.com/cc67aa8)