Reset property tree indices when layer is removed from layer tree

An orphaned layer has no relation to property trees, so it is an error
for it to have property tree indices.

BUG=489725

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

Cr-Commit-Position: refs/heads/master@{#330739}
(cherry picked from commit 692444f78ce635ca7a5dc4e5e9cb925c64500fad)

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

Cr-Commit-Position: refs/branch-heads/2403@{#31}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}

8 files changed, 145 insertions, 47 deletions

diff --git a/cc/layers/layer.cc b/cc/layers/layer.cc
index 23c90c2..e19d3ff 100644
--- a/cc/layers/layer.cc
+++ b/cc/layers/layer.cc
@@ -53,6 +53,7 @@ Layer::Layer()
       transform_tree_index_(-1),
       opacity_tree_index_(-1),
       clip_tree_index_(-1),
+      property_tree_sequence_number_(-1),
       num_layer_or_descendants_with_copy_request_(0),
       num_layer_or_descendants_with_input_handler_(0),
       should_flatten_transform_from_property_tree_(false),
@@ -256,6 +257,9 @@ void Layer::SetParent(Layer* layer) {
 
   if (!layer_tree_host_)
     return;
+
+  layer_tree_host_->property_trees()->needs_rebuild = true;
+
   const LayerTreeSettings& settings = layer_tree_host_->settings();
   if (!settings.layer_transforms_should_scale_layer_contents)
     return;
@@ -984,6 +988,57 @@ void Layer::Set3dSortingContextId(int id) {
   SetNeedsCommit();
 }
 
+void Layer::SetTransformTreeIndex(int index) {
+  DCHECK(IsPropertyChangeAllowed());
+  if (transform_tree_index_ == index)
+    return;
+  transform_tree_index_ = index;
+  SetNeedsPushProperties();
+}
+
+int Layer::transform_tree_index() const {
+  if (!layer_tree_host_ ||
+      layer_tree_host_->property_trees()->sequence_number !=
+          property_tree_sequence_number_) {
+    return -1;
+  }
+  return transform_tree_index_;
+}
+
+void Layer::SetClipTreeIndex(int index) {
+  DCHECK(IsPropertyChangeAllowed());
+  if (clip_tree_index_ == index)
+    return;
+  clip_tree_index_ = index;
+  SetNeedsPushProperties();
+}
+
+int Layer::clip_tree_index() const {
+  if (!layer_tree_host_ ||
+      layer_tree_host_->property_trees()->sequence_number !=
+          property_tree_sequence_number_) {
+    return -1;
+  }
+  return clip_tree_index_;
+}
+
+void Layer::SetOpacityTreeIndex(int index) {
+  DCHECK(IsPropertyChangeAllowed());
+  if (opacity_tree_index_ == index)
+    return;
+  opacity_tree_index_ = index;
+  SetNeedsPushProperties();
+}
+
+int Layer::opacity_tree_index() const {
+  if (!layer_tree_host_ ||
+      layer_tree_host_->property_trees()->sequence_number !=
+          property_tree_sequence_number_) {
+    return -1;
+  }
+  return opacity_tree_index_;
+}
+
 void Layer::SetShouldFlattenTransform(bool should_flatten) {
   DCHECK(IsPropertyChangeAllowed());
   if (should_flatten_transform_ == should_flatten)
@@ -1128,9 +1183,9 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {
   DCHECK(!(TransformIsAnimating() && layer->TransformIsAnimatingOnImplOnly()));
   layer->Set3dSortingContextId(sorting_context_id_);
   layer->SetNumDescendantsThatDrawContent(num_descendants_that_draw_content_);
-  layer->set_transform_tree_index(transform_tree_index_);
-  layer->set_opacity_tree_index(opacity_tree_index_);
-  layer->set_clip_tree_index(clip_tree_index_);
+  layer->SetTransformTreeIndex(transform_tree_index());
+  layer->SetOpacityTreeIndex(opacity_tree_index());
+  layer->SetClipTreeIndex(clip_tree_index());
   layer->set_offset_to_transform_parent(offset_to_transform_parent_);
 
   layer->SetScrollClipLayer(scroll_clip_layer_id_);
diff --git a/cc/layers/layer.h b/cc/layers/layer.h
index 56decaa..9c7138e 100644
--- a/cc/layers/layer.h
+++ b/cc/layers/layer.h
@@ -476,27 +476,18 @@ class CC_EXPORT Layer : public base::RefCounted<Layer>,
   void Set3dSortingContextId(int id);
   int sorting_context_id() const { return sorting_context_id_; }
 
-  void set_transform_tree_index(int index) {
-    if (transform_tree_index_ == index)
-      return;
-    transform_tree_index_ = index;
-    SetNeedsPushProperties();
-  }
-  void set_clip_tree_index(int index) {
-    if (clip_tree_index_ == index)
-      return;
-    clip_tree_index_ = index;
-    SetNeedsPushProperties();
+  void set_property_tree_sequence_number(int sequence_number) {
+    property_tree_sequence_number_ = sequence_number;
   }
-  void set_opacity_tree_index(int index) {
-    if (opacity_tree_index_ == index)
-      return;
-    opacity_tree_index_ = index;
-    SetNeedsPushProperties();
-  }
-  int clip_tree_index() const { return clip_tree_index_; }
-  int transform_tree_index() const { return transform_tree_index_; }
-  int opacity_tree_index() const { return opacity_tree_index_; }
+
+  void SetTransformTreeIndex(int index);
+  int transform_tree_index() const;
+
+  void SetClipTreeIndex(int index);
+  int clip_tree_index() const;
+
+  void SetOpacityTreeIndex(int index);
+  int opacity_tree_index() const;
 
   void set_offset_to_transform_parent(gfx::Vector2dF offset) {
     if (offset_to_transform_parent_ == offset)
@@ -709,6 +700,7 @@ class CC_EXPORT Layer : public base::RefCounted<Layer>,
   int transform_tree_index_;
   int opacity_tree_index_;
   int clip_tree_index_;
+  int property_tree_sequence_number_;
   int num_layer_or_descendants_with_copy_request_;
   int num_layer_or_descendants_with_input_handler_;
   gfx::Vector2dF offset_to_transform_parent_;
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index c4e59d8..8b3467a 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -208,6 +208,21 @@ void LayerImpl::SetClipChildren(std::set<LayerImpl*>* children) {
   SetNeedsPushProperties();
 }
 
+void LayerImpl::SetTransformTreeIndex(int index) {
+  transform_tree_index_ = index;
+  SetNeedsPushProperties();
+}
+
+void LayerImpl::SetClipTreeIndex(int index) {
+  clip_tree_index_ = index;
+  SetNeedsPushProperties();
+}
+
+void LayerImpl::SetOpacityTreeIndex(int index) {
+  opacity_tree_index_ = index;
+  SetNeedsPushProperties();
+}
+
 void LayerImpl::PassCopyRequests(ScopedPtrVector<CopyOutputRequest>* requests) {
   if (requests->empty())
     return;
@@ -552,9 +567,9 @@ void LayerImpl::PushPropertiesTo(LayerImpl* layer) {
   layer->Set3dSortingContextId(sorting_context_id_);
   layer->SetNumDescendantsThatDrawContent(num_descendants_that_draw_content_);
 
-  layer->set_transform_tree_index(transform_tree_index_);
-  layer->set_opacity_tree_index(opacity_tree_index_);
-  layer->set_clip_tree_index(clip_tree_index_);
+  layer->SetTransformTreeIndex(transform_tree_index_);
+  layer->SetClipTreeIndex(clip_tree_index_);
+  layer->SetOpacityTreeIndex(opacity_tree_index_);
   layer->set_offset_to_transform_parent(offset_to_transform_parent_);
 
   LayerImpl* scroll_parent = nullptr;
diff --git a/cc/layers/layer_impl.h b/cc/layers/layer_impl.h
index 84bff48..08db87f 100644
--- a/cc/layers/layer_impl.h
+++ b/cc/layers/layer_impl.h
@@ -151,20 +151,15 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver,
     return scroll_children_.get();
   }
 
-  void set_transform_tree_index(int index) {
-    transform_tree_index_ = index;
-    SetNeedsPushProperties();
-  }
-  void set_clip_tree_index(int index) {
-    clip_tree_index_ = index;
-    SetNeedsPushProperties();
-  }
-  void set_opacity_tree_index(int index) {
-    opacity_tree_index_ = index;
-    SetNeedsPushProperties();
-  }
-  int clip_tree_index() const { return clip_tree_index_; }
+  void set_property_tree_sequence_number(int sequence_number) {}
+
+  void SetTransformTreeIndex(int index);
   int transform_tree_index() const { return transform_tree_index_; }
+
+  void SetClipTreeIndex(int index);
+  int clip_tree_index() const { return clip_tree_index_; }
+
+  void SetOpacityTreeIndex(int index);
   int opacity_tree_index() const { return opacity_tree_index_; }
 
   void set_offset_to_transform_parent(const gfx::Vector2dF& offset) {
diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc
index 591f48c..7c861bd 100644
--- a/cc/trees/layer_tree_host_common_unittest.cc
+++ b/cc/trees/layer_tree_host_common_unittest.cc
@@ -9596,5 +9596,32 @@ TEST_F(LayerTreeHostCommonTest, InputHandlersRecursiveUpdateTest) {
   EXPECT_EQ(root->num_layer_or_descendants_with_input_handler(), 0);
 }
 
+TEST_F(LayerTreeHostCommonTest, ResetPropertyTreeIndices) {
+  gfx::Transform identity;
+  gfx::Transform translate_z;
+  translate_z.Translate3d(0, 0, 10);
+
+  scoped_refptr<Layer> root = Layer::Create();
+  SetLayerPropertiesForTesting(root.get(), identity, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(800, 800), true, false);
+
+  scoped_refptr<Layer> child = Layer::Create();
+  SetLayerPropertiesForTesting(child.get(), translate_z, gfx::Point3F(),
+                               gfx::PointF(), gfx::Size(100, 100), true, false);
+
+  root->AddChild(child);
+
+  scoped_ptr<FakeLayerTreeHost> host(CreateFakeLayerTreeHost());
+  host->SetRootLayer(root);
+
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root.get());
+  EXPECT_NE(-1, child->transform_tree_index());
+
+  child->RemoveFromParent();
+
+  ExecuteCalculateDrawPropertiesWithPropertyTrees(root.get());
+  EXPECT_EQ(-1, child->transform_tree_index());
+}
+
 }  // namespace
 }  // namespace cc
diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc
index 2603140..d369ad9 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -382,7 +382,7 @@ void TransformTree::UpdateSnapping(TransformNode* node) {
   node->data.scroll_snap = translation;
 }
 
-PropertyTrees::PropertyTrees() : needs_rebuild(true) {
+PropertyTrees::PropertyTrees() : needs_rebuild(true), sequence_number(0) {
 }
 
 }  // namespace cc
diff --git a/cc/trees/property_tree.h b/cc/trees/property_tree.h
index 2341493b1..caa819c 100644
--- a/cc/trees/property_tree.h
+++ b/cc/trees/property_tree.h
@@ -140,8 +140,16 @@ class CC_EXPORT PropertyTree {
 
   int Insert(const T& tree_node, int parent_id);
 
-  T* Node(int i) { return i > -1 ? &nodes_[i] : nullptr; }
-  const T* Node(int i) const { return i > -1 ? &nodes_[i] : nullptr; }
+  T* Node(int i) {
+    // TODO(vollick): remove this.
+    CHECK(i < static_cast<int>(nodes_.size()));
+    return i > -1 ? &nodes_[i] : nullptr;
+  }
+  const T* Node(int i) const {
+    // TODO(vollick): remove this.
+    CHECK(i < static_cast<int>(nodes_.size()));
+    return i > -1 ? &nodes_[i] : nullptr;
+  }
 
   T* parent(const T* t) { return Node(t->parent_id); }
   const T* parent(const T* t) const { return Node(t->parent_id); }
@@ -245,6 +253,7 @@ class CC_EXPORT PropertyTrees final {
   OpacityTree opacity_tree;
   ClipTree clip_tree;
   bool needs_rebuild;
+  int sequence_number;
 };
 
 }  // namespace cc
diff --git a/cc/trees/property_tree_builder.cc b/cc/trees/property_tree_builder.cc
index 6d8c989..815692d 100644
--- a/cc/trees/property_tree_builder.cc
+++ b/cc/trees/property_tree_builder.cc
@@ -38,6 +38,7 @@ struct DataForRecursion {
   bool ancestor_clips_subtree;
   const gfx::Transform* device_transform;
   gfx::Vector2dF scroll_compensation_adjustment;
+  int sequence_number;
 };
 
 template <typename LayerType>
@@ -148,7 +149,7 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
         data_for_children->clip_tree->Insert(node, parent_id);
   }
 
-  layer->set_clip_tree_index(
+  layer->SetClipTreeIndex(
       has_unclipped_surface ? 0 : data_for_children->clip_tree_parent);
 
   // TODO(awoloszyn): Right now when we hit a node with a replica, we reset the
@@ -233,14 +234,14 @@ bool AddTransformNodeIfNeeded(
                                           local_offset);
     layer->set_should_flatten_transform_from_property_tree(
         data_from_ancestor.should_flatten);
-    layer->set_transform_tree_index(parent_index);
+    layer->SetTransformTreeIndex(parent_index);
     return false;
   }
 
   data_for_children->transform_tree->Insert(TransformNode(), parent_index);
 
   TransformNode* node = data_for_children->transform_tree->back();
-  layer->set_transform_tree_index(node->id);
+  layer->SetTransformTreeIndex(node->id);
 
   node->data.scrolls = is_scrollable;
   node->data.flattens_inherited_transform = data_for_children->should_flatten;
@@ -328,7 +329,7 @@ void AddOpacityNodeIfNeeded(
   int parent_id = data_from_ancestor.opacity_tree_parent;
 
   if (!requires_node) {
-    layer->set_opacity_tree_index(parent_id);
+    layer->SetOpacityTreeIndex(parent_id);
     data_for_children->opacity_tree_parent = parent_id;
     return;
   }
@@ -338,13 +339,14 @@ void AddOpacityNodeIfNeeded(
   node.data = layer->opacity();
   data_for_children->opacity_tree_parent =
       data_for_children->opacity_tree->Insert(node, parent_id);
-  layer->set_opacity_tree_index(data_for_children->opacity_tree_parent);
+  layer->SetOpacityTreeIndex(data_for_children->opacity_tree_parent);
 }
 
 template <typename LayerType>
 void BuildPropertyTreesInternal(
     LayerType* layer,
     const DataForRecursion<LayerType>& data_from_parent) {
+  layer->set_property_tree_sequence_number(data_from_parent.sequence_number);
   DataForRecursion<LayerType> data_for_children(data_from_parent);
   if (layer->render_surface())
     data_for_children.render_target = layer;
@@ -385,6 +387,8 @@ void BuildPropertyTreesTopLevelInternal(LayerType* root_layer,
                                         const gfx::Rect& viewport,
                                         const gfx::Transform& device_transform,
                                         PropertyTrees* property_trees) {
+  property_trees->sequence_number++;
+
   DataForRecursion<LayerType> data_for_recursion;
   data_for_recursion.transform_tree = &property_trees->transform_tree;
   data_for_recursion.clip_tree = &property_trees->clip_tree;
@@ -405,6 +409,7 @@ void BuildPropertyTreesTopLevelInternal(LayerType* root_layer,
   data_for_recursion.transform_tree->clear();
   data_for_recursion.clip_tree->clear();
   data_for_recursion.opacity_tree->clear();
+  data_for_recursion.sequence_number = property_trees->sequence_number;
 
   ClipNode root_clip;
   root_clip.data.clip = viewport;