Bug 928336. Make defining unforgeable properties on objects faster by just copying them from an unforgeable holder object. r=peterv

dom/bindings/BindingUtils.h

@@ -222,7 +222,7 @@ UnwrapDOMObjectToISupports(JSObject* aObject)
     return nullptr;
   }
 
-  return UnwrapDOMObject<nsISupports>(aObject);
+  return UnwrapPossiblyNotInitializedDOMObject<nsISupports>(aObject);
 }
 
 inline bool
@@ -3036,8 +3036,10 @@ struct CreateGlobalOptions<nsGlobalWindow>
 nsresult
 RegisterDOMNames();
 
+// The return value is whatever the ProtoHandleGetter we used
+// returned.  This should be the DOM prototype for the global.
 template <class T, ProtoHandleGetter GetProto>
-bool
+JS::Handle<JSObject*>
 CreateGlobal(JSContext* aCx, T* aNative, nsWrapperCache* aCache,
              const JSClass* aClass, JS::CompartmentOptions& aOptions,
              JSPrincipals* aPrincipal, bool aInitStandardClasses,
@@ -3049,7 +3051,7 @@ CreateGlobal(JSContext* aCx, T* aNative, nsWrapperCache* aCache,
                                  JS::DontFireOnNewGlobalHook, aOptions));
   if (!aGlobal) {
     NS_WARNING("Failed to create global");
-    return false;
+    return JS::NullPtr();
   }
 
   JSAutoCompartment ac(aCx, aGlobal);
@@ -3064,7 +3066,7 @@ CreateGlobal(JSContext* aCx, T* aNative, nsWrapperCache* aCache,
                                     CreateGlobalOptions<T>::ProtoAndIfaceCacheKind);
 
     if (!CreateGlobalOptions<T>::PostCreateGlobal(aCx, aGlobal)) {
-      return false;
+      return JS::NullPtr();
     }
   }
 
@@ -3072,16 +3074,16 @@ CreateGlobal(JSContext* aCx, T* aNative, nsWrapperCache* aCache,
       !CreateGlobalOptions<T>::ForceInitStandardClassesToFalse &&
       !JS_InitStandardClasses(aCx, aGlobal)) {
     NS_WARNING("Failed to init standard classes");
-    return false;
+    return JS::NullPtr();
   }
 
   JS::Handle<JSObject*> proto = GetProto(aCx, aGlobal);
   if (!proto || !JS_SplicePrototype(aCx, aGlobal, proto)) {
     NS_WARNING("Failed to set proto");
-    return false;
+    return JS::NullPtr();
   }
 
-  return true;
+  return proto;
 }
 
 /*

dom/bindings/Codegen.py

@@ -12,7 +12,7 @@ import textwrap
 import functools
 
 from WebIDL import BuiltinTypes, IDLBuiltinType, IDLNullValue, IDLSequenceType, IDLType, IDLAttribute, IDLInterfaceMember, IDLUndefinedValue, IDLEmptySequenceValue, IDLDictionary
-from Configuration import NoSuchDescriptorError, getTypesFromDescriptor, getTypesFromDictionary, getTypesFromCallback, getAllTypes, Descriptor
+from Configuration import NoSuchDescriptorError, getTypesFromDescriptor, getTypesFromDictionary, getTypesFromCallback, getAllTypes, Descriptor, MemberIsUnforgeable
 
 AUTOGENERATED_WARNING_COMMENT = \
     "/* THIS FILE IS AUTOGENERATED - DO NOT EDIT */\n\n"
@@ -552,22 +552,6 @@ def PrototypeIDAndDepth(descriptor):
     return (prototypeID, depth)
 
 
-def MemberIsUnforgeable(member, descriptor):
-    # Note: "or" and "and" return either their LHS or RHS, not
-    # necessarily booleans.  Make sure to return a boolean from this
-    # method, because callers will compare its return value to
-    # booleans.
-    return bool((member.isAttr() or member.isMethod()) and
-                not member.isStatic() and
-                (member.isUnforgeable() or
-                 descriptor.interface.getExtendedAttribute("Unforgeable")))
-
-
-def HasUnforgeableMembers(descriptor):
-    return any(MemberIsUnforgeable(m, descriptor) for m in
-               descriptor.interface.members)
-
-
 def InterfacePrototypeObjectProtoGetter(descriptor):
     """
     Returns a tuple with two elements:
@@ -611,6 +595,8 @@ class CGPrototypeJSClass(CGThing):
     def define(self):
         prototypeID, depth = PrototypeIDAndDepth(self.descriptor)
         slotCount = "DOM_INTERFACE_PROTO_SLOTS_BASE"
+        if self.descriptor.hasUnforgeableMembers:
+            slotCount += " + 1 /* slot for the JSObject holding the unforgeable properties */"
         (protoGetter, _) = InterfacePrototypeObjectProtoGetter(self.descriptor)
         type = "eGlobalInterfacePrototype" if self.descriptor.isGlobal() else "eInterfacePrototype"
         return fill(
@@ -1524,8 +1510,9 @@ class CGAddPropertyHook(CGAbstractClassHook):
     def generate_code(self):
         assert self.descriptor.wrapperCache
         return dedent("""
-            // We don't want to preserve if we don't have a wrapper.
+            // We don't want to preserve if we don't have a wrapper, and we
-            if (self->GetWrapperPreserveColor()) {
+            // obviously can't preserve if we're not initialized.
+            if (self && self->GetWrapperPreserveColor()) {
               PreserveWrapper(self);
             }
             return true;
@@ -2634,6 +2621,55 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
         else:
             prefCache = None
 
+        if self.descriptor.hasUnforgeableMembers:
+            assert needInterfacePrototypeObject
+            # We want to use the same JSClass and prototype as the object we'll
+            # end up defining the unforgeable properties on in the end, so that
+            # we can use JS_InitializePropertiesFromCompatibleNativeObject to do
+            # a fast copy.  In the case of proxies that's null, because the
+            # expando object is a vanilla object, but in the case of other DOM
+            # objects it's whatever our class is.
+            #
+            # Also, for a global we can't use the global's class; just use
+            # nullpr and when we do the copy off the holder we'll take a slower
+            # path.  This also means that we don't need to worry about matching
+            # the prototype.
+            if self.descriptor.proxy or self.descriptor.isGlobal():
+                holderClass = "nullptr"
+                holderProto = "nullptr"
+            else:
+                holderClass = "Class.ToJSClass()"
+                holderProto = "*protoCache"
+            failureCode = dedent(
+                """
+                *protoCache = nullptr;
+                if (interfaceCache) {
+                  *interfaceCache = nullptr;
+                }
+                return;
+                """)
+            createUnforgeableHolder = CGGeneric(fill(
+                """
+                JS::Rooted<JSObject*> unforgeableHolder(aCx);
+                {
+                  JS::Rooted<JSObject*> holderProto(aCx, ${holderProto});
+                  unforgeableHolder = JS_NewObjectWithoutMetadata(aCx, ${holderClass}, holderProto);
+                  if (!unforgeableHolder) {
+                    $*{failureCode}
+                  }
+                }
+                """,
+                holderProto=holderProto,
+                holderClass=holderClass,
+                failureCode=failureCode))
+            defineUnforgeables = InitUnforgeablePropertiesOnHolder(self.descriptor,
+                                                                   self.properties,
+                                                                   failureCode)
+            createUnforgeableHolder = CGList(
+                [createUnforgeableHolder, defineUnforgeables])
+        else:
+            createUnforgeableHolder = None
+
         getParentProto = fill(
             """
             JS::${type}<JSObject*> parentProto(${getParentProto});
@@ -2699,10 +2735,12 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
 
         call = fill(
             """
+            JS::Heap<JSObject*>* protoCache = ${protoCache};
+            JS::Heap<JSObject*>* interfaceCache = ${interfaceCache};
             dom::CreateInterfaceObjects(aCx, aGlobal, parentProto,
-                                        ${protoClass}, ${protoCache},
+                                        ${protoClass}, protoCache,
                                         constructorProto, ${interfaceClass}, ${constructHookHolder}, ${constructArgs}, ${namedConstructors},
-                                        ${interfaceCache},
+                                        interfaceCache,
                                         ${properties},
                                         ${chromeProperties},
                                         ${name}, aDefineOnGlobal);
@@ -2718,9 +2756,21 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
             chromeProperties=chromeProperties,
             name='"' + self.descriptor.interface.identifier.name + '"' if needInterfaceObject else "nullptr")
 
+        if self.descriptor.hasUnforgeableMembers:
+            assert needInterfacePrototypeObject
+            setUnforgeableHolder = CGGeneric(fill(
+                """
+                if (*protoCache) {
+                  js::SetReservedSlot(*protoCache, DOM_INTERFACE_PROTO_SLOTS_BASE,
+                                      JS::ObjectValue(*unforgeableHolder));
+                }
+                """,
+                name=self.descriptor.name))
+        else:
+            setUnforgeableHolder = None
         return CGList(
             [CGGeneric(getParentProto), CGGeneric(getConstructorProto), initIds,
-             prefCache, CGGeneric(call)],
+             prefCache, CGGeneric(call), createUnforgeableHolder, setUnforgeableHolder],
             "\n").define()
 
 
@@ -3023,15 +3073,71 @@ def CreateBindingJSObject(descriptor, properties):
     return objDecl + create
 
 
-def InitUnforgeableProperties(descriptor, properties, wrapperCache):
+def InitUnforgeablePropertiesOnHolder(descriptor, properties, failureCode):
     """
-    properties is a PropertyArrays instance
+    Define the unforgeable properties on the unforgeable holder for
+    the interface represented by descriptor.
+
+    properties is a PropertyArrays instance.
+
     """
-    unforgeableAttrs = properties.unforgeableAttrs
+    assert (properties.unforgeableAttrs.hasNonChromeOnly() or
-    if not unforgeableAttrs.hasNonChromeOnly() and not unforgeableAttrs.hasChromeOnly():
+            properties.unforgeableAttrs.hasChromeOnly() or
+            properties.unforgeableMethods.hasNonChromeOnly() or
+            properties.unforgeableMethods.hasChromeOnly)
+
+    unforgeables = []
+
+    defineUnforgeableAttrs = fill(
+        """
+        if (!DefineUnforgeableAttributes(aCx, unforgeableHolder, %s)) {
+          $*{failureCode}
+        }
+        """,
+        failureCode=failureCode)
+    defineUnforgeableMethods = fill(
+        """
+        if (!DefineUnforgeableMethods(aCx, unforgeableHolder, %s)) {
+          $*{failureCode}
+        }
+        """,
+        failureCode=failureCode)
+
+    unforgeableMembers = [
+        (defineUnforgeableAttrs, properties.unforgeableAttrs),
+        (defineUnforgeableMethods, properties.unforgeableMethods)
+    ]
+    for (template, array) in unforgeableMembers:
+        if array.hasNonChromeOnly():
+            unforgeables.append(CGGeneric(template % array.variableName(False)))
+        if array.hasChromeOnly():
+            unforgeables.append(
+                CGIfWrapper(CGGeneric(template % array.variableName(True)),
+                            "nsContentUtils::ThreadsafeIsCallerChrome()"))
+
+    if descriptor.interface.getExtendedAttribute("Unforgeable"):
+        # We do our undefined toJSON here, not as a regular property
+        # because we don't have a concept of value props anywhere in IDL.
+        unforgeables.append(CGGeneric(fill(
+            """
+            if (!JS_DefineProperty(aCx, unforgeableHolder, "toJSON", JS::UndefinedHandleValue,
+                                   JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT)) {
+              $*{failureCode}
+            }
+            """,
+            failureCode=failureCode)))
+
+    return CGWrapper(CGList(unforgeables), pre="\n")
+
+def CopyUnforgeablePropertiesToInstance(descriptor, wrapperCache):
+    """
+    Copy the unforgeable properties from the unforgeable holder for
+    this interface to the instance object we have.
+    """
+    if not descriptor.hasUnforgeableMembers:
         return ""
 
-    unforgeables = [
+    copyCode = [
         CGGeneric(dedent(
             """
             // Important: do unforgeable property setup after we have handed
@@ -3053,7 +3159,7 @@ def InitUnforgeableProperties(descriptor, properties, wrapperCache):
     # For proxies, we want to define on the expando object, not directly on the
     # reflector, so we can make sure we don't get confused by named getters.
     if descriptor.proxy:
-        unforgeables.append(CGGeneric(fill(
+        copyCode.append(CGGeneric(fill(
             """
             JS::Rooted<JSObject*> expando(aCx,
               DOMProxyHandler::EnsureExpandoObject(aCx, aReflector));
@@ -3067,52 +3173,30 @@ def InitUnforgeableProperties(descriptor, properties, wrapperCache):
     else:
         obj = "aReflector"
 
-    defineUnforgeableAttrs = fill(
+    # We can't do the fast copy for globals, because we can't allocate the
+    # unforgeable holder for those with the right JSClass.  Luckily, there
+    # aren't too many globals being created.
+    if descriptor.isGlobal():
+        copyFunc = "JS_CopyPropertiesFrom"
+    else:
+        copyFunc = "JS_InitializePropertiesFromCompatibleNativeObject"
+    copyCode.append(CGGeneric(fill(
         """
-        if (!DefineUnforgeableAttributes(aCx, ${obj}, %s)) {
+        // XXXbz Once we allow subclassing, we'll want to make sure that
+        // this uses the canonical proto, not whatever random passed-in
+        // proto we end up using for the object.
+        JS::Rooted<JSObject*> unforgeableHolder(aCx,
+          &js::GetReservedSlot(proto, DOM_INTERFACE_PROTO_SLOTS_BASE).toObject());
+        if (!${copyFunc}(aCx, ${obj}, unforgeableHolder)) {
           $*{cleanup}
           return false;
         }
         """,
+        copyFunc=copyFunc,
         obj=obj,
-        cleanup=cleanup)
+        cleanup=cleanup)))
-    defineUnforgeableMethods = fill(
-        """
-        if (!DefineUnforgeableMethods(aCx, ${obj}, %s)) {
-          $*{cleanup}
-          return false;
-        }
-        """,
-        obj=obj,
-        cleanup=cleanup)
 
-    unforgeableMembers = [
+    return CGWrapper(CGList(copyCode), pre="\n").define()
-        (defineUnforgeableAttrs, properties.unforgeableAttrs),
-        (defineUnforgeableMethods, properties.unforgeableMethods)
-    ]
-    for (template, array) in unforgeableMembers:
-        if array.hasNonChromeOnly():
-            unforgeables.append(CGGeneric(template % array.variableName(False)))
-        if array.hasChromeOnly():
-            unforgeables.append(
-                CGIfWrapper(CGGeneric(template % array.variableName(True)),
-                            "nsContentUtils::ThreadsafeIsCallerChrome()"))
-
-    if descriptor.interface.getExtendedAttribute("Unforgeable"):
-        # We do our undefined toJSON here, not as a regular property
-        # because we don't have a concept of value props anywhere.
-        unforgeables.append(CGGeneric(fill(
-            """
-            if (!JS_DefineProperty(aCx, ${obj}, "toJSON", JS::UndefinedHandleValue,
-                                   JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT)) {
-              $*{cleanup}
-              return false;
-            }
-            """,
-            obj=obj,
-            cleanup=cleanup)))
-
-    return CGWrapper(CGList(unforgeables), pre="\n").define()
 
 
 def AssertInheritanceChain(descriptor):
@@ -3174,16 +3258,6 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
         self.properties = properties
 
     def definition_body(self):
-        # For proxies, we have to SetWrapper() before we init unforgeables.  But
-        # for non-proxies we'd rather do it the other way around, so our
-        # unforgeables don't force preservation of the wrapper.
-        setWrapper = "aCache->SetWrapper(aReflector);\n"
-        if self.descriptor.proxy:
-            setWrapperProxy = setWrapper
-            setWrapperNonProxy = ""
-        else:
-            setWrapperProxy = ""
-            setWrapperNonProxy = setWrapper
         return fill(
             """
             $*{assertion}
@@ -3212,18 +3286,15 @@ class CGWrapWithCacheMethod(CGAbstractMethod):
 
             $*{createObject}
 
-            $*{setWrapperProxy}
+            aCache->SetWrapper(aReflector);
             $*{unforgeable}
-            $*{setWrapperNonProxy}
             $*{slots}
             creator.InitializationSucceeded();
             return true;
             """,
             assertion=AssertInheritanceChain(self.descriptor),
             createObject=CreateBindingJSObject(self.descriptor, self.properties),
-            setWrapperProxy=setWrapperProxy,
+            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor, True),
-            unforgeable=InitUnforgeableProperties(self.descriptor, self.properties, True),
-            setWrapperNonProxy=setWrapperNonProxy,
             slots=InitMemberSlots(self.descriptor, True))
 
 
@@ -3280,7 +3351,7 @@ class CGWrapNonWrapperCacheMethod(CGAbstractMethod):
             """,
             assertions=AssertInheritanceChain(self.descriptor),
             createObject=CreateBindingJSObject(self.descriptor, self.properties),
-            unforgeable=InitUnforgeableProperties(self.descriptor, self.properties, False),
+            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor, False),
             slots=InitMemberSlots(self.descriptor, False))
 
 
@@ -3321,13 +3392,23 @@ class CGWrapGlobalMethod(CGAbstractMethod):
         else:
             fireOnNewGlobal = ""
 
+        if self.descriptor.hasUnforgeableMembers:
+            declareProto = "JS::Handle<JSObject*> proto =\n"
+            assertProto = (
+                "MOZ_ASSERT(proto &&\n"
+                "           IsDOMIfaceAndProtoClass(js::GetObjectClass(proto)));\n")
+        else:
+            declareProto = ""
+            assertProto = ""
+
         return fill(
             """
             $*{assertions}
             MOZ_ASSERT(ToSupportsIsOnPrimaryInheritanceChain(aObject, aCache),
                        "nsISupports must be on our primary inheritance chain");
 
-            CreateGlobal<${nativeType}, GetProtoObjectHandle>(aCx,
+            $*{declareProto}
+              CreateGlobal<${nativeType}, GetProtoObjectHandle>(aCx,
                                              aObject,
                                              aCache,
                                              Class.ToJSClass(),
@@ -3338,6 +3419,7 @@ class CGWrapGlobalMethod(CGAbstractMethod):
             if (!aReflector) {
               return false;
             }
+            $*{assertProto}
 
             // aReflector is a new global, so has a new compartment.  Enter it
             // before doing anything with it.
@@ -3355,9 +3437,11 @@ class CGWrapGlobalMethod(CGAbstractMethod):
             """,
             assertions=AssertInheritanceChain(self.descriptor),
             nativeType=self.descriptor.nativeType,
+            declareProto=declareProto,
+            assertProto=assertProto,
             properties=properties,
             chromeProperties=chromeProperties,
-            unforgeable=InitUnforgeableProperties(self.descriptor, self.properties, True),
+            unforgeable=CopyUnforgeablePropertiesToInstance(self.descriptor, True),
             slots=InitMemberSlots(self.descriptor, True),
             fireOnNewGlobal=fireOnNewGlobal)
 
@@ -10178,7 +10262,7 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
 
         namedSetter = self.descriptor.operations['NamedSetter']
         if namedSetter:
-            if HasUnforgeableMembers(self.descriptor):
+            if self.descriptor.hasUnforgeableMembers:
                 raise TypeError("Can't handle a named setter on an interface "
                                 "that has unforgeables.  Figure out how that "
                                 "should work!")
@@ -10235,7 +10319,7 @@ class CGDOMJSProxyHandler_delete(ClassMethod):
             assert type in ("Named", "Indexed")
             deleter = self.descriptor.operations[type + 'Deleter']
             if deleter:
-                if HasUnforgeableMembers(self.descriptor):
+                if self.descriptor.hasUnforgeableMembers:
                     raise TypeError("Can't handle a deleter on an interface "
                                     "that has unforgeables.  Figure out how "
                                     "that should work!")
@@ -10591,7 +10675,7 @@ class CGDOMJSProxyHandler_setCustom(ClassMethod):
             if self.descriptor.operations['NamedCreator'] is not namedSetter:
                 raise ValueError("In interface " + self.descriptor.name + ": " +
                                  "Can't cope with named setter that is not also a named creator")
-            if HasUnforgeableMembers(self.descriptor):
+            if self.descriptor.hasUnforgeableMembers:
                 raise ValueError("In interface " + self.descriptor.name + ": " +
                                  "Can't cope with [OverrideBuiltins] and unforgeable members")
 

dom/bindings/Configuration.py

@@ -294,6 +294,18 @@ def methodReturnsJSObject(method):
 
     return False
 
+
+def MemberIsUnforgeable(member, descriptor):
+    # Note: "or" and "and" return either their LHS or RHS, not
+    # necessarily booleans.  Make sure to return a boolean from this
+    # method, because callers will compare its return value to
+    # booleans.
+    return bool((member.isAttr() or member.isMethod()) and
+                not member.isStatic() and
+                (member.isUnforgeable() or
+                 descriptor.interface.getExtendedAttribute("Unforgeable")))
+
+
 class Descriptor(DescriptorProvider):
     """
     Represents a single descriptor for an interface. See Bindings.conf.
@@ -370,6 +382,9 @@ class Descriptor(DescriptorProvider):
         self.concrete = (not self.interface.isExternal() and
                          not self.interface.isCallback() and
                          desc.get('concrete', True))
+        self.hasUnforgeableMembers = (self.concrete and
+                                      any(MemberIsUnforgeable(m, self) for m in
+                                          self.interface.members))
         self.operations = {
             'IndexedGetter': None,
             'IndexedSetter': None,

dom/bindings/JSSlots.h

@@ -26,7 +26,8 @@
 #define DOM_INTERFACE_SLOTS_BASE 0
 
 // Interface prototype objects store a number of reserved slots equal to
-// DOM_INTERFACE_PROTO_SLOTS_BASE.
+// DOM_INTERFACE_PROTO_SLOTS_BASE or DOM_INTERFACE_PROTO_SLOTS_BASE + 1 if a
+// slot for the unforgeable holder is needed.
 #define DOM_INTERFACE_PROTO_SLOTS_BASE 0
 
 #endif /* mozilla_dom_DOMSlots_h */

xpcom/base/CycleCollectedJSRuntime.cpp

@@ -619,10 +619,14 @@ CycleCollectedJSRuntime::NoteGCThingXPCOMChildren(const js::Class* aClasp,
     const DOMJSClass* domClass = GetDOMClass(aObj);
     if (domClass) {
       NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "UnwrapDOMObject(obj)");
+      // It's possible that our object is an unforgeable holder object, in
+      // which case it doesn't actually have a C++ DOM object associated with
+      // it.  Use UnwrapPossiblyNotInitializedDOMObject, which produces null in
+      // that case, since NoteXPCOMChild/NoteNativeChild are null-safe.
       if (domClass->mDOMObjectIsISupports) {
-        aCb.NoteXPCOMChild(UnwrapDOMObject<nsISupports>(aObj));
+        aCb.NoteXPCOMChild(UnwrapPossiblyNotInitializedDOMObject<nsISupports>(aObj));
       } else if (domClass->mParticipant) {
-        aCb.NoteNativeChild(UnwrapDOMObject<void>(aObj),
+        aCb.NoteNativeChild(UnwrapPossiblyNotInitializedDOMObject<void>(aObj),
                             domClass->mParticipant);
       }
     }