More specifically, LayerShouldBeSkipped should just be : !has_inherited_invertible_or_animated_transform || hidden_by_bf_visibility (everything else can be deleted)

I think we can also merge the two paths used by main and impl. I think probably I can do that in my CL, but it seems better to do that after the simplification.

Yes, I agree. After simplification they should be the same.

I will first remove subtree skipping in layer_tree_host_common and then try to simplify the logic.

The CL I'm currently working on may have code conflict with this issue: https://codereview.chromium.org/1864183002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e83ea4d3b872053c0304bc81b71262041be8c3b

commit 5e83ea4d3b872053c0304bc81b71262041be8c3b
Author: jaydasika <jaydasika@chromium.org>
Date: Thu Apr 07 19:26:55 2016

cc : Remove subtree skipping in LayerTreeHostCommon

This CL makes sure we visit all layers while calculating
render target and RSLL. This will help us in deleting code
in layer skipping that is there to handle subtree skipping.

BUG= 601088 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1865693003

Cr-Commit-Position: refs/heads/master@{#385836}

[modify] https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b/cc/trees/layer_tree_host_common.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc1427651679c7035211920eeb01944593a6b636

commit fc1427651679c7035211920eeb01944593a6b636
Author: jaydasika <jaydasika@chromium.org>
Date: Fri Apr 08 19:02:40 2016

Revert of cc : Remove subtree skipping in LayerTreeHostCommon (patchset #5 id:80001 of https://codereview.chromium.org/1865693003/ )

Reason for revert:
crbug.com/601812

Original issue's description:
> cc : Remove subtree skipping in LayerTreeHostCommon
>
> This CL makes sure we visit all layers while calculating
> render target and RSLL. This will help us in deleting code
> in layer skipping that is there to handle subtree skipping.
>
> BUG= 601088 
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/5e83ea4d3b872053c0304bc81b71262041be8c3b
> Cr-Commit-Position: refs/heads/master@{#385836}

TBR=weiliangc@chromium.org,ajuma@chromium.org,vollick@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 601088 , 601812
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1872003003

Cr-Commit-Position: refs/heads/master@{#386159}

[modify] https://crrev.com/fc1427651679c7035211920eeb01944593a6b636/cc/trees/layer_tree_host_common.cc

Before removing subtree skipping, we need to ensure that every layer in the subtree gets skipped using layer skipping when a layer has singular transform and no animation. I think we can do this by adding a force_skip bool to transform tree.

if (parent && parent->force_skip)
  node->force_skip = true
else
  node->force_skip = !is_invertible && !animation

Any thoughts or other ideas ?

I don't think the transform tree should be aware of the concept of skipping, but if there's a transform-related concept that we need in order to make skipping decisions (like we have to_screen_is_animated and ancestors_are_invertbile), it could make sense to add that.

What is it specifically about this case that's problematic? Is it just that we're skipping a layer without skipping all of its descendants, or is it more than that?

It is just that we're skipping a layer without skipping all of its descendants.

It'd be good to try to address what goes wrong in that situation. Is it just that the descendant layer has a null render target? If so, note that after Wei's patch (https://codereview.chromium.org/1868003002/), skipped layers won't have null render targets, since the render target will be found using the effect tree rather than set as a draw property.

In general, we should make possible to skip a layer without skipping all descendants, since eventually we'll just have a list of layers and will need to be able to skip an arbitrary subset of these. (But it's true that in the specific example we're discussing here, it makes sense to skip all the descendants of a singular non-animated transform.)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f187b06b55a08ca8efb94361282428d7f8532113

commit f187b06b55a08ca8efb94361282428d7f8532113
Author: jaydasika <jaydasika@chromium.org>
Date: Mon Apr 11 23:30:27 2016

cc : Add node and ancestors are animated or invertible to transform tree

This is needed to decide if a layer can be skipped during visible rect
and RSLL computation.

BUG= 601088 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1876903003

Cr-Commit-Position: refs/heads/master@{#386514}

[modify] https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113/cc/proto/property_tree.proto
[modify] https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113/cc/trees/property_tree.cc
[modify] https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113/cc/trees/property_tree.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f16ba33dccc2da7772619c6afe075be37190f08

commit 6f16ba33dccc2da7772619c6afe075be37190f08
Author: jaydasika <jaydasika@chromium.org>
Date: Tue Apr 12 15:04:55 2016

cc : Reland remove subtree skipping in LayerTreeHostCommon

This CL makes sure we visit all layers while calculating RSLL. This
will help us in deleting code in layer skipping that is there to handle
subtree skipping

BUG= 601088 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1883443002

Cr-Commit-Position: refs/heads/master@{#386687}

[modify] https://crrev.com/6f16ba33dccc2da7772619c6afe075be37190f08/cc/trees/layer_tree_host_common.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8185d3075363d17df8cd2348ffbc27af8740f3ca

commit 8185d3075363d17df8cd2348ffbc27af8740f3ca
Author: jaydasika <jaydasika@chromium.org>
Date: Thu Apr 14 15:20:06 2016

cc : Simplify layer skipping logic

Since we no longer skip subtrees, we can remove the logic that supports
subtree skipping.

BUG= 601088 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1884613005

Cr-Commit-Position: refs/heads/master@{#387320}

[modify] https://crrev.com/8185d3075363d17df8cd2348ffbc27af8740f3ca/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/8185d3075363d17df8cd2348ffbc27af8740f3ca/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/8185d3075363d17df8cd2348ffbc27af8740f3ca/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/8185d3075363d17df8cd2348ffbc27af8740f3ca/cc/trees/property_tree.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a

commit 7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a
Author: jaydasika <jaydasika@chromium.org>
Date: Thu Apr 14 17:18:57 2016

cc : Delete unused properties in effect node

There are unused after simplification of layer skipping logic

BUG= 601088 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1888073002

Cr-Commit-Position: refs/heads/master@{#387346}

[modify] https://crrev.com/7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a/cc/proto/property_tree.proto
[modify] https://crrev.com/7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a/cc/trees/property_tree.cc
[modify] https://crrev.com/7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a/cc/trees/property_tree.h
[modify] https://crrev.com/7c8b702e187b1fefdd6bf9dfd9aaa303dddfdc5a/cc/trees/property_tree_builder.cc