Don't clear render surfaces unnecessarily.

Don't clear render surfaces unnecessarily.

As an optimization, we used to clear render surfaces when they were
elided from the RenderSurfaceLayerList. It turns out that these surfaces
are still important from time to time; a scroll child will need to
access the transforms of these surfaces in order to put its clip in the
right space.

This CL retains the layer lists and it also updates RenderSurface to
own a LayerList rather than a RenderSurfaceLayerList (which was always
a bit of a kludge).

R=danakj@chromium.org
BUG=366386
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266519

[danakj, 2014-04-25 22:59:24]

Let's clear the RenderSurface::layer_list_ instead of deleting the
RenderSurface. That way we don't introduce new long-lived layer refs.

https://codereview.chromium.org/250803013/diff/1/cc/layers/layer_lists.h
File cc/layers/layer_lists.h (right):

https://codereview.chromium.org/250803013/diff/1/cc/layers/layer_lists.h#newc...
cc/layers/layer_lists.h:41: LayerList* layer_list() { return &list_; }
Can we call this AsLayerList() and have it return LayerList& instead?

https://codereview.chromium.org/250803013/diff/1/cc/trees/layer_tree_host_com...
File cc/trees/layer_tree_host_common.h (right):

https://codereview.chromium.org/250803013/diff/1/cc/trees/layer_tree_host_com...
cc/trees/layer_tree_host_common.h:142: static LayerImpl*
get_child_as_raw_ptr(const LayerImplList& children,
Would you mind doing a followup CL to rename all three of these methods to not
refer to children?

[Ian Vollick, 2014-04-25 23:14:39]

https://codereview.chromium.org/250803013/diff/1/cc/layers/layer_lists.h
File cc/layers/layer_lists.h (right):

https://codereview.chromium.org/250803013/diff/1/cc/layers/layer_lists.h#newc...
cc/layers/layer_lists.h:41: LayerList* layer_list() { return &list_; }
On 2014/04/25 22:59:24, danakj wrote:
> Can we call this AsLayerList() and have it return LayerList& instead?

Done.

https://codereview.chromium.org/250803013/diff/1/cc/trees/layer_tree_host_com...
File cc/trees/layer_tree_host_common.h (right):

https://codereview.chromium.org/250803013/diff/1/cc/trees/layer_tree_host_com...
cc/trees/layer_tree_host_common.h:142: static LayerImpl*
get_child_as_raw_ptr(const LayerImplList& children,
On 2014/04/25 22:59:24, danakj wrote:
> Would you mind doing a followup CL to rename all three of these methods to not
> refer to children?

Totally.

[danakj, 2014-04-25 23:17:28]

LGTM

[danakj, 2014-04-25 23:17:32]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-04-25 23:22:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/40001

[commit-bot: I haz the power, 2014-04-26 03:14:14]

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

[commit-bot: I haz the power, 2014-04-26 03:14:15]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Ian Vollick, 2014-04-26 09:53:38]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-26 09:54:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/40001

[commit-bot: I haz the power, 2014-04-26 10:43:02]

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

[commit-bot: I haz the power, 2014-04-26 10:43:02]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Ian Vollick, 2014-04-26 11:58:18]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-26 11:58:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/40001

[commit-bot: I haz the power, 2014-04-26 12:37:36]

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

[commit-bot: I haz the power, 2014-04-26 12:37:36]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Ian Vollick, 2014-04-26 13:51:48]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-26 13:52:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/40001

[commit-bot: I haz the power, 2014-04-26 14:25:52]

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

[commit-bot: I haz the power, 2014-04-26 14:25:52]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Ian Vollick, 2014-04-27 01:39:36]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-27 01:40:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/40001

[commit-bot: I haz the power, 2014-04-27 02:18:23]

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

[commit-bot: I haz the power, 2014-04-27 02:18:23]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Ian Vollick, 2014-04-27 02:19:24]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-27 02:19:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/40001

[commit-bot: I haz the power, 2014-04-27 02:29:56]

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

[commit-bot: I haz the power, 2014-04-27 02:29:57]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Ian Vollick, 2014-04-27 17:56:06]

On 2014/04/27 02:29:57, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on linux_chromium_rel

Looks like the failures are legit. Keeping the empty render surfaces around is
breaking some RSLL assumptions that the layer_iterators depend on. Looking into
it now.

[Ian Vollick, 2014-04-27 18:42:16]

On 2014/04/27 17:56:06, Ian Vollick wrote:
> On 2014/04/27 02:29:57, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   tryserver.chromium on linux_chromium_rel
> 
> Looks like the failures are legit. Keeping the empty render surfaces around is
> breaking some RSLL assumptions that the layer_iterators depend on. Looking
into
> it now.

Found it: after CalcDrawProps recurs into the children, we check the existence
of the render surface to determine if we need to add the child to |descendants|.
This is no longer correct because we don't clear those render surfaces; we also
have to check if the surface has an empty layer list.

Going to kick off some tryjobs now. Fingers crossed.

[Ian Vollick, 2014-04-27 18:42:16]

On 2014/04/27 17:56:06, Ian Vollick wrote:
> On 2014/04/27 02:29:57, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   tryserver.chromium on linux_chromium_rel
> 
> Looks like the failures are legit. Keeping the empty render surfaces around is
> breaking some RSLL assumptions that the layer_iterators depend on. Looking
into
> it now.

Found it: after CalcDrawProps recurs into the children, we check the existence
of the render surface to determine if we need to add the child to |descendants|.
This is no longer correct because we don't clear those render surfaces; we also
have to check if the surface has an empty layer list.

Going to kick off some tryjobs now. Fingers crossed.

[Ian Vollick, 2014-04-27 20:21:15]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-27 20:21:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/60001

[Ian Vollick, 2014-04-28 07:57:00]

The CQ bit was unchecked by vollick@chromium.org

[Ian Vollick, 2014-04-28 07:57:15]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-04-28 07:57:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/250803013/60001

[commit-bot: I haz the power, 2014-04-28 12:20:37]

Change committed as 266519

[danakj, 2014-04-28 15:02:16]

https://codereview.chromium.org/250803013/diff/60001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/250803013/diff/60001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_common.cc:2077:
!child->render_surface()->layer_list().empty() &&
How come the render surface's content rect is not empty if the layer list is
empty?