Bug 861022 part 1. Root the non-globals in WebIDL prototype and interface object setup. r=peterv,terrence
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 01 May 2013 23:44:11 -0400
changeset 141521 60e671f364b941c704282cf191892dc65758bbe0
parent 141520 a6c9a09469c606aecbf9a07b6befa7cf27dc740c
child 141522 a606d921d45aae01f97b86bc6932a6839413c6c4
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, terrence
bugs861022
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 861022 part 1. Root the non-globals in WebIDL prototype and interface object setup. r=peterv,terrence
content/base/src/nsObjectLoadingContent.cpp
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/DOMJSClass.h
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -2974,17 +2974,17 @@ nsObjectLoadingContent::SetupProtoChain(
   if (!pi_obj) {
     // Didn't get a plugin instance JSObject, nothing we can do then.
     return;
   }
 
   // If we got an xpconnect-wrapped plugin object, set obj's
   // prototype's prototype to the scriptable plugin.
 
-  JSObject *my_proto =
+  JS::Handle<JSObject*> my_proto =
     GetDOMClass(aObject)->mGetProto(aCx, JS_GetGlobalForObject(aCx, aObject));
   MOZ_ASSERT(my_proto);
 
   // Set 'this.__proto__' to pi
   if (!::JS_SetPrototype(aCx, aObject, pi_obj)) {
     return;
   }
 
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -289,17 +289,17 @@ CreateConstructor(JSContext* cx, JSObjec
   js::SetFunctionNativeReserved(constructor,
                                 CONSTRUCTOR_NATIVE_HOLDER_RESERVED_SLOT,
                                 js::PrivateValue(const_cast<JSNativeHolder*>(nativeHolder)));
   return constructor;
 }
 
 static bool
 DefineConstructor(JSContext* cx, JSObject* global, const char* name,
-                  JSObject* constructor)
+                  JS::Handle<JSObject*> constructor)
 {
   JSBool alreadyDefined;
   if (!JS_AlreadyHasOwnProperty(cx, global, name, &alreadyDefined)) {
     return false;
   }
 
   // This is Enumerable: False per spec.
   return alreadyDefined ||
@@ -307,22 +307,22 @@ DefineConstructor(JSContext* cx, JSObjec
                            nullptr, nullptr, 0);
 }
 
 static JSObject*
 CreateInterfaceObject(JSContext* cx, JSObject* global,
                       JSClass* constructorClass,
                       const JSNativeHolder* constructorNative,
                       unsigned ctorNargs, const NamedConstructor* namedConstructors,
-                      JSObject* proto,
+                      JS::Handle<JSObject*> proto,
                       const NativeProperties* properties,
                       const NativeProperties* chromeOnlyProperties,
                       const char* name)
 {
-  JSObject* constructor;
+  JS::Rooted<JSObject*> constructor(cx);
   bool isCallbackInterface = constructorClass == js::Jsvalify(&js::ObjectClass);
   if (constructorClass) {
     JSObject* constructorProto;
     if (isCallbackInterface) {
       constructorProto = JS_GetObjectPrototype(cx, global);
     } else {
       constructorProto = JS_GetFunctionPrototype(cx, global);
     }
@@ -337,32 +337,33 @@ CreateInterfaceObject(JSContext* cx, JSO
   }
   if (!constructor) {
     return NULL;
   }
 
   if (constructorClass && !isCallbackInterface) {
     // Have to shadow Function.prototype.toString, since that throws
     // on things that are not js::FunctionClass.
-    JSFunction* toString = js::DefineFunctionWithReserved(cx, constructor,
-                                                          "toString",
-                                                          InterfaceObjectToString,
-                                                          0, 0);
+    JS::Rooted<JSFunction*> toString(cx,
+      js::DefineFunctionWithReserved(cx, constructor,
+                                     "toString",
+                                     InterfaceObjectToString,
+                                     0, 0));
     if (!toString) {
       return NULL;
     }
 
-    JSObject* toStringObj = JS_GetFunctionObject(toString);
-    js::SetFunctionNativeReserved(toStringObj, TOSTRING_CLASS_RESERVED_SLOT,
-                                  PRIVATE_TO_JSVAL(constructorClass));
-
     JSString *str = ::JS_InternString(cx, name);
     if (!str) {
       return NULL;
     }
+    JSObject* toStringObj = JS_GetFunctionObject(toString);
+    js::SetFunctionNativeReserved(toStringObj, TOSTRING_CLASS_RESERVED_SLOT,
+                                  PRIVATE_TO_JSVAL(constructorClass));
+
     js::SetFunctionNativeReserved(toStringObj, TOSTRING_NAME_RESERVED_SLOT,
                                   STRING_TO_JSVAL(str));
 
     if (!JS_DefineProperty(cx, constructor, "length", JS::Int32Value(ctorNargs),
                            nullptr, nullptr, JSPROP_READONLY | JSPROP_PERMANENT)) {
       return NULL;
     }
   }
@@ -408,20 +409,20 @@ CreateInterfaceObject(JSContext* cx, JSO
 
   if (!DefineConstructor(cx, global, name, constructor)) {
     return nullptr;
   }
 
   if (namedConstructors) {
     int namedConstructorSlot = DOM_INTERFACE_SLOTS_BASE;
     while (namedConstructors->mName) {
-      JSObject* namedConstructor = CreateConstructor(cx, global,
-                                                     namedConstructors->mName,
-                                                     &namedConstructors->mHolder,
-                                                     namedConstructors->mNargs);
+      JS::Rooted<JSObject*> namedConstructor(cx,
+        CreateConstructor(cx, global, namedConstructors->mName,
+                          &namedConstructors->mHolder,
+                          namedConstructors->mNargs));
       if (!namedConstructor ||
           !JS_DefineProperty(cx, namedConstructor, "prototype",
                              JS::ObjectValue(*proto), JS_PropertyStub,
                              JS_StrictPropertyStub,
                              JSPROP_PERMANENT | JSPROP_READONLY) ||
           !DefineConstructor(cx, global, namedConstructors->mName,
                              namedConstructor)) {
         return nullptr;
@@ -448,22 +449,23 @@ DefineWebIDLBindingPropertiesOnXPCProto(
     return false;
   }
 
   return true;
 }
 
 static JSObject*
 CreateInterfacePrototypeObject(JSContext* cx, JSObject* global,
-                               JSObject* parentProto, JSClass* protoClass,
+                               JS::Handle<JSObject*> parentProto,
+                               JSClass* protoClass,
                                const NativeProperties* properties,
                                const NativeProperties* chromeOnlyProperties)
 {
-  JSObject* ourProto = JS_NewObjectWithUniqueType(cx, protoClass, parentProto,
-                                                  global);
+  JS::Rooted<JSObject*> ourProto(cx,
+    JS_NewObjectWithUniqueType(cx, protoClass, parentProto, global));
   if (!ourProto) {
     return NULL;
   }
 
   if (properties) {
     if (properties->methods &&
         !DefinePrefable(cx, ourProto, properties->methods)) {
       return nullptr;
@@ -496,17 +498,18 @@ CreateInterfacePrototypeObject(JSContext
       return nullptr;
     }
   }
 
   return ourProto;
 }
 
 void
-CreateInterfaceObjects(JSContext* cx, JSObject* global, JSObject* protoProto,
+CreateInterfaceObjects(JSContext* cx, JSObject* global,
+                       JS::Handle<JSObject*> protoProto,
                        JSClass* protoClass, JSObject** protoCache,
                        JSClass* constructorClass, const JSNativeHolder* constructor,
                        unsigned ctorNargs, const NamedConstructor* namedConstructors,
                        JSObject** constructorCache, const DOMClass* domClass,
                        const NativeProperties* properties,
                        const NativeProperties* chromeOnlyProperties,
                        const char* name)
 {
@@ -530,31 +533,32 @@ CreateInterfaceObjects(JSContext* cx, JS
   MOZ_ASSERT(!constructorClass || !constructor);
   MOZ_ASSERT(!protoClass == !protoCache,
              "If, and only if, there is an interface prototype object we need "
              "to cache it");
   MOZ_ASSERT(!(constructorClass || constructor) == !constructorCache,
              "If, and only if, there is an interface object we need to cache "
              "it");
 
-  JSObject* proto;
+  JS::Rooted<JSObject*> proto(cx);
   if (protoClass) {
-    proto = CreateInterfacePrototypeObject(cx, global, protoProto, protoClass,
-                                           properties, chromeOnlyProperties);
+    proto =
+      CreateInterfacePrototypeObject(cx, global, protoProto, protoClass,
+                                     properties, chromeOnlyProperties);
     if (!proto) {
       return;
     }
 
     js::SetReservedSlot(proto, DOM_PROTO_INSTANCE_CLASS_SLOT,
                         JS::PrivateValue(const_cast<DOMClass*>(domClass)));
 
     *protoCache = proto;
   }
   else {
-    proto = NULL;
+    MOZ_ASSERT(!proto);
   }
 
   JSObject* interface;
   if (constructorClass || constructor) {
     interface = CreateInterfaceObject(cx, global, constructorClass, constructor,
                                       ctorNargs, namedConstructors, proto,
                                       properties, chromeOnlyProperties, name);
     if (!interface) {
@@ -1465,17 +1469,17 @@ ReparentWrapper(JSContext* aCx, JS::Hand
 
   JSAutoCompartment newAc(aCx, newParent);
 
   // First we clone the reflector. We get a copy of its properties and clone its
   // expando chain. The only part that is dangerous here is that if we have to
   // return early we must avoid ending up with two reflectors pointing to the
   // same native. Other than that, the objects we create will just go away.
 
-  JSObject *proto =
+  JS::Handle<JSObject*> proto =
     (domClass->mGetProto)(aCx,
                           js::GetGlobalForObjectCrossCompartment(newParent));
   if (!proto) {
     return NS_ERROR_FAILURE;
   }
 
   JSObject *newobj = JS_CloneObject(aCx, aObj, proto, newParent);
   if (!newobj) {
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -326,17 +326,18 @@ struct NamedConstructor
  *                  object is being created in non-chrome compartment.
  *
  * At least one of protoClass, constructorClass or constructor should be
  * non-null. If constructorClass or constructor are non-null, the resulting
  * interface object will be defined on the given global with property name
  * |name|, which must also be non-null.
  */
 void
-CreateInterfaceObjects(JSContext* cx, JSObject* global, JSObject* protoProto,
+CreateInterfaceObjects(JSContext* cx, JSObject* global,
+                       JS::Handle<JSObject*> protoProto,
                        JSClass* protoClass, JSObject** protoCache,
                        JSClass* constructorClass, const JSNativeHolder* constructor,
                        unsigned ctorNargs, const NamedConstructor* namedConstructors,
                        JSObject** constructorCache, const DOMClass* domClass,
                        const NativeProperties* regularProperties,
                        const NativeProperties* chromeOnlyProperties,
                        const char* name);
 
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -1674,21 +1674,23 @@ class CGCreateInterfaceObjectsMethod(CGA
     def __init__(self, descriptor, properties):
         args = [Argument('JSContext*', 'aCx'), Argument('JSObject*', 'aGlobal'),
                 Argument('JSObject**', 'protoAndIfaceArray')]
         CGAbstractMethod.__init__(self, descriptor, 'CreateInterfaceObjects', 'void', args)
         self.properties = properties
     def definition_body(self):
         protoChain = self.descriptor.prototypeChain
         if len(protoChain) == 1:
-            getParentProto = "JS_GetObjectPrototype(aCx, aGlobal)"
+            getParentProto = "aCx, JS_GetObjectPrototype(aCx, aGlobal)"
+            parentProtoType = "Rooted"
         else:
             parentProtoName = self.descriptor.prototypeChain[-2]
             getParentProto = ("%s::GetProtoObject(aCx, aGlobal)" %
                               toBindingNamespace(parentProtoName))
+            parentProtoType = "Handle"
 
         needInterfaceObject = self.descriptor.interface.hasInterfaceObject()
         needInterfacePrototypeObject = self.descriptor.interface.hasInterfacePrototypeObject()
 
         # if we don't need to create anything, why are we generating this?
         assert needInterfaceObject or needInterfacePrototypeObject
 
         idsToInit = []
@@ -1746,20 +1748,20 @@ if (!unforgeableHolder) {
                                                                    "unforgeableHolder",
                                                                    self.properties)
             createUnforgeableHolder = CGList([createUnforgeableHolder,
                                               defineUnforgeables],
                                              "\n")
         else:
             createUnforgeableHolder = None
 
-        getParentProto = ("JSObject* parentProto = %s;\n" +
+        getParentProto = ("JS::%s<JSObject*> parentProto(%s);\n" +
                           "if (!parentProto) {\n" +
                           "  return;\n" +
-                          "}\n") % getParentProto
+                          "}\n") % (parentProtoType, getParentProto)
 
         if (needInterfaceObject and
             self.descriptor.needsConstructHookHolder()):
             constructHookHolder = "&" + CONSTRUCT_HOOK_NAME + "_holder"
         else:
             constructHookHolder = "nullptr"
         if self.descriptor.interface.ctor():
             constructArgs = methodLength(self.descriptor.interface.ctor())
@@ -1837,35 +1839,34 @@ if (!unforgeableHolder) {
 class CGGetPerInterfaceObject(CGAbstractMethod):
     """
     A method for getting a per-interface object (a prototype object or interface
     constructor object).
     """
     def __init__(self, descriptor, name, idPrefix=""):
         args = [Argument('JSContext*', 'aCx'), Argument('JSObject*', 'aGlobal')]
         CGAbstractMethod.__init__(self, descriptor, name,
-                                  'JSObject*', args, inline=True)
+                                  'JS::Handle<JSObject*>', args, inline=True)
         self.id = idPrefix + "id::" + self.descriptor.name
     def definition_body(self):
-        return """
+        return ("""
 
   /* Make sure our global is sane.  Hopefully we can remove this sometime */
   if (!(js::GetObjectClass(aGlobal)->flags & JSCLASS_DOM_GLOBAL)) {
-    return NULL;
+    return JS::NullPtr();
   }
   /* Check to see whether the interface objects are already installed */
   JSObject** protoAndIfaceArray = GetProtoAndIfaceArray(aGlobal);
-  JSObject* cachedObject = protoAndIfaceArray[%s];
-  if (!cachedObject) {
+  if (!protoAndIfaceArray[%s]) {
     CreateInterfaceObjects(aCx, aGlobal, protoAndIfaceArray);
-    cachedObject = protoAndIfaceArray[%s];
   }
 
-  /* cachedObject might _still_ be null, but that's OK */
-  return cachedObject;""" % (self.id, self.id)
+  /* The object might _still_ be null, but that's OK */
+  return JS::Handle<JSObject*>::fromMarkedLocation(&protoAndIfaceArray[%s]);""" %
+                (self.id, self.id))
 
 class CGGetProtoObjectMethod(CGGetPerInterfaceObject):
     """
     A method for getting the interface prototype object.
     """
     def __init__(self, descriptor):
         CGGetPerInterfaceObject.__init__(self, descriptor, "GetProtoObject",
                                          "prototypes::")
@@ -2108,17 +2109,17 @@ class CGWrapWithCacheMethod(CGAbstractMe
     JSObject* obj = aCache->GetWrapper();
     if (obj) {
       return obj;
     }
   }
 
   JSAutoCompartment ac(aCx, parent);
   JSObject* global = JS_GetGlobalForObject(aCx, parent);
-  JSObject* proto = GetProtoObject(aCx, global);
+  JS::Handle<JSObject*> proto = GetProtoObject(aCx, global);
   if (!proto) {
     return NULL;
   }
 
 %s
 %s
   aCache->SetWrapper(obj);
 
@@ -2155,17 +2156,17 @@ class CGWrapNonWrapperCacheMethod(CGAbst
         if descriptor.nativeOwnership == 'owned':
             args.append(Argument('bool*', 'aTookOwnership'))
         CGAbstractMethod.__init__(self, descriptor, 'Wrap', 'JSObject*', args)
         self.properties = properties
 
     def definition_body(self):
         return """%s
   JSObject* global = JS_GetGlobalForObject(aCx, aScope);
-  JSObject* proto = GetProtoObject(aCx, global);
+  JS::Handle<JSObject*> proto = GetProtoObject(aCx, global);
   if (!proto) {
     return NULL;
   }
 
 %s
 %s
   return obj;""" % (AssertInheritanceChain(self.descriptor),
                     CreateBindingJSObject(self.descriptor, "global"),
--- a/dom/bindings/DOMJSClass.h
+++ b/dom/bindings/DOMJSClass.h
@@ -152,17 +152,23 @@ struct NativePropertyHooks
 
 enum DOMObjectType {
   eInstance,
   eInterface,
   eInterfacePrototype
 };
 
 typedef JSObject* (*ParentGetter)(JSContext* aCx, JS::Handle<JSObject*> aObj);
-typedef JSObject* (*ProtoGetter)(JSContext* aCx, JSObject* aGlobal);
+/**
+ * Returns a handle to the relevent WebIDL prototype object for the given global
+ * (which may be a handle to null on out of memory).  Once allocated, the
+ * prototype object is guaranteed to exist as long as the global does, since the
+ * global traces its array of WebIDL prototypes and constructors.
+ */
+typedef JS::Handle<JSObject*> (*ProtoGetter)(JSContext* aCx, JSObject* aGlobal);
 
 struct DOMClass
 {
   // A list of interfaces that this object implements, in order of decreasing
   // derivedness.
   const prototypes::ID mInterfaceChain[MAX_PROTOTYPE_CHAIN_LENGTH];
 
   // We store the DOM object in reserved slot with index DOM_OBJECT_SLOT or in