New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 617791 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 617366



Sign in to add a comment

Remove layer hierarchy dependency in layer destruction

Project Member Reported by jaydasika@chromium.org, Jun 6 2016

Issue description

We depend on layer hierarchy in layer destruction because destroying a layer destroys all its children (~LayerImpl() calls ClearChildList()).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 8 2016

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

commit e536d2292fa548ee6231ff23ac65203c261e4a14
Author: jaydasika <jaydasika@chromium.org>
Date: Wed Jun 08 17:22:18 2016

cc : Make LayerImpl destruction independent of tree hierarchy

Instead of deleting all layers when the root is deleted, we delete
all layers when the layer tree impl is shutdown.

BUG= 617791 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2043963005
Cr-Commit-Position: refs/heads/master@{#398606}

[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/layers/layer_impl.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/layers/layer_impl.h
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/tree_synchronizer.cc
[modify] https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14/cc/trees/tree_synchronizer_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 8 2016

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

commit f3a9e5397cce794930f73f6c6a43c0621ecf06a6
Author: rockot <rockot@chromium.org>
Date: Wed Jun 08 18:23:59 2016

Revert of cc : Make LayerImpl destruction independent of tree hierarchy (patchset #2 id:20001 of https://codereview.chromium.org/2043963005/ )

Reason for revert:
Tentative revert for strange linker failures:

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder%20%28dbg%29/builds/156821

I already tried reverting the other CL in the blame list (mine) with no success. Doesn't make sense that this CL would cause it either, but just ruling it out. I'll revert the revert if the builder stays red.

Original issue's description:
> cc : Make LayerImpl destruction independent of tree hierarchy
>
> Instead of deleting all layers when the root is deleted, we delete
> all layers when the layer tree impl is shutdown.
>
> BUG= 617791 
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14
> Cr-Commit-Position: refs/heads/master@{#398606}

TBR=ajuma@chromium.org,weiliangc@chromium.org,jaydasika@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 617791 

Review-Url: https://codereview.chromium.org/2054483002
Cr-Commit-Position: refs/heads/master@{#398623}

[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/layers/layer_impl.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/layers/layer_impl.h
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/tree_synchronizer.cc
[modify] https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6/cc/trees/tree_synchronizer_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 8 2016

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

commit 01cf254d590c4d82dd12ea8bd9a54d71bc9281a9
Author: rockot <rockot@chromium.org>
Date: Wed Jun 08 19:07:59 2016

Reland of cc : Make LayerImpl destruction independent of tree hierarchy (patchset #1 id:1 of https://codereview.chromium.org/2054483002/ )

Reason for revert:
Didn't fix anything

Original issue's description:
> Revert of cc : Make LayerImpl destruction independent of tree hierarchy (patchset #2 id:20001 of https://codereview.chromium.org/2043963005/ )
>
> Reason for revert:
> Tentative revert for strange linker failures:
>
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder%20%28dbg%29/builds/156821
>
> I already tried reverting the other CL in the blame list (mine) with no success. Doesn't make sense that this CL would cause it either, but just ruling it out. I'll revert the revert if the builder stays red.
>
> Original issue's description:
> > cc : Make LayerImpl destruction independent of tree hierarchy
> >
> > Instead of deleting all layers when the root is deleted, we delete
> > all layers when the layer tree impl is shutdown.
> >
> > BUG= 617791 
> > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> >
> > Committed: https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14
> > Cr-Commit-Position: refs/heads/master@{#398606}
>
> TBR=ajuma@chromium.org,weiliangc@chromium.org,jaydasika@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 617791 
>
> Committed: https://crrev.com/f3a9e5397cce794930f73f6c6a43c0621ecf06a6
> Cr-Commit-Position: refs/heads/master@{#398623}

TBR=ajuma@chromium.org,weiliangc@chromium.org,jaydasika@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 617791 

Review-Url: https://codereview.chromium.org/2048863003
Cr-Commit-Position: refs/heads/master@{#398632}

[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/layers/layer_impl.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/layers/layer_impl.h
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/tree_synchronizer.cc
[modify] https://crrev.com/01cf254d590c4d82dd12ea8bd9a54d71bc9281a9/cc/trees/tree_synchronizer_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 8 2016

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

commit ef1625ae9feafee9ddaa2a58b9476175d7956b8b
Author: rockot <rockot@chromium.org>
Date: Wed Jun 08 19:08:44 2016

Revert of cc : Make LayerImpl destruction independent of tree hierarchy (patchset #2 id:20001 of https://codereview.chromium.org/2043963005/ )

Reason for revert:
Didn't fix anything.

Original issue's description:
> cc : Make LayerImpl destruction independent of tree hierarchy
>
> Instead of deleting all layers when the root is deleted, we delete
> all layers when the layer tree impl is shutdown.
>
> BUG= 617791 
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14
> Cr-Commit-Position: refs/heads/master@{#398606}

TBR=ajuma@chromium.org,weiliangc@chromium.org,jaydasika@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 617791 

Review-Url: https://codereview.chromium.org/2044253004
Cr-Commit-Position: refs/heads/master@{#398633}

[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/layers/layer_impl.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/layers/layer_impl.h
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/tree_synchronizer.cc
[modify] https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b/cc/trees/tree_synchronizer_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 8 2016

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

commit 2176f9255c324d96d8d980ccfae345225cee1872
Author: rockot <rockot@chromium.org>
Date: Wed Jun 08 19:18:32 2016

Reland of cc : Make LayerImpl destruction independent of tree hierarchy (patchset #1 id:1 of https://codereview.chromium.org/2044253004/ )

Reason for revert:
around and around we go :D

Original issue's description:
> Revert of cc : Make LayerImpl destruction independent of tree hierarchy (patchset #2 id:20001 of https://codereview.chromium.org/2043963005/ )
>
> Reason for revert:
> Didn't fix anything.
>
> Original issue's description:
> > cc : Make LayerImpl destruction independent of tree hierarchy
> >
> > Instead of deleting all layers when the root is deleted, we delete
> > all layers when the layer tree impl is shutdown.
> >
> > BUG= 617791 
> > CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> >
> > Committed: https://crrev.com/e536d2292fa548ee6231ff23ac65203c261e4a14
> > Cr-Commit-Position: refs/heads/master@{#398606}
>
> TBR=ajuma@chromium.org,weiliangc@chromium.org,jaydasika@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 617791 
>
> Committed: https://crrev.com/ef1625ae9feafee9ddaa2a58b9476175d7956b8b
> Cr-Commit-Position: refs/heads/master@{#398633}

TBR=ajuma@chromium.org,weiliangc@chromium.org,jaydasika@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 617791 

Review-Url: https://codereview.chromium.org/2049883002
Cr-Commit-Position: refs/heads/master@{#398637}

[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/layers/layer_impl.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/layers/layer_impl.h
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/layers/picture_layer_impl_perftest.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/tree_synchronizer.cc
[modify] https://crrev.com/2176f9255c324d96d8d980ccfae345225cee1872/cc/trees/tree_synchronizer_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment