Move computation of 3D descendants to the descendant-dependent flags walk.

Move computation of 3D descendants to the descendant-dependent flags walk.

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

Committed: https://crrev.com/8624b809c7f75c609e509f17cc4ee20fc3fceb90
Cr-Commit-Position: refs/heads/master@{#440909}

[chrishtr, 2016-12-28 02:13:14]

Description was changed from

==========
none


none

BUG=
==========

to

==========
none


none

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

[chrishtr, 2016-12-28 02:14:10]

Description was changed from

==========
none


none

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

to

==========
Move computation of 3D descendants to the descendant-dependent flags walk.

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

[chrishtr, 2016-12-28 02:14:45]

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

[commit-bot: I haz the power, 2016-12-28 02:14:53]

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, 2016-12-28 03:28:52]

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

[commit-bot: I haz the power, 2016-12-28 03:28:53]

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

[chrishtr, 2016-12-28 17:30:19]

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

[commit-bot: I haz the power, 2016-12-28 17:30:25]

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

[chrishtr, 2016-12-28 17:50:37]

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

[commit-bot: I haz the power, 2016-12-28 17:50:47]

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

[chrishtr, 2016-12-28 17:52:02]

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

[Xianzhu, 2016-12-28 18:08:11]

https://codereview.chromium.org/2604973002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2604973002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:766:
m_has3DTransformedDescendant = m_has3DTransformedDescendant || childHas3D;
We should break the loop when m_has3DTransformedDescendant is set to true the
first time.

Also we seem to iterate the children twice: once in
updateDescendantDependentFlags(), once here. Can we integrate this function into
the loop in updateDescendantDependentFlags()?

https://codereview.chromium.org/2604973002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp (right):

https://codereview.chromium.org/2604973002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:322: if
(object.isBoxModelObject() &&
Not related to the change, but just a question: we support non-box object here,
but don't  in updateTransform() (which doesn't create transform node if
!object.isBox()). Is there any problem?

[chrishtr, 2016-12-28 18:49:47]

https://codereview.chromium.org/2604973002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2604973002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:766:
m_has3DTransformedDescendant = m_has3DTransformedDescendant || childHas3D;
On 2016/12/28 at 18:08:11, Xianzhu wrote:
> We should break the loop when m_has3DTransformedDescendant is set to true the
first time.

Fixed.

> 
> Also we seem to iterate the children twice: once in
updateDescendantDependentFlags(), once here. Can we integrate this function into
the loop in updateDescendantDependentFlags()?

This one loops over the stacking children, which may not even be direct
children.
Alternatively, I considered getting rid of the stacking children concept and
just look at children, but that would end up promoting more PaintLayers than
necessary.

[chrishtr, 2016-12-28 18:49:58]

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

[commit-bot: I haz the power, 2016-12-28 18:50:08]

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-28 18:56:33]

lgtm

https://codereview.chromium.org/2604973002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2604973002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:766:
m_has3DTransformedDescendant = m_has3DTransformedDescendant || childHas3D;
Nit: Would the following look better:

  if (childHas3D) {
    m_has3DTransformedDescendant = true;
    break;
  }

[chrishtr, 2016-12-28 19:01:00]

https://codereview.chromium.org/2604973002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2604973002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:766:
m_has3DTransformedDescendant = m_has3DTransformedDescendant || childHas3D;
On 2016/12/28 at 18:56:33, Xianzhu wrote:
> Nit: Would the following look better:
> 
>   if (childHas3D) {
>     m_has3DTransformedDescendant = true;
>     break;
>   }

Done.

[chrishtr, 2016-12-28 19:01:07]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-12-28 19:01:08]

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/2604973002/#ps80001
(title: "none")

[commit-bot: I haz the power, 2016-12-28 19:01:17]

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

[chrishtr, 2016-12-28 19:21:42]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2016-12-28 19:26:12]

Slightly updated code for property tree building to specify a more exact
compositing reason.

[chrishtr, 2016-12-28 19:26:21]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-12-28 19:26:21]

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/2604973002/#ps100001
(title: "none")

[commit-bot: I haz the power, 2016-12-28 19:26:35]

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-28 21:09:55]

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

[commit-bot: I haz the power, 2016-12-28 21:09:56]

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

[chrishtr, 2016-12-28 22:52:44]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-12-28 22:52:45]

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/2604973002/#ps120001
(title: "none")

[commit-bot: I haz the power, 2016-12-28 22:52:51]

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-29 00:10:34]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1482965564877800,
"parent_rev": "c421ae7020ed0e27dfcf2707942ada303c0da633", "commit_rev":
"c3db2bb4e127b0a7e3d58c1ac38d0f1e73dda118"}

[commit-bot: I haz the power, 2016-12-29 00:11:05]

Description was changed from

==========
Move computation of 3D descendants to the descendant-dependent flags walk.

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

to

==========
Move computation of 3D descendants to the descendant-dependent flags walk.

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

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

[commit-bot: I haz the power, 2016-12-29 00:11:06]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2017-01-02 15:50:52]

Description was changed from

==========
Move computation of 3D descendants to the descendant-dependent flags walk.

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

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

to

==========
Move computation of 3D descendants to the descendant-dependent flags walk.

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

Committed: https://crrev.com/8624b809c7f75c609e509f17cc4ee20fc3fceb90
Cr-Commit-Position: refs/heads/master@{#440909}
==========

[commit-bot: I haz the power, 2017-01-02 15:50:54]

Patchset 7 (id:??) landed as
https://crrev.com/8624b809c7f75c609e509f17cc4ee20fc3fceb90
Cr-Commit-Position: refs/heads/master@{#440909}