Bug 1028588 - Fix dangerous public destructors in dom/bindings - r=bz

dom/bindings/CallbackObject.h

@@ -55,11 +55,6 @@ public:
     Init(aCallback, aIncumbentGlobal);
   }
 
-  virtual ~CallbackObject()
-  {
-    DropJSObjects();
-  }
-
   JS::Handle<JSObject*> Callback() const
   {
     JS::ExposeObjectToActiveJS(mCallback);
@@ -103,6 +98,11 @@ public:
   }
 
 protected:
+  virtual ~CallbackObject()
+  {
+    DropJSObjects();
+  }
+
   explicit CallbackObject(CallbackObject* aCallbackObject)
   {
     Init(aCallbackObject->mCallback, aCallbackObject->mIncumbentGlobal);

dom/bindings/Codegen.py

@@ -12108,6 +12108,7 @@ class CGExampleClass(CGBindingImplClass):
                 bases.append(ClassBase("NonRefcountedDOMObject"))
 
         if self.refcounted:
+            destructorVisibility = "protected"
             if self.parentIface:
                 extradeclarations = (
                     "public:\n"
@@ -12122,6 +12123,7 @@ class CGExampleClass(CGBindingImplClass):
                     "  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(%s)\n"
                     "\n" % self.nativeLeafName(descriptor))
         else:
+            destructorVisibility = "public"
             extradeclarations = ""
 
         if descriptor.interface.hasChildInterfaces():
@@ -12133,7 +12135,7 @@ class CGExampleClass(CGBindingImplClass):
                          bases=bases,
                          constructors=[ClassConstructor([],
                                                         visibility="public")],
-                         destructor=ClassDestructor(visibility="public"),
+                         destructor=ClassDestructor(visibility=destructorVisibility),
                          methods=self.methodDecls,
                          decorators=decorators,
                          extradeclarations=extradeclarations)
@@ -12501,11 +12503,11 @@ class CGJSImplClass(CGBindingImplClass):
 
         if descriptor.interface.hasChildInterfaces():
             decorators = ""
-            # We need a public virtual destructor our subclasses can use
-            destructor = ClassDestructor(virtual=True, visibility="public")
+            # We need a protected virtual destructor our subclasses can use
+            destructor = ClassDestructor(virtual=True, visibility="protected")
         else:
             decorators = "MOZ_FINAL"
-            destructor = None
+            destructor = ClassDestructor(virtual=False, visibility="private")
 
         baseConstructors = [
             ("mImpl(new %s(aJSImplObject, /* aIncumbentGlobal = */ nullptr))" %

dom/bindings/Exceptions.cpp

@@ -191,6 +191,7 @@ public:
     mozilla::HoldJSObjects(this);
   }
 
+protected:
   ~StackDescriptionOwner()
   {
     // Make sure to set mDescription to null before calling DropJSObjects, since
@@ -203,6 +204,7 @@ public:
     mozilla::DropJSObjects(this);
   }
 
+public:
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(StackDescriptionOwner)
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(StackDescriptionOwner)
 
@@ -256,7 +258,6 @@ public:
   // JSStackFrame will never look at the stack description.  Instead,
   // it is expected to be initialized by the caller as needed.
   JSStackFrame(StackDescriptionOwner* aStackDescription, size_t aIndex);
-  virtual ~JSStackFrame();
 
   static already_AddRefed<nsIStackFrame>
   CreateStack(JSContext* aCx, int32_t aMaxDepth = -1);
@@ -268,6 +269,8 @@ public:
                            nsIStackFrame* aCaller);
 
 private:
+  virtual ~JSStackFrame();
+
   bool IsJSFrame() const {
     return mLanguage == nsIProgrammingLanguage::JAVASCRIPT;
   }