Bug 1039986. Make Function.prototype.toString work on Web IDL interface objects. r=jorendorff,peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 27 Oct 2015 16:25:14 -0400
changeset 269871 e7929212ce5c8ca6511ad5b425e2c821bc1aa174
parent 269870 4a06e6b6c22d1fc04922f21afa0f67f5aed3dfb0
child 269872 912688ff102a86671bb7318e562f82f3519e4c93
push id29593
push usercbook@mozilla.com
push dateWed, 28 Oct 2015 09:44:28 +0000
treeherdermozilla-central@fc706d376f06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, peterv
bugs1039986
milestone44.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 1039986. Make Function.prototype.toString work on Web IDL interface objects. r=jorendorff,peterv
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/test/test_interfaceToString.html
js/public/Class.h
js/src/jsfriendapi.h
js/src/jsfun.cpp
js/src/proxy/Proxy.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -532,54 +532,29 @@ DefineUnforgeableAttributes(JSContext* c
                             const Prefable<const JSPropertySpec>* props)
 {
   return DefinePrefable(cx, obj, props);
 }
 
 
 // We should use JSFunction objects for interface objects, but we need a custom
 // hasInstance hook because we have new interface objects on prototype chains of
-// old (XPConnect-based) bindings. Because Function.prototype.toString throws if
-// passed a non-Function object we also need to provide our own toString method
-// for interface objects.
-
-static bool
-InterfaceObjectToString(JSContext* cx, unsigned argc, JS::Value *vp)
+// old (XPConnect-based) bindings. We also need Xrays and arbitrary numbers of
+// reserved slots (e.g. for named constructors).  So we define a custom
+// funToString ObjectOps member for interface objects.
+JSString*
+InterfaceObjectToString(JSContext* aCx, JS::Handle<JSObject*> aObject,
+                        unsigned /* indent */)
 {
-  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  if (!args.thisv().isObject()) {
-    JS_ReportErrorNumber(cx, js::GetErrorMessage, nullptr,
-                         JSMSG_CANT_CONVERT_TO, "null", "object");
-    return false;
-  }
-
-  JS::Rooted<JSObject*> thisObj(cx, &args.thisv().toObject());
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(thisObj, /* stopAtOuter = */ false));
-  if (!obj) {
-    JS_ReportError(cx, "Permission denied to access object");
-    return false;
-  }
-
-  const js::Class* clasp = js::GetObjectClass(obj);
-  if (!IsDOMIfaceAndProtoClass(clasp)) {
-    JS_ReportError(cx, "toString called on incompatible object");
-    return false;
-  }
+  const js::Class* clasp = js::GetObjectClass(aObject);
+  MOZ_ASSERT(IsDOMIfaceAndProtoClass(clasp));
 
   const DOMIfaceAndProtoJSClass* ifaceAndProtoJSClass =
     DOMIfaceAndProtoJSClass::FromJSClass(clasp);
-  JS::Rooted<JSString*> str(cx,
-                            JS_NewStringCopyZ(cx,
-                                              ifaceAndProtoJSClass->mToString));
-  if (!str) {
-    return false;
-  }
-
-  args.rval().setString(str);
-  return true;
+  return JS_NewStringCopyZ(aCx, ifaceAndProtoJSClass->mToString);
 }
 
 bool
 Constructor(JSContext* cx, unsigned argc, JS::Value* vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JS::Value& v =
     js::GetFunctionNativeReserved(&args.callee(),
@@ -642,25 +617,16 @@ CreateInterfaceObject(JSContext* cx, JS:
     constructor = CreateConstructor(cx, global, name, constructorNative,
                                     ctorNargs);
   }
   if (!constructor) {
     return nullptr;
   }
 
   if (constructorClass) {
-    // Have to shadow Function.prototype.toString, since that throws
-    // on things that are not js::FunctionClass.
-    JS::Rooted<JSFunction*> toString(cx,
-      JS_DefineFunction(cx, constructor, "toString", InterfaceObjectToString,
-                        0, 0));
-    if (!toString) {
-      return nullptr;
-    }
-
     if (!JS_DefineProperty(cx, constructor, "length", ctorNargs,
                            JSPROP_READONLY)) {
       return nullptr;
     }
 
     // Might as well intern, since we're going to need an atomized
     // version of name anyway when we stick our constructor on the
     // global.
@@ -1565,32 +1531,16 @@ XrayResolveOwnProperty(JSContext* cx, JS
   } else if (type == eInterface) {
     if (IdEquals(id, "prototype")) {
       return nativePropertyHooks->mPrototypeID == prototypes::id::_ID_Count ||
              ResolvePrototypeOrConstructor(cx, wrapper, obj,
                                            nativePropertyHooks->mPrototypeID,
                                            JSPROP_PERMANENT | JSPROP_READONLY,
                                            desc, cacheOnHolder);
     }
-
-    if (IdEquals(id, "toString") && !JS_ObjectIsFunction(cx, obj)) {
-      MOZ_ASSERT(IsDOMIfaceAndProtoClass(js::GetObjectClass(obj)));
-
-      JS::Rooted<JSFunction*> toString(cx, JS_NewFunction(cx, InterfaceObjectToString, 0, 0, "toString"));
-      if (!toString) {
-        return false;
-      }
-
-      cacheOnHolder = true;
-
-      FillPropertyDescriptor(desc, wrapper, 0,
-                             JS::ObjectValue(*JS_GetFunctionObject(toString)));
-
-      return JS_WrapPropertyDescriptor(cx, desc);
-    }
   } else {
     MOZ_ASSERT(IsInterfacePrototype(type));
 
     if (IdEquals(id, "constructor")) {
       return nativePropertyHooks->mConstructorID == constructors::id::_ID_Count ||
              ResolvePrototypeOrConstructor(cx, wrapper, obj,
                                            nativePropertyHooks->mConstructorID,
                                            0, desc, cacheOnHolder);
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -3354,12 +3354,16 @@ void
 SetDocumentAndPageUseCounter(JSContext* aCx, JSObject* aObject,
                              UseCounter aUseCounter);
 
 // Warnings
 void
 DeprecationWarning(JSContext* aCx, JSObject* aObject,
                    nsIDocument::DeprecatedOperations aOperation);
 
+// A callback to perform funToString on an interface object
+JSString*
+InterfaceObjectToString(JSContext* aCx, JS::Handle<JSObject*> aObject,
+                        unsigned /* indent */);
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_BindingUtils_h__ */
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -448,17 +448,18 @@ class CGDOMJSClass(CGThing):
                       nullptr, /* getProperty */
                       nullptr, /* setProperty */
                       nullptr, /* getOwnPropertyDescriptor */
                       nullptr, /* deleteProperty */
                       nullptr, /* watch */
                       nullptr, /* unwatch */
                       nullptr, /* getElements */
                       nullptr, /* enumerate */
-                      mozilla::dom::ObjectToOuterObjectValue /* thisValue */
+                      mozilla::dom::ObjectToOuterObjectValue, /* thisValue */
+                      nullptr, /* funToString */
                     }
                     """,
                     objectMoved=objectMovedHook)
         else:
             classFlags += "JSCLASS_HAS_RESERVED_SLOTS(%d)" % slotCount
             reservedSlots = slotCount
         if self.descriptor.interface.getExtendedAttribute("NeedResolve"):
             resolveHook = RESOLVE_HOOK_NAME
@@ -725,17 +726,31 @@ class CGInterfaceObjectJSClass(CGThing):
                 nullptr,               /* mayResolve */
                 nullptr,               /* finalize */
                 ${ctorname}, /* call */
                 ${hasInstance}, /* hasInstance */
                 ${ctorname}, /* construct */
                 nullptr,               /* trace */
                 JS_NULL_CLASS_SPEC,
                 JS_NULL_CLASS_EXT,
-                JS_NULL_OBJECT_OPS
+                {
+                  nullptr, /* lookupProperty */
+                  nullptr, /* defineProperty */
+                  nullptr, /* hasProperty */
+                  nullptr, /* getProperty */
+                  nullptr, /* setProperty */
+                  nullptr, /* getOwnPropertyDescriptor */
+                  nullptr, /* deleteProperty */
+                  nullptr, /* watch */
+                  nullptr, /* unwatch */
+                  nullptr, /* getElements */
+                  nullptr, /* enumerate */
+                  nullptr, /* thisObject */
+                  InterfaceObjectToString, /* funToString */
+                }
               },
               eInterface,
               ${hooks},
               "function ${name}() {\\n    [native code]\\n}",
               ${prototypeID},
               ${depth},
               ${protoGetter}
             };
--- a/dom/bindings/test/test_interfaceToString.html
+++ b/dom/bindings/test/test_interfaceToString.html
@@ -26,13 +26,22 @@ try {
     is(eventTargetToString, nativeToString,
        "Stringifying a DOM interface object should return the same string" +
        "as stringifying a native function.");
 }
 catch (e) {
     ok(false, "Stringifying a DOM interface object shouldn't throw.");
 }
 
+try {
+    eventTargetToString = Function.prototype.toString.call(EventTarget);
+    is(eventTargetToString, nativeToString,
+       "Stringifying a DOM interface object via Function.prototype.toString " +
+       "should return the same string as stringifying a native function.");
+}
+catch (e) {
+    ok(false, "Stringifying a DOM interface object shouldn't throw.");
+}
 
 </script>
 </pre>
 </body>
 </html>
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -319,16 +319,24 @@ typedef bool
 /**
  * The old-style JSClass.enumerate op should define all lazy properties not
  * yet reflected in obj.
  */
 typedef bool
 (* JSEnumerateOp)(JSContext* cx, JS::HandleObject obj);
 
 /**
+ * The type of ObjectOps::funToString.  This callback allows an object to
+ * provide a custom string to use when Function.prototype.toString is invoked on
+ * that object.  A null return value means OOM.
+ */
+typedef JSString*
+(* JSFunToStringOp)(JSContext* cx, JS::HandleObject obj, unsigned indent);
+
+/**
  * Resolve a lazy property named by id in obj by defining it directly in obj.
  * Lazy properties are those reflected from some peer native property space
  * (e.g., the DOM attributes for a given node reflected as obj) on demand.
  *
  * JS looks for a property in an object, and if not found, tries to resolve
  * the given id. *resolvedp should be set to true iff the property was
  * was defined on |obj|.
  */
@@ -645,32 +653,33 @@ struct ObjectOps
     SetPropertyOp       setProperty;
     GetOwnPropertyOp    getOwnPropertyDescriptor;
     DeletePropertyOp    deleteProperty;
     WatchOp             watch;
     UnwatchOp           unwatch;
     GetElementsOp       getElements;
     JSNewEnumerateOp    enumerate;
     ThisValueOp         thisValue;
+    JSFunToStringOp     funToString;
 };
 
 #define JS_NULL_OBJECT_OPS                                                    \
     {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,  \
      nullptr, nullptr, nullptr, nullptr}
 
 } // namespace js
 
 // Classes, objects, and properties.
 
 typedef void (*JSClassInternal)();
 
 struct JSClass {
     JS_CLASS_MEMBERS(JSFinalizeOp);
 
-    void*               reserved[25];
+    void*               reserved[26];
 };
 
 #define JSCLASS_HAS_PRIVATE             (1<<0)  // objects have private slot
 #define JSCLASS_DELAY_METADATA_CALLBACK (1<<1)  // class's initialization code
                                                 // will call
                                                 // SetNewObjectMetadata itself
 #define JSCLASS_PRIVATE_IS_NSISUPPORTS  (1<<3)  // private is (nsISupports*)
 #define JSCLASS_IS_DOMJSCLASS           (1<<4)  // objects are DOM
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -349,16 +349,17 @@ namespace js {
             js::proxy_GetProperty,                                                      \
             js::proxy_SetProperty,                                                      \
             js::proxy_GetOwnPropertyDescriptor,                                         \
             js::proxy_DeleteProperty,                                                   \
             js::proxy_Watch, js::proxy_Unwatch,                                         \
             js::proxy_GetElements,                                                      \
             nullptr,             /* enumerate       */                                  \
             nullptr,             /* thisObject      */                                  \
+            js::proxy_FunToString,                                                      \
         }                                                                               \
     }
 
 #define PROXY_CLASS_DEF(name, flags)                                    \
   PROXY_CLASS_WITH_EXT(name, flags,                                     \
                        PROXY_MAKE_EXT(                                  \
                          nullptr, /* outerObject */                     \
                          nullptr, /* innerObject */                     \
@@ -414,16 +415,18 @@ extern JS_FRIEND_API(JSObject*)
 proxy_innerObject(JSObject* obj);
 extern JS_FRIEND_API(bool)
 proxy_Watch(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::HandleObject callable);
 extern JS_FRIEND_API(bool)
 proxy_Unwatch(JSContext* cx, JS::HandleObject obj, JS::HandleId id);
 extern JS_FRIEND_API(bool)
 proxy_GetElements(JSContext* cx, JS::HandleObject proxy, uint32_t begin, uint32_t end,
                   ElementAdder* adder);
+extern JS_FRIEND_API(JSString*)
+proxy_FunToString(JSContext* cx, JS::HandleObject proxy, unsigned indent);
 
 /**
  * A class of objects that return source code on demand.
  *
  * When code is compiled with setSourceIsLazy(true), SpiderMonkey doesn't
  * retain the source code (and doesn't do lazy bytecode generation). If we ever
  * need the source code, say, in response to a call to Function.prototype.
  * toSource or Debugger.Source.prototype.text, then we call the 'load' member
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1103,18 +1103,19 @@ js::FunctionToString(JSContext* cx, Hand
     }
     return out.finishString();
 }
 
 JSString*
 fun_toStringHelper(JSContext* cx, HandleObject obj, unsigned indent)
 {
     if (!obj->is<JSFunction>()) {
-        if (obj->is<ProxyObject>())
-            return Proxy::fun_toString(cx, obj, indent);
+        if (JSFunToStringOp op = obj->getOps()->funToString)
+            return op(cx, obj, indent);
+
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                              JSMSG_INCOMPATIBLE_PROTO,
                              js_Function_str, js_toString_str,
                              "object");
         return nullptr;
     }
 
     RootedFunction fun(cx, &obj->as<JSFunction>());
--- a/js/src/proxy/Proxy.cpp
+++ b/js/src/proxy/Proxy.cpp
@@ -733,16 +733,22 @@ js::proxy_Unwatch(JSContext* cx, HandleO
 
 bool
 js::proxy_GetElements(JSContext* cx, HandleObject proxy, uint32_t begin, uint32_t end,
                       ElementAdder* adder)
 {
     return Proxy::getElements(cx, proxy, begin, end, adder);
 }
 
+JSString*
+js::proxy_FunToString(JSContext* cx, HandleObject proxy, unsigned indent)
+{
+    return Proxy::fun_toString(cx, proxy, indent);
+}
+
 const Class js::ProxyObject::class_ =
     PROXY_CLASS_DEF("Proxy", JSCLASS_HAS_CACHED_PROTO(JSProto_Proxy));
 
 const Class* const js::ProxyClassPtr = &js::ProxyObject::class_;
 
 JS_FRIEND_API(JSObject*)
 js::NewProxyObject(JSContext* cx, const BaseProxyHandler* handler, HandleValue priv, JSObject* proto_,
                    const ProxyOptions& options)
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -661,16 +661,17 @@ const XPCWrappedNativeJSClass XPC_WN_NoH
         nullptr, // getProperty
         nullptr, // setProperty
         nullptr, // getOwnPropertyDescriptor
         nullptr, // deleteProperty
         nullptr, nullptr, // watch/unwatch
         nullptr, // getElements
         nullptr, // enumerate
         XPC_WN_JSOp_ThisValue,
+        nullptr, // funToString
     }
   }
 };
 
 
 /***************************************************************************/
 
 static bool