From 1a1cc411ffa1bc1614bd9747d6a3190cda3a342f Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Mon, 23 Jun 2014 14:49:08 -0400 Subject: [PATCH] Bug 1028588 - Fix dangerous public destructors in dom/bindings - r=bz --- dom/bindings/CallbackObject.h | 10 +++++----- dom/bindings/Codegen.py | 10 ++++++---- dom/bindings/Exceptions.cpp | 5 ++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h index 2663cc65613..264e8a91b87 100644 --- a/dom/bindings/CallbackObject.h +++ b/dom/bindings/CallbackObject.h @@ -55,11 +55,6 @@ public: Init(aCallback, aIncumbentGlobal); } - virtual ~CallbackObject() - { - DropJSObjects(); - } - JS::Handle Callback() const { JS::ExposeObjectToActiveJS(mCallback); @@ -103,6 +98,11 @@ public: } protected: + virtual ~CallbackObject() + { + DropJSObjects(); + } + explicit CallbackObject(CallbackObject* aCallbackObject) { Init(aCallbackObject->mCallback, aCallbackObject->mIncumbentGlobal); diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 467e3100490..a42d7cf3d66 100644 --- a/dom/bindings/Codegen.py +++ b/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))" % diff --git a/dom/bindings/Exceptions.cpp b/dom/bindings/Exceptions.cpp index 50d77d6a8bc..c262a151435 100644 --- a/dom/bindings/Exceptions.cpp +++ b/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 CreateStack(JSContext* aCx, int32_t aMaxDepth = -1); @@ -268,6 +269,8 @@ public: nsIStackFrame* aCaller); private: + virtual ~JSStackFrame(); + bool IsJSFrame() const { return mLanguage == nsIProgrammingLanguage::JAVASCRIPT; }