Bug 748983. Fix the instanceof behavior for new bindings in situations where we don't need a custom hasInstance hook. r=peterv, a=akeybl
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 02 May 2012 15:24:37 -0400
changeset 95619 faaebf77bbc825edb3cba37f15a246d830343ca5
parent 95618 3bf38599348034f2433db03832a8d39a7eadc130
child 95620 445db7a929dce0d7b0905578b2cae932064b1fb8
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, akeybl
bugs748983
milestone14.0a2
Bug 748983. Fix the instanceof behavior for new bindings in situations where we don't need a custom hasInstance hook. r=peterv, a=akeybl
dom/bindings/Codegen.py
dom/bindings/Utils.cpp
dom/bindings/Utils.h
dom/bindings/test/Makefile.in
dom/bindings/test/test_InstanceOf.html
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -147,18 +147,20 @@ static JSClass PrototypeClass = {
 class CGInterfaceObjectJSClass(CGThing):
     def __init__(self, descriptor):
         CGThing.__init__(self)
         self.descriptor = descriptor
     def declare(self):
         # We're purely for internal consumption
         return ""
     def define(self):
+        if not self.descriptor.hasInstanceInterface:
+            return ""
         ctorname = "NULL" if not self.descriptor.interface.ctor() else CONSTRUCT_HOOK_NAME
-        hasinstance = "NULL" if not self.descriptor.hasInstanceInterface else HASINSTANCE_HOOK_NAME
+        hasinstance = HASINSTANCE_HOOK_NAME
         return """
 static JSClass InterfaceObjectClass = {
   "Function", 0,
   JS_PropertyStub,       /* addProperty */
   JS_PropertyStub,       /* delProperty */
   JS_PropertyStub,       /* getProperty */
   JS_StrictPropertyStub, /* setProperty */
   JS_EnumerateStub,
@@ -630,29 +632,29 @@ class PropertyDefiner:
             return "s" + self.name
         return "NULL"
     def __str__(self):
         str = self.generateArray(self.regular, self.variableName(False))
         if self.hasChromeOnly():
             str += self.generateArray(self.chrome, self.variableName(True))
         return str
 
+# The length of a method is the maximum of the lengths of the
+# argument lists of all its overloads.
+def methodLength(method):
+    signatures = method.signatures()
+    return max([len(arguments) for (retType, arguments) in signatures])
+
 class MethodDefiner(PropertyDefiner):
     """
     A class for defining methods on a prototype object.
     """
     def __init__(self, descriptor, name, static):
         PropertyDefiner.__init__(self, descriptor, name)
 
-        # The length of a method is the maximum of the lengths of the
-        # argument lists of all its overloads.
-        def methodLength(method):
-            signatures = method.signatures()
-            return max([len(arguments) for (retType, arguments) in signatures])
-
         methods = [m for m in descriptor.interface.members if
                    m.isMethod() and m.isStatic() == static]
         self.chrome = [{"name": m.identifier.name,
                         "length": methodLength(m),
                         "flags": "JSPROP_ENUMERATE"} for m in methods]
         self.regular = [{"name": m.identifier.name,
                          "length": methodLength(m),
                          "flags": "JSPROP_ENUMERATE"}
@@ -823,22 +825,35 @@ class CGCreateInterfaceObjectsMethod(CGA
                        "    return NULL;\n"
                        "  }\n\n") % (idsToInit[0], init, idsToInit[0])
             
         getParentProto = ("  JSObject* parentProto = %s;\n" +
                           "  if (!parentProto) {\n" +
                           "    return NULL;\n" +
                           "  }") % getParentProto
 
+        needInterfaceObjectClass = (needInterfaceObject and
+                                    self.descriptor.hasInstanceInterface)
+        needConstructor = (needInterfaceObject and
+                           not self.descriptor.hasInstanceInterface)
+        if self.descriptor.interface.ctor():
+            constructHook = CONSTRUCT_HOOK_NAME
+            constructArgs = methodLength(self.descriptor.interface.ctor())
+        else:
+            constructHook = "ThrowingConstructorWorkers" if self.descriptor.workers else "ThrowingConstructor"
+            constructArgs = 0
+
         call = """return bindings::CreateInterfaceObjects(aCx, aGlobal, aReceiver, parentProto,
-                                          %s, %s,
+                                          %s, %s, %s, %d,
                                           %%(methods)s, %%(attrs)s, %%(consts)s, %%(staticMethods)s,
                                           %s);""" % (
             "&PrototypeClass" if needInterfacePrototypeObject else "NULL",
-            "&InterfaceObjectClass" if needInterfaceObject else "NULL",
+            "&InterfaceObjectClass" if needInterfaceObjectClass else "NULL",
+            constructHook if needConstructor else "NULL",
+            constructArgs,
             '"' + self.descriptor.interface.identifier.name + '"' if needInterfaceObject else "NULL")
 
         if self.properties.hasChromeOnly():
             if self.descriptor.workers:
                 accessCheck = "mozilla::dom::workers::GetWorkerPrivateFromContext(aCx)->IsChromeWorker()"
             else:
                 accessCheck = "xpc::AccessCheck::isChrome(js::GetObjectCompartment(aGlobal))"
             chrome = """
--- a/dom/bindings/Utils.cpp
+++ b/dom/bindings/Utils.cpp
@@ -21,27 +21,37 @@ DefineConstants(JSContext* cx, JSObject*
       return false;
     }
   }
   return true;
 }
 
 static JSObject*
 CreateInterfaceObject(JSContext* cx, JSObject* global, JSObject* receiver,
-                      JSClass* constructorClass, JSObject* proto,
+                      JSClass* constructorClass, JSNative constructorNative,
+                      unsigned ctorNargs, JSObject* proto,
                       JSFunctionSpec* staticMethods, ConstantSpec* constants,
                       const char* name)
 {
-  JSObject* functionProto = JS_GetFunctionPrototype(cx, global);
-  if (!functionProto) {
-    return NULL;
+  JSObject* constructor;
+  if (constructorClass) {
+    JSObject* functionProto = JS_GetFunctionPrototype(cx, global);
+    if (!functionProto) {
+      return NULL;
+    }
+    constructor = JS_NewObject(cx, constructorClass, functionProto, global);
+  } else {
+    MOZ_ASSERT(constructorNative);
+    JSFunction* fun = JS_NewFunction(cx, constructorNative, ctorNargs,
+                                     JSFUN_CONSTRUCTOR, global, name);
+    if (!fun) {
+      return NULL;
+    }
+    constructor = JS_GetFunctionObject(fun);
   }
-
-  JSObject* constructor =
-    JS_NewObject(cx, constructorClass, functionProto, global);
   if (!constructor) {
     return NULL;
   }
 
   if (staticMethods && !JS_DefineFunctions(cx, constructor, staticMethods)) {
     return NULL;
   }
 
@@ -87,44 +97,48 @@ CreateInterfacePrototypeObject(JSContext
   }
 
   return ourProto;
 }
 
 JSObject*
 CreateInterfaceObjects(JSContext* cx, JSObject* global, JSObject *receiver,
                        JSObject* protoProto, JSClass* protoClass,
-                       JSClass* constructorClass, JSFunctionSpec* methods,
+                       JSClass* constructorClass, JSNative constructor,
+                       unsigned ctorNargs, JSFunctionSpec* methods,
                        JSPropertySpec* properties, ConstantSpec* constants,
                        JSFunctionSpec* staticMethods, const char* name)
 {
-  MOZ_ASSERT(protoClass || constructorClass, "Need at least one class!");
+  MOZ_ASSERT(protoClass || constructorClass || constructor,
+             "Need at least one class or a constructor!");
   MOZ_ASSERT(!(methods || properties) || protoClass,
              "Methods or properties but no protoClass!");
-  MOZ_ASSERT(!staticMethods || constructorClass,
+  MOZ_ASSERT(!staticMethods || (constructorClass || constructor),
              "Static methods but no constructorClass!");
-  MOZ_ASSERT(bool(name) == bool(constructorClass),
+  MOZ_ASSERT(bool(name) == bool(constructorClass || constructor),
              "Must have name precisely when we have an interface object");
+  MOZ_ASSERT(!constructorClass || !constructor);
 
   JSObject* proto;
   if (protoClass) {
     proto = CreateInterfacePrototypeObject(cx, global, protoProto, protoClass,
                                            methods, properties, constants);
     if (!proto) {
       return NULL;
     }
   }
   else {
     proto = NULL;
   }
 
   JSObject* interface;
-  if (constructorClass) {
+  if (constructorClass || constructor) {
     interface = CreateInterfaceObject(cx, global, receiver, constructorClass,
-                                      proto, staticMethods, constants, name);
+                                      constructor, ctorNargs, proto,
+                                      staticMethods, constants, name);
     if (!interface) {
       return NULL;
     }
   }
 
   return protoClass ? proto : interface;
 }
 
@@ -234,11 +248,23 @@ QueryInterface(JSContext* cx, unsigned a
     return WrapObject(cx, obj, ci, &NS_GET_IID(nsIClassInfo), vp);
   }
   
   // Lie, otherwise we need to check classinfo or QI
   *vp = thisv;
   return true;
 }
 
+JSBool
+ThrowingConstructor(JSContext* cx, unsigned argc, JS::Value* vp)
+{
+  return Throw<true>(cx, NS_ERROR_FAILURE);
+}
+
+JSBool
+ThrowingConstructorWorkers(JSContext* cx, unsigned argc, JS::Value* vp)
+{
+  return Throw<false>(cx, NS_ERROR_FAILURE);
+}
+
 } // namespace bindings
 } // namespace dom
 } // namespace mozilla
--- a/dom/bindings/Utils.h
+++ b/dom/bindings/Utils.h
@@ -220,17 +220,24 @@ struct ConstantSpec
  *        prototype object
  * receiver is the object on which we need to define the interface object as a
  *          property
  * protoProto is the prototype to use for the interface prototype object.
  * protoClass is the JSClass to use for the interface prototype object.
  *            This is null if we should not create an interface prototype
  *            object.
  * constructorClass is the JSClass to use for the interface object.
- *                  This is null if we should not create an interface object.
+ *                  This is null if we should not create an interface object or
+ *                  if it should be a function object.
+ * constructor is the JSNative to use as a constructor.  If this is non-null, it
+ *             should be used as a JSNative to back the interface object, which
+ *             should be a Function.  If this is null, then we should create an
+ *             object of constructorClass, unless that's also null, in which
+ *             case we should not create an interface object at all.
+ * ctorNargs is the length of the constructor function; 0 if no constructor
  * methods and properties are to be defined on the interface prototype object;
  *                        these arguments are allowed to be null if there are no
  *                        methods or properties respectively.
  * constants are to be defined on the interface object and on the interface
  *           prototype object; allowed to be null if there are no constants.
  * staticMethods are to be defined on the interface object; allowed to be null
  *               if there are no static methods.
  *
@@ -240,17 +247,18 @@ struct ConstantSpec
  * non-null.
  *
  * returns the interface prototype object if protoClass is non-null, else it
  * returns the interface object.
  */
 JSObject*
 CreateInterfaceObjects(JSContext* cx, JSObject* global, JSObject* receiver,
                        JSObject* protoProto, JSClass* protoClass,
-                       JSClass* constructorClass, JSFunctionSpec* methods,
+                       JSClass* constructorClass, JSNative constructor,
+                       unsigned ctorNargs, JSFunctionSpec* methods,
                        JSPropertySpec* properties, ConstantSpec* constants,
                        JSFunctionSpec* staticMethods, const char* name);
 
 template <class T>
 inline bool
 WrapNewBindingObject(JSContext* cx, JSObject* scope, T* value, JS::Value* vp)
 {
   JSObject* obj = value->GetWrapper();
@@ -499,14 +507,18 @@ InitIds(JSContext* cx, Spec* specs, jsid
     *ids = INTERNED_STRING_TO_JSID(cx, str);
   } while (++ids, (++spec)->name);
 
   return true;
 }
 
 JSBool
 QueryInterface(JSContext* cx, unsigned argc, JS::Value* vp);
+JSBool
+ThrowingConstructor(JSContext* cx, unsigned argc, JS::Value* vp);
+JSBool
+ThrowingConstructorWorkers(JSContext* cx, unsigned argc, JS::Value* vp);
 
 } // namespace bindings
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_bindings_Utils_h__ */
--- a/dom/bindings/test/Makefile.in
+++ b/dom/bindings/test/Makefile.in
@@ -8,12 +8,13 @@ srcdir = @srcdir@
 VPATH = @srcdir@
 relativesrcdir = dom/bindings/test
 
 include $(DEPTH)/config/autoconf.mk
 include $(topsrcdir)/config/rules.mk
 
 _TEST_FILES = \
   test_lookupGetter.html \
+  test_InstanceOf.html \
   $(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_InstanceOf.html
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=748983
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 748983</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=748983">Mozilla Bug 748983</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 748983 **/
+ok(document instanceof EventTarget, "document is an event target")
+ok(new XMLHttpRequest() instanceof XMLHttpRequest, "instanceof should work on XHR");
+
+</script>
+</pre>
+</body>
+</html>