Bug 1096328 - Remove nativeOwnership from Bindings.conf, make the WrapObject signature for non-refcounted objects the same as for refcounted objects. r=bz.

* * *
[mq]: owned_fold.patch

--HG--
extra : rebase_source : 1eec318046dae59e740ee5409a97c7cbfd88c9b1

dom/bindings/BindingUtils.h

@@ -1064,11 +1064,9 @@ WrapNewBindingNonWrapperCachedOwnedObject(JSContext* cx,
       ac.emplace(cx, scope);
     }
 
-    bool tookOwnership = false;
     MOZ_ASSERT(js::IsObjectInContextCompartment(scope, cx));
-    obj = value->WrapObject(cx, &tookOwnership);
-    MOZ_ASSERT_IF(obj, tookOwnership);
-    if (tookOwnership) {
+    obj = value->WrapObject(cx);
+    if (obj) {
       value.forget();
     }
   }
@@ -2523,11 +2521,6 @@ HasConstructor(JSObject* obj)
 }
  #endif
  
-inline void
-MustInheritFromNonRefcountedDOMObject(NonRefcountedDOMObject*)
-{
-}
-
 template<class T, bool hasCallback=NativeHasMember<T>::JSBindingFinalized>
 struct JSBindingFinalized
 {
@@ -2755,6 +2748,101 @@ ToSupportsIsOnPrimaryInheritanceChain(T* aObject, nsWrapperCache* aCache)
                                                                      aCache);
 }
 
+// The BindingJSObjectCreator class is supposed to be used by a caller that
+// wants to create and initialise a binding JSObject. After initialisation has
+// been successfully completed it should call ForgetObject().
+// The BindingJSObjectCreator object will root the JSObject until ForgetObject()
+// is called on it. If the native object for the binding is refcounted it will
+// also hold a strong reference to it, that reference is transferred to the
+// JSObject (which holds the native in a slot) when ForgetObject() is called. If
+// the BindingJSObjectCreator object is destroyed and ForgetObject() was never
+// called on it then the JSObject's slot holding the native will be set to
+// undefined, and for a refcounted native the strong reference will be released.
+template<class T>
+class MOZ_STACK_CLASS BindingJSObjectCreator
+{
+public:
+  explicit BindingJSObjectCreator(JSContext* aCx)
+    : mObject(aCx)
+  {
+  }
+
+  ~BindingJSObjectCreator()
+  {
+    if (mObject) {
+      js::SetReservedOrProxyPrivateSlot(mObject, DOM_OBJECT_SLOT,
+                                        JS::UndefinedValue());
+    }
+  }
+
+  JS::Rooted<JSObject*>&
+  CreateProxyObject(JSContext* aCx, const js::Class* aClass,
+                    const DOMProxyHandler* aHandler,
+                    JS::Handle<JSObject*> aProto,
+                    JS::Handle<JSObject*> aParent, T* aNative)
+  {
+    js::ProxyOptions options;
+    options.setClass(aClass);
+    JS::Rooted<JS::Value> proxyPrivateVal(aCx, JS::PrivateValue(aNative));
+    mObject = js::NewProxyObject(aCx, aHandler, proxyPrivateVal, aProto,
+                                 aParent, options);
+    if (mObject) {
+      mNative = aNative;
+    }
+    return mObject;
+  }
+
+  JS::Rooted<JSObject*>&
+  CreateObject(JSContext* aCx, const JSClass* aClass,
+               JS::Handle<JSObject*> aProto, JS::Handle<JSObject*> aParent,
+               T* aNative)
+  {
+    mObject = JS_NewObject(aCx, aClass, aProto, aParent);
+    if (mObject) {
+      js::SetReservedSlot(mObject, DOM_OBJECT_SLOT, JS::PrivateValue(aNative));
+      mNative = aNative;
+    }
+    return mObject;
+  }
+
+  JSObject*
+  ForgetObject()
+  {
+    void* dummy;
+    mNative.forget(&dummy);
+
+    JSObject* obj = mObject;
+    mObject = nullptr;
+    return obj;
+  }
+
+private:
+  struct OwnedNative
+  {
+    // Make sure the native objects inherit from NonRefcountedDOMObject so
+    // that we log their ctor and dtor.
+    static_assert(IsBaseOf<NonRefcountedDOMObject, T>::value,
+                  "Non-refcounted objects with DOM bindings should inherit "
+                  "from NonRefcountedDOMObject.");
+
+    OwnedNative&
+    operator=(T* aNative)
+    {
+      return *this;
+    }
+
+    // This signature sucks, but it's the only one that will make a nsRefPtr
+    // just forget about its pointer without warning.
+    void
+    forget(void**)
+    {
+    }
+  };
+
+  JS::Rooted<JSObject*> mObject;
+  typename Conditional<IsRefcounted<T>::value, nsRefPtr<T>, OwnedNative>::Type mNative;
+};
+
 template<class T,
          bool isISupports=IsBaseOf<nsISupports, T>::value>
 class DeferredFinalizer

dom/bindings/Codegen.py

@@ -3057,17 +3057,16 @@ class CGConstructorEnabled(CGAbstractMethod):
 
 
 def CreateBindingJSObject(descriptor, properties):
+    objDecl = "BindingJSObjectCreator<%s> creator(aCx);\n" % descriptor.nativeType
+
     # We don't always need to root obj, but there are a variety
     # of cases where we do, so for simplicity, just always root it.
-    objDecl = "JS::Rooted<JSObject*> obj(aCx);\n"
     if descriptor.proxy:
         create = dedent(
             """
-            JS::Rooted<JS::Value> proxyPrivateVal(aCx, JS::PrivateValue(aObject));
-            js::ProxyOptions options;
-            options.setClass(&Class.mBase);
-            obj = NewProxyObject(aCx, DOMProxyHandler::getInstance(),
-                                 proxyPrivateVal, proto, global, options);
+            const JS::Rooted<JSObject*>& obj =
+              creator.CreateProxyObject(aCx, &Class.mBase, DOMProxyHandler::getInstance(),
+                                        proto, global, aObject);
             if (!obj) {
               return nullptr;
             }
@@ -3082,25 +3081,13 @@ def CreateBindingJSObject(descriptor, properties):
     else:
         create = dedent(
             """
-            obj = JS_NewObject(aCx, Class.ToJSClass(), proto, global);
+            const JS::Rooted<JSObject*>& obj =
+              creator.CreateObject(aCx, Class.ToJSClass(), proto, global, aObject);
             if (!obj) {
               return nullptr;
             }
-
-            js::SetReservedSlot(obj, DOM_OBJECT_SLOT, PRIVATE_TO_JSVAL(aObject));
             """)
-    create = objDecl + create
-
-    if descriptor.nativeOwnership == 'refcounted':
-        create += "NS_ADDREF(aObject);\n"
-    else:
-        create += dedent("""
-            // Make sure the native objects inherit from NonRefcountedDOMObject so that we
-            // log their ctor and dtor.
-            MustInheritFromNonRefcountedDOMObject(aObject);
-            *aTookOwnership = true;
-            """)
-    return create
+    return objDecl + create
 
 
 def InitUnforgeablePropertiesOnObject(descriptor, obj, properties, failureReturnValue=""):
@@ -3267,7 +3254,7 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
 
             aCache->SetWrapper(obj);
             $*{slots}
-            return obj;
+            return creator.ForgetObject();
             """,
             assertion=AssertInheritanceChain(self.descriptor),
             createObject=CreateBindingJSObject(self.descriptor, self.properties),
@@ -3300,8 +3287,6 @@ class CGWrapNonWrapperCacheMethod(CGAbstractMethod):
         assert descriptor.interface.hasInterfacePrototypeObject()
         args = [Argument('JSContext*', 'aCx'),
                 Argument(descriptor.nativeType + '*', 'aObject')]
-        if descriptor.nativeOwnership == 'owned':
-            args.append(Argument('bool*', 'aTookOwnership'))
         CGAbstractMethod.__init__(self, descriptor, 'Wrap', 'JSObject*', args)
         self.properties = properties
 
@@ -3321,7 +3306,7 @@ class CGWrapNonWrapperCacheMethod(CGAbstractMethod):
             $*{unforgeable}
 
             $*{slots}
-            return obj;
+            return creator.ForgetObject();
             """,
             assertions=AssertInheritanceChain(self.descriptor),
             createObject=CreateBindingJSObject(self.descriptor, self.properties),

dom/canvas/TextMetrics.h

@@ -30,9 +30,9 @@ public:
     return width;
   }
 
-  JSObject* WrapObject(JSContext* aCx, bool* aTookOwnership)
+  JSObject* WrapObject(JSContext* aCx)
   {
-    return TextMetricsBinding::Wrap(aCx, this, aTookOwnership);
+    return TextMetricsBinding::Wrap(aCx, this);
   }
 
 private:

dom/encoding/TextDecoder.h

@@ -49,9 +49,9 @@ public:
     MOZ_COUNT_DTOR(TextDecoder);
   }
 
-  JSObject* WrapObject(JSContext* aCx, bool* aTookOwnership)
+  JSObject* WrapObject(JSContext* aCx)
   {
-    return TextDecoderBinding::Wrap(aCx, this, aTookOwnership);
+    return TextDecoderBinding::Wrap(aCx, this);
   }
 
   /**

dom/encoding/TextEncoder.h

@@ -42,9 +42,9 @@ public:
   ~TextEncoder()
   {}
 
-  JSObject* WrapObject(JSContext* aCx, bool* aTookOwnership)
+  JSObject* WrapObject(JSContext* aCx)
   {
-    return TextEncoderBinding::Wrap(aCx, this, aTookOwnership);
+    return TextEncoderBinding::Wrap(aCx, this);
   }
 
 protected:

dom/xslt/xpath/XPathExpression.h

@@ -34,9 +34,9 @@ public:
                     nsIDocument *aDocument);
     ~XPathExpression();
 
-    JSObject* WrapObject(JSContext* aCx, bool* aTookOwnership)
+    JSObject* WrapObject(JSContext* aCx)
     {
-        return XPathExpressionBinding::Wrap(aCx, this, aTookOwnership);
+        return XPathExpressionBinding::Wrap(aCx, this);
     }
 
     already_AddRefed<XPathResult>