From a75b9a2ab0cdf1d5d786127dacb47835006632ee Mon Sep 17 00:00:00 2001
From: Benoit Jacob <bjacob@mozilla.com>
Date: Fri, 21 Feb 2014 16:50:25 -0500
Subject: [PATCH] Bug 970747 - 3/6 - Make ContainerLayer::RemoveChild always
 perform checks and return bool - r=mattwoodrow

---
 gfx/layers/Layers.cpp                         | 15 +++++++++-----
 gfx/layers/Layers.h                           |  6 +++---
 gfx/layers/basic/BasicContainerLayer.h        | 10 ++++++----
 gfx/layers/client/ClientContainerLayer.h      | 20 ++++++++++++-------
 .../gtest/TestAsyncPanZoomController.cpp      |  2 +-
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/gfx/layers/Layers.cpp b/gfx/layers/Layers.cpp
index 535ec2ecb16a..806eaf58f7eb 100644
--- a/gfx/layers/Layers.cpp
+++ b/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;
 }
 
 
diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h
index e1c0b32eed82..b51d08a48f5f 100644
--- a/gfx/layers/Layers.h
+++ b/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(); }
diff --git a/gfx/layers/basic/BasicContainerLayer.h b/gfx/layers/basic/BasicContainerLayer.h
index 70e261ebe167..fe7034db8fbf 100644
--- a/gfx/layers/basic/BasicContainerLayer.h
+++ b/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)
diff --git a/gfx/layers/client/ClientContainerLayer.h b/gfx/layers/client/ClientContainerLayer.h
index 8180be70fd9a..bc93da530834 100644
--- a/gfx/layers/client/ClientContainerLayer.h
+++ b/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
diff --git a/gfx/tests/gtest/TestAsyncPanZoomController.cpp b/gfx/tests/gtest/TestAsyncPanZoomController.cpp
index 9bfeb99f2d69..9c64ec39b1fc 100644
--- a/gfx/tests/gtest/TestAsyncPanZoomController.cpp
+++ b/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) {}