Bug 970747 - 3/6 - Make ContainerLayer::RemoveChild always perform checks and return bool - r=mattwoodrow

gfx/layers/Layers.cpp

@@ -770,13 +770,17 @@ ContainerLayer::InsertAfter(Layer* aChild, Layer* aAfter)
   return true;
 }
 
-void
+bool
 ContainerLayer::RemoveChild(Layer *aChild)
 {
-  NS_ASSERTION(aChild->Manager() == Manager(),
-               "Child has wrong manager");
-  NS_ASSERTION(aChild->GetParent() == this,
-               "aChild not our child");
+  if (aChild->Manager() != Manager()) {
+    NS_ERROR("Child has wrong manager");
+    return false;
+  }
+  if (aChild->GetParent() != this) {
+    NS_ERROR("aChild not our child");
+    return false;
+  }
 
   Layer* prev = aChild->GetPrevSibling();
   Layer* next = aChild->GetNextSibling();
@@ -797,6 +801,7 @@ ContainerLayer::RemoveChild(Layer *aChild)
 
   this->DidRemoveChild(aChild);
   NS_RELEASE(aChild);
+  return true;
 }
 
 

gfx/layers/Layers.h

@@ -1567,7 +1567,7 @@ public:
    * Remove aChild from the child list of this container. aChild must
    * be a child of this container.
    */
-  virtual void RemoveChild(Layer* aChild);
+  virtual bool RemoveChild(Layer* aChild);
   /**
    * CONSTRUCTION PHASE ONLY
    * Reposition aChild from the child list of this container. aChild must
@@ -1965,8 +1965,8 @@ private:
   virtual bool InsertAfter(Layer* aChild, Layer* aAfter) MOZ_OVERRIDE
   { MOZ_CRASH(); return false; }
 
-  virtual void RemoveChild(Layer* aChild)
-  { MOZ_CRASH(); }
+  virtual bool RemoveChild(Layer* aChild)
+  { MOZ_CRASH(); return false; }
 
   virtual void RepositionChild(Layer* aChild, Layer* aAfter)
   { MOZ_CRASH(); }

gfx/layers/basic/BasicContainerLayer.h

@@ -43,11 +43,13 @@ public:
     return ContainerLayer::InsertAfter(aChild, aAfter);
   }
 
-  virtual void RemoveChild(Layer* aChild)
+  virtual bool RemoveChild(Layer* aChild)
   { 
-    NS_ASSERTION(BasicManager()->InConstruction(),
-                 "Can only set properties in construction phase");
-    ContainerLayer::RemoveChild(aChild);
+    if (!BasicManager()->InConstruction()) {
+      NS_ERROR("Can only set properties in construction phase");
+      return false;
+    }
+    return ContainerLayer::RemoveChild(aChild);
   }
 
   virtual void RepositionChild(Layer* aChild, Layer* aAfter)

gfx/layers/client/ClientContainerLayer.h

@@ -107,13 +107,19 @@ public:
     return true;
   }
 
-  virtual void RemoveChild(Layer* aChild) MOZ_OVERRIDE
-  { 
-    NS_ASSERTION(ClientManager()->InConstruction(),
-                 "Can only set properties in construction phase");
-    ClientManager()->AsShadowForwarder()->RemoveChild(ClientManager()->Hold(this),
-                                                      ClientManager()->Hold(aChild));
-    ContainerLayer::RemoveChild(aChild);
+  virtual bool RemoveChild(Layer* aChild) MOZ_OVERRIDE
+  {
+    if (!ClientManager()->InConstruction()) {
+      NS_ERROR("Can only set properties in construction phase");
+      return false;
+    }
+    // hold on to aChild before we remove it!
+    ShadowableLayer *heldChild = ClientManager()->Hold(aChild);
+    if (!ContainerLayer::RemoveChild(aChild)) {
+      return false;
+    }
+    ClientManager()->AsShadowForwarder()->RemoveChild(ClientManager()->Hold(this), heldChild);
+    return true;
   }
 
   virtual void RepositionChild(Layer* aChild, Layer* aAfter) MOZ_OVERRIDE

gfx/tests/gtest/TestAsyncPanZoomController.cpp

@@ -85,7 +85,7 @@ class TestAPZCContainerLayer : public ContainerLayer {
     TestAPZCContainerLayer()
       : ContainerLayer(nullptr, nullptr)
     {}
-  void RemoveChild(Layer* aChild) {}
+  bool RemoveChild(Layer* aChild) { return true; }
   bool InsertAfter(Layer* aChild, Layer* aAfter) { return true; }
   void ComputeEffectiveTransforms(const Matrix4x4& aTransformToSurface) {}
   void RepositionChild(Layer* aChild, Layer* aAfter) {}