cc : Compute visible rects dynamically (2)

cc : Compute visible rects dynamically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[jaydasika, 2016-09-20 00:20:54]

Description was changed from

==========
cc : Compute visible rects dyanmically (2)

This CL computes visible rects dynamically for all cases.

BUG=642054
==========

to

==========
cc : Compute visible rects dyanmically (2)

This CL computes visible rects dynamically for all cases.

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[jaydasika, 2016-09-20 00:43:50]

Description was changed from

==========
cc : Compute visible rects dyanmically (2)

This CL computes visible rects dynamically for all cases.

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc : Compute visible rects dyanmically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[jaydasika, 2016-09-20 00:44:24]

Description was changed from

==========
cc : Compute visible rects dyanmically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc : Compute visible rects dynamically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[jaydasika, 2016-09-20 00:51:50]

jaydasika@chromium.org changed reviewers:
+ ajuma@chromium.org, weiliangc@chromium.org

[jaydasika, 2016-09-20 00:51:51]

PTAL

Patch 2 will be easier to review than patch 3, the diff between them is just a
rename. I want to rename ExecuteCDPWithPropertyTrees because its confusing (we
should always use property trees). What does function does differently from the
other ExecuteCDP is that, it locally saves the update_layer_list that CDP
computes.

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:248: property_trees);
We were not checking clip tree calculations when this method was called. Added
that. Adding VerifyTransformTreeCalculations here leads to a test failure (has
something to do with singular transforms), will sort it out in a separate CL.

[jaydasika, 2016-09-20 17:25:04]

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

[commit-bot: I haz the power, 2016-09-20 17:25:28]

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-09-20 18:14:39]

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

[commit-bot: I haz the power, 2016-09-20 18:14:39]

Dry run: This issue passed the CQ dry run.

[weiliangc, 2016-09-20 23:54:13]

Overall looks good.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1214: int effect_ancestor_with_copy_request =
Actually, just create a bool non_root_copy_request =
effect_ancestor_with_copy_request > kContentsRootNodeId. and use the bool
instead of checking more than once below.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1219: if (effect_ancestor_with_copy_request > 1)
{
Same as below, don't use 1.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1234: effect_ancestor_with_copy_request > 1
Use kContentsRootNodeId instead of 1.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1254: for (auto* layer : layer_list) {
Just auto should be fine.

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:213: void
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList(
For update_layer_list_, there is currently two getters? We should probably
delete one of them. (also maybe in another CL)

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:1742:
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList(root);
Hmm why do we have both this and the ExecuteCalculateDrawProperties function?
Can we unite the two? (possibly in another CL)

[jaydasika, 2016-09-21 01:33:52]

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1214: int effect_ancestor_with_copy_request =
On 2016/09/20 23:54:12, weiliangc wrote:
> Actually, just create a bool non_root_copy_request =
> effect_ancestor_with_copy_request > kContentsRootNodeId. and use the bool
> instead of checking more than once below.

Done.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1219: if (effect_ancestor_with_copy_request > 1)
{
On 2016/09/20 23:54:12, weiliangc wrote:
> Same as below, don't use 1.

Done.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1234: effect_ancestor_with_copy_request > 1
On 2016/09/20 23:54:12, weiliangc wrote:
> Use kContentsRootNodeId instead of 1.

Done.

https://codereview.chromium.org/2354753003/diff/20001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:1254: for (auto* layer : layer_list) {
On 2016/09/20 23:54:12, weiliangc wrote:
> Just auto should be fine.

Done.

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:213: void
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList(
On 2016/09/20 23:54:13, weiliangc wrote:
> For update_layer_list_, there is currently two getters? We should probably
> delete one of them. (also maybe in another CL)

Acknowledged.

https://codereview.chromium.org/2354753003/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:1742:
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList(root);
On 2016/09/20 23:54:13, weiliangc wrote:
> Hmm why do we have both this and the ExecuteCalculateDrawProperties function?
> Can we unite the two? (possibly in another CL)

Acknowledged.

[jaydasika, 2016-09-21 01:34:00]

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

[commit-bot: I haz the power, 2016-09-21 01:34:45]

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-09-21 02:29:04]

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

[commit-bot: I haz the power, 2016-09-21 02:29:05]

Dry run: This issue passed the CQ dry run.

[weiliangc, 2016-09-21 14:24:52]

The CQ bit was checked by weiliangc@chromium.org

[weiliangc, 2016-09-21 14:24:53]

LGTM

[commit-bot: I haz the power, 2016-09-21 14:25: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-09-21 14:30:16]

Description was changed from

==========
cc : Compute visible rects dynamically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc : Compute visible rects dynamically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-09-21 14:30:17]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-21 14:32:15]

Description was changed from

==========
cc : Compute visible rects dynamically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc : Compute visible rects dynamically (2)

This CL computes visible rects dynamically for all cases. It also renames
ExecuteCalculateDrawPropertiesWithPropertyTrees to
ExecuteCalculateDrawPropertiesAndSaveUpdateLayerList

BUG=642054
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[commit-bot: I haz the power, 2016-09-21 14:32:16]

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