cc: Add id->LayerImpl map to LayerTreeImpl

This is a prerequisite to two LayerTreeImpls, as PictureLayerImpls in the
active tree may need to ask about their pending tree sibilings with the
same id.  Once this lands, functionality like findScrollingLayerbyId can
be wrapped into this too.

LayerImpl now registers itself with its tree on creation and destruction.
As there are now asserts that a given id is not already in use, a number
of tests have been updated to not duplicate layer ids.

There doesn't technically need to be an unregister step, but as LayerImpl
doesn't necessarily have to be in the LayerImpl tree rooted in the
LayerTreeImpl, this unregister check will blow up if anybody has a layer
outliving their registered tree (which is bad).

As LayerImpl has single ownership, it's impossible to assign the same
non-NULL replica or mask to a layer, so the LayerImpl unit tests are
updated to not test this anymore.  Masks and replicas are also modified
to keep their id in sync with their pointer during tree synchronization
to prevent asserts about duplicate ids.

R=danakj@chromium.org
BUG=155209


Review URL: https://chromiumcodereview.appspot.com/11575018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@173110 0039d316-1c4b-4281-b951-d872f2087c98

cc/layer_impl.cc

@@ -56,6 +56,7 @@ LayerImpl::LayerImpl(LayerTreeImpl* treeImpl, int id)
 {
     DCHECK(m_layerId > 0);
     DCHECK(m_layerTreeImpl);
+    m_layerTreeImpl->RegisterLayer(this);
 }
 
 LayerImpl::~LayerImpl()
@@ -63,6 +64,7 @@ LayerImpl::~LayerImpl()
 #ifndef NDEBUG
     DCHECK(!m_betweenWillDrawAndDidDraw);
 #endif
+    m_layerTreeImpl->UnregisterLayer(this);
 }
 
 void LayerImpl::addChild(scoped_ptr<LayerImpl> child)
@@ -494,32 +496,46 @@ void LayerImpl::setBounds(const gfx::Size& bounds)
 
 void LayerImpl::setMaskLayer(scoped_ptr<LayerImpl> maskLayer)
 {
-    if (maskLayer)
+    int newLayerId = maskLayer ? maskLayer->id() : -1;
-        DCHECK_EQ(layerTreeImpl(), maskLayer->layerTreeImpl());
-    m_maskLayer = maskLayer.Pass();
 
-    int newLayerId = m_maskLayer ? m_maskLayer->id() : -1;
+    if (maskLayer) {
-    if (newLayerId == m_maskLayerId)
+        DCHECK_EQ(layerTreeImpl(), maskLayer->layerTreeImpl());
+        DCHECK_NE(newLayerId, m_maskLayerId);
+    } else if (newLayerId == m_maskLayerId)
         return;
 
+    m_maskLayer = maskLayer.Pass();
     m_maskLayerId = newLayerId;
     noteLayerPropertyChangedForSubtree();
 }
 
+scoped_ptr<LayerImpl> LayerImpl::takeMaskLayer()
+{
+  m_maskLayerId = -1;
+  return m_maskLayer.Pass();
+}
+
 void LayerImpl::setReplicaLayer(scoped_ptr<LayerImpl> replicaLayer)
 {
-    if (replicaLayer)
+    int newLayerId = replicaLayer ? replicaLayer->id() : -1;
-        DCHECK_EQ(layerTreeImpl(), replicaLayer->layerTreeImpl());
-    m_replicaLayer = replicaLayer.Pass();
 
-    int newLayerId = m_replicaLayer ? m_replicaLayer->id() : -1;
+    if (replicaLayer) {
-    if (newLayerId == m_replicaLayerId)
+        DCHECK_EQ(layerTreeImpl(), replicaLayer->layerTreeImpl());
+        DCHECK_NE(newLayerId, m_replicaLayerId);
+    } else if (newLayerId == m_replicaLayerId)
         return;
 
+    m_replicaLayer = replicaLayer.Pass();
     m_replicaLayerId = newLayerId;
     noteLayerPropertyChangedForSubtree();
 }
 
+scoped_ptr<LayerImpl> LayerImpl::takeReplicaLayer()
+{
+  m_replicaLayerId = -1;
+  return m_replicaLayer.Pass();
+}
+
 void LayerImpl::setDrawsContent(bool drawsContent)
 {
     if (m_drawsContent == drawsContent)

cc/layer_impl.h

@@ -293,6 +293,9 @@ protected:
     static std::string indentString(int indent);
 
 private:
+    scoped_ptr<LayerImpl> takeMaskLayer();
+    scoped_ptr<LayerImpl> takeReplicaLayer();
+
     void setParent(LayerImpl* parent) { m_parent = parent; }
     friend class TreeSynchronizer;
     void clearChildList(); // Warning: This does not preserve tree structure invariants and so is only exposed to the tree synchronizer.

cc/layer_impl_unittest.cc

@@ -228,10 +228,8 @@ TEST(LayerImplTest, VerifyNeedsUpdateDrawProperties)
     // Unrelated functions, set to the same values, no needs update.
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setAnchorPointZ(arbitraryNumber));
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setFilter(arbitraryFilter));
-    VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setMaskLayer(LayerImpl::create(hostImpl.activeTree(), 4)));
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setMasksToBounds(true));
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setContentsOpaque(true));
-    VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setReplicaLayer(LayerImpl::create(hostImpl.activeTree(), 5)));
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setPosition(arbitraryPointF));
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setPreserves3D(true));
     VERIFY_NO_NEEDS_UPDATE_DRAW_PROPERTIES(root->setDoubleSided(false)); // constructor initializes it to "true".

cc/layer_tree_host_impl_unittest.cc

@@ -1073,27 +1073,27 @@ TEST_P(LayerTreeHostImplTest, prepareToDrawFailsWhenAnimationUsesCheckerboard)
     m_hostImpl->didDrawAllLayers(frame);
 
     // When a texture is missing and we're not animating, we draw as usual with checkerboarding.
-    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(m_hostImpl->activeTree(), 1));
+    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(m_hostImpl->activeTree(), 3));
     root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
-    root->addChild(MissingTextureAnimatingLayer::create(m_hostImpl->activeTree(), 2, true, false, false, m_hostImpl->resourceProvider()));
+    root->addChild(MissingTextureAnimatingLayer::create(m_hostImpl->activeTree(), 4, true, false, false, m_hostImpl->resourceProvider()));
 
     EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
     m_hostImpl->drawLayers(frame);
     m_hostImpl->didDrawAllLayers(frame);
 
     // When a texture is missing and we're animating, we don't want to draw anything.
-    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(m_hostImpl->activeTree(), 1));
+    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(m_hostImpl->activeTree(), 5));
     root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
-    root->addChild(MissingTextureAnimatingLayer::create(m_hostImpl->activeTree(), 2, true, false, true, m_hostImpl->resourceProvider()));
+    root->addChild(MissingTextureAnimatingLayer::create(m_hostImpl->activeTree(), 6, true, false, true, m_hostImpl->resourceProvider()));
 
     EXPECT_FALSE(m_hostImpl->prepareToDraw(frame));
     m_hostImpl->drawLayers(frame);
     m_hostImpl->didDrawAllLayers(frame);
 
     // When the layer skips draw and we're animating, we still draw the frame.
-    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(m_hostImpl->activeTree(), 1));
+    m_hostImpl->setRootLayer(DidDrawCheckLayer::create(m_hostImpl->activeTree(), 7));
     root = static_cast<DidDrawCheckLayer*>(m_hostImpl->rootLayer());
-    root->addChild(MissingTextureAnimatingLayer::create(m_hostImpl->activeTree(), 2, false, true, true, m_hostImpl->resourceProvider()));
+    root->addChild(MissingTextureAnimatingLayer::create(m_hostImpl->activeTree(), 8, false, true, true, m_hostImpl->resourceProvider()));
 
     EXPECT_TRUE(m_hostImpl->prepareToDraw(frame));
     m_hostImpl->drawLayers(frame);
@@ -2479,10 +2479,10 @@ TEST_P(LayerTreeHostImplTest, outputSurfaceLostAndRestoredNotificationSentToAllL
     m_hostImpl->setRootLayer(OutputSurfaceLostNotificationCheckLayer::create(m_hostImpl->activeTree(), 1));
     OutputSurfaceLostNotificationCheckLayer* root = static_cast<OutputSurfaceLostNotificationCheckLayer*>(m_hostImpl->rootLayer());
 
-    root->addChild(OutputSurfaceLostNotificationCheckLayer::create(m_hostImpl->activeTree(), 1));
+    root->addChild(OutputSurfaceLostNotificationCheckLayer::create(m_hostImpl->activeTree(), 2));
     OutputSurfaceLostNotificationCheckLayer* layer1 = static_cast<OutputSurfaceLostNotificationCheckLayer*>(root->children()[0]);
 
-    layer1->addChild(OutputSurfaceLostNotificationCheckLayer::create(m_hostImpl->activeTree(), 2));
+    layer1->addChild(OutputSurfaceLostNotificationCheckLayer::create(m_hostImpl->activeTree(), 3));
     OutputSurfaceLostNotificationCheckLayer* layer2 = static_cast<OutputSurfaceLostNotificationCheckLayer*>(layer1->children()[0]);
 
     EXPECT_FALSE(root->didLoseOutputSurfaceCalled());

cc/layer_tree_impl.cc

@@ -19,6 +19,9 @@ LayerTreeImpl::LayerTreeImpl(LayerTreeHostImpl* layer_tree_host_impl)
 }
 
 LayerTreeImpl::~LayerTreeImpl() {
+  // Need to explicitly clear the tree prior to destroying this so that
+  // the LayerTreeImpl pointer is still valid in the LayerImpl dtor.
+  root_layer_.reset();
 }
 
 static LayerImpl* findRootScrollLayer(LayerImpl* layer)
@@ -68,6 +71,21 @@ void LayerTreeImpl::ClearCurrentlyScrollingLayer() {
   scrolling_layer_id_from_previous_tree_ = 0;
 }
 
+LayerImpl* LayerTreeImpl::LayerById(int id) {
+  LayerIdMap::iterator iter = layer_id_map_.find(id);
+  return iter != layer_id_map_.end() ? iter->second : NULL;
+}
+
+void LayerTreeImpl::RegisterLayer(LayerImpl* layer) {
+  DCHECK(!LayerById(layer->id()));
+  layer_id_map_[layer->id()] = layer;
+}
+
+void LayerTreeImpl::UnregisterLayer(LayerImpl* layer) {
+  DCHECK(LayerById(layer->id()));
+  layer_id_map_.erase(layer->id());
+}
+
 const LayerTreeSettings& LayerTreeImpl::settings() const {
   return layer_tree_host_impl_->settings();
 }

cc/layer_tree_impl.h

@@ -5,8 +5,20 @@
 #ifndef CC_LAYER_TREE_IMPL_H_
 #define CC_LAYER_TREE_IMPL_H_
 
+#include "base/hash_tables.h"
 #include "cc/layer_impl.h"
 
+#if defined(COMPILER_GCC)
+namespace BASE_HASH_NAMESPACE {
+template<>
+struct hash<cc::LayerImpl*> {
+  size_t operator()(cc::LayerImpl* ptr) const {
+    return hash<size_t>()(reinterpret_cast<size_t>(ptr));
+  }
+};
+} // namespace BASE_HASH_NAMESPACE
+#endif // COMPILER
+
 namespace cc {
 
 class DebugRectHistory;
@@ -70,6 +82,12 @@ class CC_EXPORT LayerTreeImpl {
 
   void ClearCurrentlyScrollingLayer();
 
+  LayerImpl* LayerById(int id);
+
+  // These should be called by LayerImpl's ctor/dtor.
+  void RegisterLayer(LayerImpl* layer);
+  void UnregisterLayer(LayerImpl* layer);
+
 protected:
   LayerTreeImpl(LayerTreeHostImpl* layer_tree_host_impl);
 
@@ -80,6 +98,9 @@ protected:
   LayerImpl* root_scroll_layer_;
   LayerImpl* currently_scrolling_layer_;
 
+  typedef base::hash_map<int, LayerImpl*> LayerIdMap;
+  LayerIdMap layer_id_map_;
+
   // Persisted state
   int scrolling_layer_id_from_previous_tree_;
 

cc/quad_culler_unittest.cc

@@ -43,12 +43,13 @@ class QuadCullerTest : public testing::Test
 public:
     QuadCullerTest()
         : m_hostImpl(&m_proxy)
+        , m_layerId(1)
     {
     }
 
     scoped_ptr<TiledLayerImpl> makeLayer(TiledLayerImpl* parent, const gfx::Transform& drawTransform, const gfx::Rect& layerRect, float opacity, bool opaque, const gfx::Rect& layerOpaqueRect, std::vector<LayerImpl*>& surfaceLayerList)
     {
-        scoped_ptr<TiledLayerImpl> layer = TiledLayerImpl::create(m_hostImpl.activeTree(), 1);
+        scoped_ptr<TiledLayerImpl> layer = TiledLayerImpl::create(m_hostImpl.activeTree(), m_layerId++);
         scoped_ptr<LayerTilingData> tiler = LayerTilingData::create(gfx::Size(100, 100), LayerTilingData::NoBorderTexels);
         tiler->setBounds(layerRect.size());
         layer->setTilingData(*tiler);
@@ -96,6 +97,7 @@ public:
 protected:
     FakeImplProxy m_proxy;
     FakeLayerTreeHostImpl m_hostImpl;
+    int m_layerId;
 };
 
 #define DECLARE_AND_INITIALIZE_TEST_QUADS               \

cc/render_surface_unittest.cc

@@ -59,7 +59,7 @@ TEST(RenderSurfaceTest, verifySurfaceChangesAreTrackedProperly)
     EXECUTE_AND_VERIFY_SURFACE_DID_NOT_CHANGE(renderSurface->setClipRect(testRect));
     EXECUTE_AND_VERIFY_SURFACE_DID_NOT_CHANGE(renderSurface->setContentRect(testRect));
 
-    scoped_ptr<LayerImpl> dummyMask = LayerImpl::create(hostImpl.activeTree(), 1);
+    scoped_ptr<LayerImpl> dummyMask = LayerImpl::create(hostImpl.activeTree(), 2);
     gfx::Transform dummyMatrix;
     dummyMatrix.Translate(1.0, 2.0);
 

cc/tree_synchronizer.cc

@@ -37,8 +37,8 @@ void TreeSynchronizer::collectExistingLayerImplRecursive(ScopedPtrLayerImplMap&
     for (size_t i = 0; i < children.size(); ++i)
         collectExistingLayerImplRecursive(oldLayers, children.take(i));
 
-    collectExistingLayerImplRecursive(oldLayers, layerImpl->m_maskLayer.Pass());
+    collectExistingLayerImplRecursive(oldLayers, layerImpl->takeMaskLayer());
-    collectExistingLayerImplRecursive(oldLayers, layerImpl->m_replicaLayer.Pass());
+    collectExistingLayerImplRecursive(oldLayers, layerImpl->takeReplicaLayer());
 
     int id = layerImpl->id();
     oldLayers.set(id, layerImpl.Pass());