Bug 1085062. Remove hasXPConnectImpls support from bindings codegen. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 04 Apr 2018 14:39:52 -0400
changeset 465316 6d94022fef7a99bc7d246ee561ed6443836b1244
parent 465315 a9a254ed2869dad2802a6658f73a941438ff9cc2
child 465317 ac90c20959de0499253cabb9f272f2faa7a87919
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1085062
milestone61.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 1085062. Remove hasXPConnectImpls support from bindings codegen. r=peterv
dom/base/nsWrapperCache.h
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/Configuration.py
--- a/dom/base/nsWrapperCache.h
+++ b/dom/base/nsWrapperCache.h
@@ -338,17 +338,19 @@ protected:
       // dereferenced.
       mWrapper = reinterpret_cast<JSObject*>(1);
     }
   }
 
 private:
   // Friend declarations for things that need to be able to call
   // SetIsNotDOMBinding().  The goal is to get rid of all of these, and
-  // SetIsNotDOMBinding() too.
+  // SetIsNotDOMBinding() too.  Once that's done, we can remove the
+  // couldBeDOMBinding bits in DoGetOrCreateDOMReflector, as well as any ither
+  // consumers of IsDOMBinding().
   friend class SandboxPrivate;
   void SetIsNotDOMBinding()
   {
     MOZ_ASSERT(!mWrapper && !(GetWrapperFlags() & ~WRAPPER_IS_NOT_DOM_BINDING),
                "This flag should be set before creating any wrappers.");
     SetWrapperFlags(WRAPPER_IS_NOT_DOM_BINDING);
   }
 
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1228,18 +1228,17 @@ QueryInterface(JSContext* cx, unsigned a
                                                   /* stopAtWindowProxy = */ false));
   if (!obj) {
       JS_ReportErrorASCII(cx, "Permission denied to access object");
       return false;
   }
 
   // Switch this to UnwrapDOMObjectToISupports once our global objects are
   // using new bindings.
-  nsCOMPtr<nsISupports> native;
-  UnwrapArg<nsISupports>(cx, obj, getter_AddRefs(native));
+  nsCOMPtr<nsISupports> native = UnwrapDOMObjectToISupports(obj);
   if (!native) {
     return Throw(cx, NS_ERROR_FAILURE);
   }
 
   if (argc < 1) {
     return Throw(cx, NS_ERROR_XPC_NOT_ENOUGH_ARGS);
   }
 
@@ -3261,50 +3260,16 @@ UnwrapArgImpl(JSContext* cx,
   // We need to go through the QueryInterface logic to make this return
   // the right thing for the various 'special' interfaces; e.g.
   // nsIPropertyBag. We must use AggregatedQueryInterface in cases where
   // there is an outer to avoid nasty recursion.
   return wrappedJS->QueryInterface(iid, ppArg);
 }
 
 nsresult
-UnwrapXPConnectImpl(JSContext* cx,
-                    JS::MutableHandle<JS::Value> src,
-                    const nsIID &iid,
-                    void **ppArg)
-{
-  if (!NS_IsMainThread()) {
-    return NS_ERROR_NOT_AVAILABLE;
-  }
-
-  MOZ_ASSERT(src.isObject());
-  // Unwrap ourselves, because we're going to want access to the unwrapped
-  // object.
-  JS::Rooted<JSObject*> obj(cx,
-                            js::CheckedUnwrap(&src.toObject(),
-                                              /* stopAtWindowProxy = */ false));
-  if (!obj) {
-    return NS_ERROR_NOT_AVAILABLE;
-  }
-
-  nsCOMPtr<nsISupports> iface = xpc::UnwrapReflectorToISupports(obj);
-  if (!iface) {
-    return NS_ERROR_XPC_BAD_CONVERT_JS;
-  }
-
-  if (NS_FAILED(iface->QueryInterface(iid, ppArg))) {
-    return NS_ERROR_XPC_BAD_CONVERT_JS;
-  }
-
-  // Now update our source to keep rooting our object.
-  src.setObject(*obj);
-  return NS_OK;
-}
-
-nsresult
 UnwrapWindowProxyImpl(JSContext* cx,
                       JS::Handle<JSObject*> src,
                       nsPIDOMWindowOuter** ppArg)
 {
   nsCOMPtr<nsPIDOMWindowInner> inner;
   nsresult rv = UnwrapArg<nsPIDOMWindowInner>(cx, src, getter_AddRefs(inner));
   NS_ENSURE_SUCCESS(rv, rv);
 
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -75,39 +75,16 @@ UnwrapArg(JSContext* cx, JS::Handle<JSOb
 template <>
 inline nsresult
 UnwrapArg<nsPIDOMWindowOuter>(JSContext* cx, JS::Handle<JSObject*> src,
                               nsPIDOMWindowOuter** ppArg)
 {
   return UnwrapWindowProxyImpl(cx, src, ppArg);
 }
 
-nsresult
-UnwrapXPConnectImpl(JSContext* cx, JS::MutableHandle<JS::Value> src,
-                    const nsIID& iid, void** ppArg);
-
-/*
- * Convert a jsval being used as a Web IDL interface implementation to an XPCOM
- * pointer; this is only used for Web IDL interfaces that specify
- * hasXPConnectImpls.  This is not the same as UnwrapArg because caller _can_
- * assume that if unwrapping succeeds "val" will be updated so it's rooting the
- * XPCOM pointer.  Also, UnwrapXPConnect doesn't need to worry about doing
- * XPCWrappedJS things.
- *
- * val must be an ObjectValue.
- */
-template<class Interface>
-inline nsresult
-UnwrapXPConnect(JSContext* cx, JS::MutableHandle<JS::Value> val,
-                Interface** ppThis)
-{
-  return UnwrapXPConnectImpl(cx, val, NS_GET_TEMPLATE_IID(Interface),
-                             reinterpret_cast<void**>(ppThis));
-}
-
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
                  bool aSecurityError, const char* aInterfaceName);
 
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
                  bool aSecurityError, prototypes::ID aProtoId);
 
@@ -1088,17 +1065,18 @@ AssertReflectorHasGivenProto(JSContext* 
 template <class T, GetOrCreateReflectorWrapBehavior wrapBehavior>
 MOZ_ALWAYS_INLINE bool
 DoGetOrCreateDOMReflector(JSContext* cx, T* value,
                           JS::Handle<JSObject*> givenProto,
                           JS::MutableHandle<JS::Value> rval)
 {
   MOZ_ASSERT(value);
   MOZ_ASSERT_IF(givenProto, js::IsObjectInContextCompartment(givenProto, cx));
-  // We can get rid of this when we remove support for hasXPConnectImpls.
+  // We can get rid of this when we remove support for
+  // nsWrapperCache::SetIsNotDOMBinding.
   bool couldBeDOMBinding = CouldBeDOMBinding(value);
   JSObject* obj = value->GetWrapper();
   if (obj) {
 #ifdef DEBUG
     AssertReflectorHasGivenProto(cx, obj, givenProto);
     // Have to reget obj because AssertReflectorHasGivenProto can
     // trigger gc so the pointer may now be invalid.
     obj = value->GetWrapper();
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -681,21 +681,16 @@ class CGPrototypeJSClass(CGThing):
             slotCount=slotCount,
             type=type,
             hooks=NativePropertyHooks(self.descriptor),
             prototypeID=prototypeID,
             depth=depth,
             protoGetter=protoGetter)
 
 
-def NeedsGeneratedHasInstance(descriptor):
-    assert descriptor.interface.hasInterfaceObject()
-    return descriptor.hasXPConnectImpls
-
-
 def InterfaceObjectProtoGetter(descriptor, forXrays=False):
     """
     Returns a tuple with two elements:
 
         1) The name of the function to call to get the prototype to use for the
            interface object as a JSObject*.
 
         2) The name of the function to call to get the prototype to use for the
@@ -736,19 +731,17 @@ class CGInterfaceObjectJSClass(CGThing):
     def define(self):
         if self.descriptor.interface.ctor():
             assert not self.descriptor.interface.isNamespace()
             ctorname = CONSTRUCT_HOOK_NAME
         elif self.descriptor.interface.isNamespace():
             ctorname = "nullptr"
         else:
             ctorname = "ThrowingConstructor"
-        needsHasInstance = (
-            not NeedsGeneratedHasInstance(self.descriptor) and
-            self.descriptor.interface.hasInterfacePrototypeObject())
+        needsHasInstance = self.descriptor.interface.hasInterfacePrototypeObject()
 
         prototypeID, depth = PrototypeIDAndDepth(self.descriptor)
         slotCount = "DOM_INTERFACE_SLOTS_BASE"
         if len(self.descriptor.interface.namedConstructors) > 0:
             slotCount += (" + %i /* slots for the named constructors */" %
                           len(self.descriptor.interface.namedConstructors))
         (protoGetter, _) = InterfaceObjectProtoGetter(self.descriptor,
                                                       forXrays=True)
@@ -1101,25 +1094,16 @@ class CGHeaders(CGWrapper):
 
         # Grab the includes for checking hasInstance
         interfacesImplementingSelf = set()
         for d in descriptors:
             interfacesImplementingSelf |= d.interface.interfacesImplementingSelf
         implementationIncludes |= set(self.getDeclarationFilename(i) for i in
                                       interfacesImplementingSelf)
 
-        # Grab the includes for the things that involve XPCOM interfaces
-        hasInstanceIncludes = set(self.getDeclarationFilename(d.interface) for d
-                                  in descriptors if
-                                  d.interface.hasInterfaceObject() and
-                                  NeedsGeneratedHasInstance(d) and
-                                  d.interface.hasInterfacePrototypeObject())
-        if len(hasInstanceIncludes) > 0:
-            hasInstanceIncludes.add("nsContentUtils.h")
-
         # Now find all the things we'll need as arguments because we
         # need to wrap or unwrap them.
         bindingHeaders = set()
         declareIncludes = set(declareIncludes)
 
         def addHeadersForType((t, dictionary)):
             """
             Add the relevant headers for this type.  We use dictionary, if
@@ -1293,17 +1277,16 @@ class CGHeaders(CGWrapper):
         # Let the machinery do its thing.
         def _includeString(includes):
             return ''.join(['#include "%s"\n' % i for i in includes]) + '\n'
         CGWrapper.__init__(self, child,
                            declarePre=_includeString(sorted(declareIncludes)),
                            definePre=_includeString(sorted(set(defineIncludes) |
                                                            bindingIncludes |
                                                            bindingHeaders |
-                                                           hasInstanceIncludes |
                                                            implementationIncludes)))
 
     @staticmethod
     def getDeclarationFilename(decl):
         # Use our local version of the header, not the exported one, so that
         # test bindings, which don't export, will work correctly.
         basename = os.path.basename(decl.filename())
         return basename.replace('.webidl', 'Binding.h')
@@ -1975,62 +1958,16 @@ class CGNamedConstructors(CGThing):
               { nullptr, { nullptr, nullptr }, 0 }
             };
             """,
             name=self.descriptor.name,
             constructorID=constructorID,
             namedConstructors=namedConstructors)
 
 
-class CGHasInstanceHook(CGAbstractStaticMethod):
-    def __init__(self, descriptor):
-        args = [Argument('JSContext*', 'cx'),
-                Argument('unsigned', 'argc'),
-                Argument('JS::Value*', 'vp')]
-        assert descriptor.interface.hasInterfaceObject()
-        assert NeedsGeneratedHasInstance(descriptor)
-        CGAbstractStaticMethod.__init__(self, descriptor, HASINSTANCE_HOOK_NAME,
-                                        'bool', args)
-
-    def define(self):
-        return CGAbstractStaticMethod.define(self)
-
-    def definition_body(self):
-        return self.generate_code()
-
-    def generate_code(self):
-        return fill(
-            """
-            JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-            if (!args.get(0).isObject()) {
-              args.rval().setBoolean(false);
-              return true;
-            }
-
-            JS::Rooted<JSObject*> instance(cx, &args[0].toObject());
-
-            static_assert(IsBaseOf<nsISupports, ${nativeType}>::value,
-                          "HasInstance only works for nsISupports-based classes.");
-
-            bool ok = InterfaceHasInstance(cx, argc, vp);
-            if (!ok || args.rval().toBoolean()) {
-              return ok;
-            }
-
-            // FIXME Limit this to chrome by checking xpc::AccessCheck::isChrome(obj).
-            nsCOMPtr<nsISupports> native =
-              xpc::UnwrapReflectorToISupports(js::UncheckedUnwrap(instance, /* stopAtWindowProxy = */ false));
-            nsCOMPtr<nsIDOM${name}> qiResult = do_QueryInterface(native);
-            args.rval().setBoolean(!!qiResult);
-            return true;
-            """,
-            nativeType=self.descriptor.nativeType,
-            name=self.descriptor.interface.identifier.name)
-
-
 def isChromeOnly(m):
     return m.getExtendedAttribute("ChromeOnly")
 
 
 class MemberCondition:
     """
     An object representing the condition for a member to actually be
     exposed.  Any of the arguments can be None.  If not
@@ -2437,30 +2374,16 @@ class MethodDefiner(PropertyDefiner):
                 "name": "@@iterator",
                 "methodInfo": False,
                 "selfHostedName": "ArrayValues",
                 "length": 0,
                 "flags": "0", # Not enumerable, per spec.
                 "condition": MemberCondition()
             })
 
-        if (static and
-            not unforgeable and
-            descriptor.interface.hasInterfaceObject() and
-            NeedsGeneratedHasInstance(descriptor)):
-            self.regular.append({
-                "name": "@@hasInstance",
-                "methodInfo": False,
-                "nativeName": HASINSTANCE_HOOK_NAME,
-                "length": 1,
-                # Flags match those of Function[Symbol.hasInstance]
-                "flags": "JSPROP_READONLY | JSPROP_PERMANENT",
-                "condition": MemberCondition()
-            })
-
         # Generate the keys/values/entries aliases for value iterables.
         maplikeOrSetlikeOrIterable = descriptor.interface.maplikeOrSetlikeOrIterable
         if (not static and
             not unforgeable and
             maplikeOrSetlikeOrIterable and
             maplikeOrSetlikeOrIterable.isIterable() and
             maplikeOrSetlikeOrIterable.isValueIterator()):
             # Add our keys/values/entries/forEach
@@ -4335,24 +4258,16 @@ class CastableObjectUnwrapper():
                  exceptionCode=None, isCallbackReturnValue=False,
                  allowCrossOriginObj=False):
         self.substitution = {
             "type": descriptor.nativeType,
             "protoID": "prototypes::id::" + descriptor.name,
             "target": target,
             "codeOnFailure": codeOnFailure,
         }
-        # Supporting both the "cross origin object" case and the "has
-        # XPConnect impls" case at the same time is a pain, so let's
-        # not do that.  That allows us to assume that our source is
-        # always a Handle or MutableHandle.
-        if allowCrossOriginObj and descriptor.hasXPConnectImpls:
-            raise TypeError("Interface %s both allows a cross-origin 'this' "
-                            "and has XPConnect impls.  We don't support that" %
-                            descriptor.name)
         if allowCrossOriginObj:
             self.substitution["uncheckedObjDecl"] = fill(
                 """
                 JS::Rooted<JSObject*> maybeUncheckedObj(cx, &${source}.toObject());
                 """,
                 source=source)
             self.substitution["uncheckedObjGet"] = fill(
                 """
@@ -4363,40 +4278,24 @@ class CastableObjectUnwrapper():
                   if (!maybeUncheckedObj) {
                     $*{codeOnFailure}
                   }
                 }
                 """,
                 codeOnFailure=(codeOnFailure % { 'securityError': 'true'}))
             self.substitution["source"] = "maybeUncheckedObj"
             self.substitution["mutableSource"] = "&maybeUncheckedObj"
-            # No need to set up xpconnectUnwrap, since it won't be
-            # used in the allowCrossOriginObj case.
         else:
             self.substitution["uncheckedObjDecl"] = ""
             self.substitution["uncheckedObjGet"] = ""
             self.substitution["source"] = source
             self.substitution["mutableSource"] = mutableSource
-            xpconnectUnwrap = (
-                "nsresult rv = UnwrapXPConnect<${type}>(cx, ${mutableSource}, getter_AddRefs(objPtr));\n")
-
-        if descriptor.hasXPConnectImpls:
-            self.substitution["codeOnFailure"] = string.Template(
-                "RefPtr<${type}> objPtr;\n" +
-                xpconnectUnwrap +
-                "if (NS_FAILED(rv)) {\n"
-                "${indentedCodeOnFailure}"
-                "}\n"
-                "// We should have an object\n"
-                "MOZ_ASSERT(objPtr);\n"
-                "${target} = objPtr;\n"
-            ).substitute(self.substitution,
-                         indentedCodeOnFailure=indent(codeOnFailure))
-        elif (isCallbackReturnValue == "JSImpl" and
-              descriptor.interface.isJSImplemented()):
+
+        if (isCallbackReturnValue == "JSImpl" and
+            descriptor.interface.isJSImplemented()):
             exceptionCode = exceptionCode or codeOnFailure
             self.substitution["codeOnFailure"] = fill(
                 """
                 // Be careful to not wrap random DOM objects here, even if
                 // they're wrapped in opaque security wrappers for some reason.
                 // XXXbz Wish we could check for a JS-implemented object
                 // that already has a content reflection...
                 if (!IsDOMObject(js::UncheckedUnwrap(&${source}.toObject()))) {
@@ -6830,28 +6729,20 @@ def getWrapTemplateForType(type, descrip
                 wrapMethod = "GetOrCreateDOMReflector"
                 wrapArgs = "cx, %s, ${jsvalHandle}" % result
             else:
                 wrapMethod = "WrapNewBindingNonWrapperCachedObject"
                 wrapArgs = "cx, ${obj}, %s, ${jsvalHandle}" % result
             if isConstructorRetval:
                 wrapArgs += ", desiredProto"
             wrap = "%s(%s)" % (wrapMethod, wrapArgs)
-            if not descriptor.hasXPConnectImpls:
-                # Can only fail to wrap as a new-binding object
-                # if they already threw an exception.
-                failed = ("MOZ_ASSERT(JS_IsExceptionPending(cx));\n" +
-                          exceptionCode)
-            else:
-                if descriptor.notflattened:
-                    raise TypeError("%s has XPConnect impls but not flattened; "
-                                    "fallback won't work correctly" %
-                                    descriptor.interface.identifier.name)
-                # Try old-style wrapping for bindings which might be XPConnect impls.
-                failed = wrapAndSetPtr("HandleNewBindingWrappingFailure(cx, ${obj}, %s, ${jsvalHandle})" % result)
+            # Can only fail to wrap as a new-binding object
+            # if they already threw an exception.
+            failed = ("MOZ_ASSERT(JS_IsExceptionPending(cx));\n" +
+                      exceptionCode)
         else:
             if descriptor.notflattened:
                 getIID = "&NS_GET_IID(%s), " % descriptor.nativeType
             else:
                 getIID = ""
             wrap = "WrapObject(cx, %s, %s${jsvalHandle})" % (result, getIID)
             failed = None
 
@@ -9221,22 +9112,16 @@ class CGSpecializedGetter(CGAbstractStat
             # If the interface is maplike/setlike, there will be one getter
             # method for the size property of the backing object. Due to having
             # to unpack the backing object from the slot, this requires its own
             # generator.
             return getMaplikeOrSetlikeSizeGetterBody(self.descriptor, self.attr)
         nativeName = CGSpecializedGetter.makeNativeName(self.descriptor,
                                                         self.attr)
         if self.attr.slotIndices is not None:
-            if self.descriptor.hasXPConnectImpls:
-                raise TypeError("Interface '%s' has XPConnect impls, so we "
-                                "can't use our slot for property '%s'!" %
-                                (self.descriptor.interface.identifier.name,
-                                 self.attr.identifier.name))
-
             # We're going to store this return value in a slot on some object,
             # to cache it.  The question is, which object?  For dictionary and
             # sequence return values, we want to use a slot on the Xray expando
             # if we're called via Xrays, and a slot on our reflector otherwise.
             # On the other hand, when dealing with some interfacce types
             # (navigator properties, window.document) we want to avoid calling
             # the getter more than once.  In the case of navigator properties
             # that's because the getter actually creates a new object each time.
@@ -12858,22 +12743,16 @@ class CGDescriptor(CGThing):
         if descriptor.concrete and descriptor.wrapperCache and not descriptor.proxy:
             cgThings.append(CGClassObjectMovedHook(descriptor))
 
         # Generate the _ClearCachedFooValue methods before the property arrays that use them.
         if descriptor.interface.isJSImplemented():
             for m in clearableCachedAttrs(descriptor):
                 cgThings.append(CGJSImplClearCachedValueMethod(descriptor, m))
 
-        # Need to output our generated hasinstance bits before
-        # PropertyArrays tries to use them.
-        if (descriptor.interface.hasInterfaceObject() and
-            NeedsGeneratedHasInstance(descriptor)):
-            cgThings.append(CGHasInstanceHook(descriptor))
-
         properties = PropertyArrays(descriptor)
         cgThings.append(CGGeneric(define=str(properties)))
         cgThings.append(CGNativeProperties(descriptor, properties))
 
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGClassConstructor(descriptor,
                                                descriptor.interface.ctor()))
             cgThings.append(CGInterfaceObjectJSClass(descriptor, properties))
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -394,18 +394,16 @@ class Descriptor(DescriptorProvider):
         if self.jsImplParent == self.nativeType:
             self.jsImplParentHeader = self.headerFile
         else:
             self.jsImplParentHeader = self.jsImplParent.replace("::", "/") + ".h"
 
         self.notflattened = desc.get('notflattened', False)
         self.register = desc.get('register', True)
 
-        self.hasXPConnectImpls = desc.get('hasXPConnectImpls', False)
-
         # If we're concrete, we need to crawl our ancestor interfaces and mark
         # them as having a concrete descendant.
         self.concrete = (not self.interface.isExternal() and
                          not self.interface.isCallback() and
                          not self.interface.isNamespace() and
                          desc.get('concrete', True))
         self.hasUnforgeableMembers = (self.concrete and
                                       any(MemberIsUnforgeable(m, self) for m in
@@ -757,20 +755,20 @@ class Descriptor(DescriptorProvider):
     def needsXrayNamedDeleterHook(self):
         return self.operations["NamedDeleter"] is not None
 
     def needsSpecialGenericOps(self):
         """
         Returns true if this descriptor requires generic ops other than
         GenericBindingMethod/GenericBindingGetter/GenericBindingSetter.
 
-        In practice we need to do this if our this value might be an XPConnect
-        object or if we need to coerce null/undefined to the global.
+        In practice we need to do this if we need to coerce null/undefined
+        to the global.
         """
-        return self.hasXPConnectImpls or self.interface.isOnGlobalProtoChain()
+        return self.interface.isOnGlobalProtoChain()
 
     def isGlobal(self):
         """
         Returns true if this is the primary interface for a global object
         of some sort.
         """
         return (self.interface.getExtendedAttribute("Global") or
                 self.interface.getExtendedAttribute("PrimaryGlobal"))