cc : Make LayerImpl destruction independent of tree hierarchy

cc/layers/layer_impl.cc

Index: cc/layers/layer_impl.cc
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index 407449d774176c40a69ec524808bbb5d3ea21e35..98ea697d2e0c6c0ea4a3e8d3380bf06cb5467e9a 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -108,7 +108,7 @@ LayerImpl::~LayerImpl() {
     layer_tree_impl_->RemoveLayer(mask_layer_id_);
   if (replica_layer_)
     layer_tree_impl_->RemoveLayer(replica_layer_id_);

[ajuma, 2016/06/07 23:25:36]

Since masks and replicas are part of layers_ in LayerTreeImpl, will they already
have been destroyed at the same time that children got destroyed? That is, if
you were to DCHECK that the return value of the calls to RemoveLayer above is
always null, would that succeed?

[jaydasika, 2016/06/08 00:17:37]

I din't understand that fully. masks and replicas are in layers_ but not in
children_. But as you said, I think we need not explicitly delete them as they
are part of layers_. But I don't think they are sure to be deleted before the
layer itself. If might be true now because of the order in which we add layers
to layers_ but may not be true when we push layer list.

[jaydasika, 2016/06/08 00:49:32]

Scratch my previous comment, I understand what you mean.

[ajuma, 2016/06/08 13:48:02]

That's a good point about destruction order -- even if masks and replicas are in
layers_, there's no guarantee about the order in which things inside layers_
will be destroyed when layers_ gets cleared or destroyed. That is, we don't need
to delete the masks and replicas explicitly here, but we can't be sure they'll
be deleted before the layer itself. So the DCHECKs should be removed then.

[jaydasika, 2016/06/08 16:29:30]

I agree that there is no guarantee about destruction order, but I think it is
fine to have these DCHECKs because the destruction of layers can start only
after all the layers that are to be destroyed are removed from layers_. This is
because of the way LTI::DetachLayers is written. The function itself doesn't
lead to destructor being called. It removes all layers from layers_ and
transfers their ownership to a new vector that it returns. Layer destruction can
start after this new vector goes out of scope (so we can be sure all layers are
removed from layers_ at that point).

[ajuma, 2016/06/08 16:40:45]

Ah, got it, that makes sense.

-  ClearChildList();
+  children_.clear();
 }
 
 void LayerImpl::AddChild(std::unique_ptr<LayerImpl> child) {
@@ -130,14 +130,6 @@ void LayerImpl::SetParent(LayerImpl* parent) {
   parent_ = parent;
 }
 
-void LayerImpl::ClearChildList() {
-  if (children_.empty())
-    return;
-  for (auto* child : children_)
-    layer_tree_impl_->RemoveLayer(child->id());
-  children_.clear();
-}
-
 void LayerImpl::ClearLinksToOtherLayers() {
   children_.clear();
   mask_layer_ = nullptr;