Bug 1067501 - Make stringification of DOM Xrays use Object.prototype.toString. r=bholley.
authorPeter Van der Beken <peterv@propagandism.org>
Mon, 15 Sep 2014 16:45:38 +0200
changeset 207604 8c47153a1dd1
parent 207603 2f0c68d4d7ca
child 207605 adfe4ac95437
push id49735
push userpvanderbeken@mozilla.com
push dateMon, 29 Sep 2014 07:58:28 +0000
treeherdermozilla-inbound@090b62fdfd21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1067501
milestone35.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 1067501 - Make stringification of DOM Xrays use Object.prototype.toString. r=bholley.
dom/bindings/BindingUtils.cpp
dom/tests/mochitest/chrome/test_sandbox_bindings.xul
js/xpconnect/tests/chrome/test_nodelists.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -338,63 +338,50 @@ DefineUnforgeableAttributes(JSContext* c
 
 
 // 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.
 
-enum {
-  TOSTRING_CLASS_RESERVED_SLOT = 0,
-  TOSTRING_NAME_RESERVED_SLOT = 1
-};
-
 static bool
 InterfaceObjectToString(JSContext* cx, unsigned argc, JS::Value *vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  JS::Rooted<JSObject*> callee(cx, &args.callee());
-
   if (!args.thisv().isObject()) {
     JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr,
                          JSMSG_CANT_CONVERT_TO, "null", "object");
     return false;
   }
 
-  JS::Value v = js::GetFunctionNativeReserved(callee,
-                                              TOSTRING_CLASS_RESERVED_SLOT);
-  const JSClass* clasp = static_cast<const JSClass*>(v.toPrivate());
-
-  v = js::GetFunctionNativeReserved(callee, TOSTRING_NAME_RESERVED_SLOT);
-  JSString* jsname = v.toString();
-
-  nsAutoJSString name;
-  if (!name.init(cx, jsname)) {
+  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;
   }
 
-  if (js::GetObjectJSClass(&args.thisv().toObject()) != clasp) {
-    JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr,
-                         JSMSG_INCOMPATIBLE_PROTO,
-                         NS_ConvertUTF16toUTF8(name).get(), "toString",
-                         "object");
+  const js::Class* clasp = js::GetObjectClass(obj);
+  if (!IsDOMIfaceAndProtoClass(clasp)) {
+    JS_ReportError(cx, "toString called on incompatible object");
     return false;
   }
 
-  nsString str;
-  str.AppendLiteral("function ");
-  str.Append(name);
-  str.AppendLiteral("() {");
-  str.Append('\n');
-  str.AppendLiteral("    [native code]");
-  str.Append('\n');
-  str.Append('}');
+  const DOMIfaceAndProtoJSClass* ifaceAndProtoJSClass =
+    DOMIfaceAndProtoJSClass::FromJSClass(clasp);
+  JS::Rooted<JSString*> str(cx,
+                            JS_NewStringCopyZ(cx,
+                                              ifaceAndProtoJSClass->mToString));
+  if (!str) {
+    return false;
+  }
 
-  return xpc::NonVoidStringToJsval(cx, str, args.rval());
+  args.rval().setString(str);
+  return true;
 }
 
 bool
 Constructor(JSContext* cx, unsigned argc, JS::Value* vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JS::Value& v =
     js::GetFunctionNativeReserved(&args.callee(),
@@ -460,35 +447,22 @@ CreateInterfaceObject(JSContext* cx, JS:
   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::DefineFunctionWithReserved(cx, constructor,
-                                     "toString",
-                                     InterfaceObjectToString,
-                                     0, 0));
+      JS_DefineFunction(cx, constructor, "toString", InterfaceObjectToString,
+                        0, 0));
     if (!toString) {
       return nullptr;
     }
 
-    JSString *str = ::JS_InternString(cx, name);
-    if (!str) {
-      return nullptr;
-    }
-    JSObject* toStringObj = JS_GetFunctionObject(toString);
-    js::SetFunctionNativeReserved(toStringObj, TOSTRING_CLASS_RESERVED_SLOT,
-                                  PRIVATE_TO_JSVAL(const_cast<JSClass *>(constructorClass)));
-
-    js::SetFunctionNativeReserved(toStringObj, TOSTRING_NAME_RESERVED_SLOT,
-                                  STRING_TO_JSVAL(str));
-
     if (!JS_DefineProperty(cx, constructor, "length", ctorNargs,
                            JSPROP_READONLY | JSPROP_PERMANENT)) {
       return nullptr;
     }
   }
 
   if (properties) {
     if (properties->staticMethods &&
@@ -1278,22 +1252,40 @@ ResolvePrototypeOrConstructor(JSContext*
 /* static */ bool
 XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           const NativePropertyHooks* nativePropertyHooks,
                           DOMObjectType type, JS::Handle<JSObject*> obj,
                           JS::Handle<jsid> id,
                           JS::MutableHandle<JSPropertyDescriptor> desc,
                           bool& cacheOnHolder)
 {
-  if (type == eInterface && IdEquals(id, "prototype")) {
-    return nativePropertyHooks->mPrototypeID == prototypes::id::_ID_Count ||
-           ResolvePrototypeOrConstructor(cx, wrapper, obj,
-                                         nativePropertyHooks->mPrototypeID,
-                                         JSPROP_PERMANENT | JSPROP_READONLY,
-                                         desc, cacheOnHolder);
+  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, wrapper, "toString"));
+      if (!toString) {
+        return false;
+      }
+
+      cacheOnHolder = true;
+
+      FillPropertyDescriptor(desc, wrapper, 0,
+                             JS::ObjectValue(*JS_GetFunctionObject(toString)));
+
+      return JS_WrapPropertyDescriptor(cx, desc);
+    }
   }
 
   if (type == eInterfacePrototype && IdEquals(id, "constructor")) {
     return nativePropertyHooks->mConstructorID == constructors::id::_ID_Count ||
            ResolvePrototypeOrConstructor(cx, wrapper, obj,
                                          nativePropertyHooks->mConstructorID,
                                          0, desc, cacheOnHolder);
   }
--- a/dom/tests/mochitest/chrome/test_sandbox_bindings.xul
+++ b/dom/tests/mochitest/chrome/test_sandbox_bindings.xul
@@ -65,31 +65,32 @@ https://bugzilla.mozilla.org/show_bug.cg
         var img = Components.utils.evalInSandbox("Image.prototype", sandbox);
         ok(img, "'Image.prototype' in a sandbox should return the interface prototype object");
         ok(isXrayWrapper(img), "Getting an interface prototype object on an Xray wrapper should return an Xray wrapper");
       } catch (e) {
         ok(false, "'Image.prototype' shouldn't throw in a sandbox");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest", sandbox);
-        xhr.prototype = false;
-      } catch (e) {
-        ok(true, "'XMLHttpRequest.prototype' should be readonly");
+        xhr.prototype = "notok";
+      } finally {
+        isnot(xhr.prototype, "notok", "'XMLHttpRequest.prototype' should be readonly");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest", sandbox);
         delete xhr.prototype;
       } catch (e) {
         ok(true, "'XMLHttpRequest.prototype' should be permanent");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest.prototype", sandbox);
         xhr.constructor = "ok";
+        is(xhr.constructor, "ok", "'XMLHttpRequest.prototype.constructor' should be writeable");
       } catch (e) {
-        is(xhr.constructor, "ok", "'XMLHttpRequest.prototype.constructor' should be writeable");
+        ok(false, "'XMLHttpRequest.prototype.constructor' should be writeable");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest.prototype", sandbox);
         delete xhr.constructor;
       } catch (e) {
         is(xhr.constructor, undefined, "'XMLHttpRequest.prototype.constructor' should be permanent");
       }
       try {
@@ -104,17 +105,17 @@ https://bugzilla.mozilla.org/show_bug.cg
               "We should claim to have a DONE constant");
         isnot(Object.getOwnPropertyDescriptor(xhr, "prototype"), undefined,
               "We should claim to have 'prototype' property");
       } catch (e) {
         ok(false, "'XMLHttpRequest' shouldn't throw in a sandbox");
       }
       try {
         var xhr = Components.utils.evalInSandbox("new XMLHttpRequest()", sandbox);
-        is("" + xhr, new XMLHttpRequest() + "", "'XMLHttpRequest()' in a sandbox should create an XMLHttpRequest object");
+        is("" + xhr, new XMLHttpRequest() + "", "'new XMLHttpRequest()' in a sandbox should create an XMLHttpRequest object");
       } catch (e) {
         ok(false, "'new XMLHttpRequest()' shouldn't throw in a sandbox (1)");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest.prototype.toString = function () { return 'Failed'; }; new XMLHttpRequest();", sandbox);
         is(xhr.toString(), new XMLHttpRequest() + "", "XMLHttpRequest.prototype.toString in the sandbox should not override the native toString behaviour");
       } catch (e) {
         ok(false, "'new XMLHttpRequest()' shouldn't throw in a sandbox (2)");
@@ -131,36 +132,35 @@ https://bugzilla.mozilla.org/show_bug.cg
         isnot(props.indexOf("dispatchEvent"), -1,
            "'dispatchEvent' property should be enumerable on XMLHttpRequest.prototype");
         props = Object.getOwnPropertyNames(proto);
         is(props.indexOf("dispatchEvent"), -1,
            "'dispatchEvent' is not an own property on XMLHttpRequest.prototype; it's on EventTarget.prototype")
       } catch (e) {
         ok(false, "XMLHttpRequest.prototype manipulation via an Xray shouldn't throw" + e);
       }
-
       try {
         Components.utils.evalInSandbox("document.defaultView.XMLHttpRequest = function() {};", sandbox);
         var win = Components.utils.evalInSandbox("document.defaultView", sandbox);
         var xhr = new win.XMLHttpRequest();
-        is("" + xhr, new XMLHttpRequest() + "", "'XMLHttpRequest()' in a sandbox should create an XMLHttpRequest object");
+        is("" + xhr, new XMLHttpRequest() + "", "'new XMLHttpRequest()' in a sandbox should create an XMLHttpRequest object");
       } catch (e) {
-        ok(false, "'XMLHttpRequest()' shouldn't throw in a sandbox");
+        ok(false, "'new XMLHttpRequest()' shouldn't throw in a sandbox");
       }
       try {
         var canvas = Components.utils.evalInSandbox("document.createElement('canvas').getContext('2d')", sandbox);
         is(canvas.DRAWWINDOW_DRAW_CARET, CanvasRenderingContext2D.DRAWWINDOW_DRAW_CARET, "Constants should be defined on DOM objects in a sandbox");
       } catch (e) {
         ok(false, "'document.createElement('canvas').getContext('2D')' shouldn't throw in a sandbox");
       }
       try {
         var classList = Components.utils.evalInSandbox("document.body.className = 'a b'; document.body.classList", sandbox);
         is(classList.toString(), "a b", "Stringifier should be called");
       } catch (e) {
-        ok(false, "'document.createElement('canvas').getContext('2D')' shouldn't throw in a sandbox");
+        ok(false, "Stringifying shouldn't throw in a sandbox");
       }
       try {
         var ctx = Components.utils.evalInSandbox("var ctx = document.createElement('canvas').getContext('2d'); ctx.foopy = 5; ctx", sandbox);
         ok(!("foopy" in ctx), "We should have an Xray here");
         var data = ctx.createImageData(1, 1);
         for (var i = 0; i < data.data.length; ++i) {
           // Watch out for premultiplied bits... just set all the alphas to 255
           if (i % 4 == 3) {
--- a/js/xpconnect/tests/chrome/test_nodelists.xul
+++ b/js/xpconnect/tests/chrome/test_nodelists.xul
@@ -16,27 +16,37 @@
   <!-- test code goes here -->
   <script type="application/javascript"><![CDATA[
       SimpleTest.waitForExplicitFinish();
 
       function go() {
         var win = $('ifr').contentWindow;
         var list = win.document.getElementsByTagName('p');
         is(list.length, 3, "can get the length");
-        ok(list[0].toString().indexOf("[object HTMLParagraphElement") >= 0, "can get list[0]");
+        ok(list[0] instanceof HTMLParagraphElement, "can get list[0]");
         is(list[0], list.item(0), "list.item works");
         is(list.item, list.item, "don't recreate functions for each get");
 
         var list2 = list[2];
-        ok(list[2].toString().indexOf("[object HTMLParagraphElement") >= 0, "list[2] exists");
+        ok(list[2] instanceof HTMLParagraphElement, "list[2] exists");
         ok("2" in list, "in operator works");
 
         is(win.document.body.removeChild(win.document.body.lastChild), list2, "remove last paragraph element");
         ok(!("2" in list), "in operator doesn't see phantom element");
         is(list[2], undefined, "no node there!");
+
+        var optionList = win.document.createElement("select").options;
+        var option = win.document.createElement("option");
+        optionList[0] = option;
+        is(optionList.item(0), option, "Creators work");
+
+        option = win.document.createElement("option");
+        optionList[0] = option;
+        is(optionList.item(0), option, "Setters work");
+
         SimpleTest.finish();
       }
   ]]></script>
 
   <iframe id="ifr"
           src="http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/file_nodelists.html"
           onload="go()" />
 </window>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1061,16 +1061,19 @@ XPCWrappedNativeXrayTraits::preserveWrap
 {
     XPCWrappedNative *wn = XPCWrappedNative::Get(target);
     nsRefPtr<nsXPCClassInfo> ci;
     CallQueryInterface(wn->Native(), getter_AddRefs(ci));
     if (ci)
         ci->PreserveWrapper(wn->Native());
 }
 
+static bool
+XrayToString(JSContext *cx, unsigned argc, JS::Value *vp);
+
 bool
 XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, HandleObject wrapper,
                                                   HandleObject holder, HandleId id,
                                                   MutableHandle<JSPropertyDescriptor> desc)
 {
     MOZ_ASSERT(js::GetObjectJSClass(holder) == &HolderClass);
 
     desc.object().set(nullptr);
@@ -1134,18 +1137,31 @@ XPCWrappedNativeXrayTraits::resolveNativ
     XPCNativeInterface *iface;
     XPCNativeMember *member;
     XPCWrappedNative *wn = getWN(wrapper);
 
     if (ccx.GetWrapper() != wn || !wn->IsValid()) {
         return true;
     }
 
-    if (!(iface = ccx.GetInterface()) || !(member = ccx.GetMember()))
-        return true;
+    if (!(iface = ccx.GetInterface()) || !(member = ccx.GetMember())) {
+        if (id != nsXPConnect::GetRuntimeInstance()->GetStringID(XPCJSRuntime::IDX_TO_STRING))
+            return true;
+
+        JSFunction *toString = JS_NewFunction(cx, XrayToString, 0, 0, holder, "toString");
+        if (!toString)
+            return false;
+
+        FillPropertyDescriptor(desc, wrapper, 0,
+                               ObjectValue(*JS_GetFunctionObject(toString)));
+
+        return JS_DefinePropertyById(cx, holder, id, desc.value(), desc.attributes(),
+                                     desc.getter(), desc.setter()) &&
+               JS_GetPropertyDescriptorById(cx, holder, id, desc);
+    }
 
     desc.object().set(holder);
     desc.setAttributes(JSPROP_ENUMERATE);
     desc.setGetter(nullptr);
     desc.setSetter(nullptr);
     desc.value().set(JSVAL_VOID);
 
     RootedValue fval(cx, JSVAL_VOID);
@@ -1443,16 +1459,28 @@ DOMXrayTraits::resolveNativeProperty(JSC
                                      HandleObject holder, HandleId id,
                                      MutableHandle<JSPropertyDescriptor> desc)
 {
     bool unused;
     RootedObject obj(cx, getTargetObject(wrapper));
     if (!XrayResolveNativeProperty(cx, wrapper, obj, id, desc, unused))
         return false;
 
+    if (!desc.object() &&
+        id == nsXPConnect::GetRuntimeInstance()->GetStringID(XPCJSRuntime::IDX_TO_STRING))
+    {
+
+        JSFunction *toString = JS_NewFunction(cx, XrayToString, 0, 0, wrapper, "toString");
+        if (!toString)
+            return false;
+
+        FillPropertyDescriptor(desc, wrapper, 0,
+                               ObjectValue(*JS_GetFunctionObject(toString)));
+    }
+
     MOZ_ASSERT(!desc.object() || desc.object() == wrapper, "What did we resolve this on?");
 
     return true;
 }
 
 bool
 DOMXrayTraits::resolveOwnProperty(JSContext *cx, const Wrapper &jsWrapper, HandleObject wrapper,
                                   HandleObject holder, HandleId id,
@@ -1697,18 +1725,26 @@ XrayToString(JSContext *cx, unsigned arg
     }
     if (!IsWrapper(wrapper) || !WrapperFactory::IsXrayWrapper(wrapper)) {
         JS_ReportError(cx, "XrayToString called on an incompatible object");
         return false;
     }
 
     RootedObject obj(cx, XrayTraits::getTargetObject(wrapper));
     XrayType type = GetXrayType(obj);
-    if (type == XrayForDOMObject)
-        return NativeToString(cx, wrapper, obj, args.rval());
+    if (type == XrayForDOMObject) {
+        {
+            JSAutoCompartment ac(cx, obj);
+            JSString *str = JS_BasicObjectToString(cx, obj);
+            if (!str)
+                return false;
+            args.rval().setString(str);
+        }
+        return JS_WrapValue(cx, args.rval());
+    }
 
     if (type != XrayForWrappedNative) {
         JS_ReportError(cx, "XrayToString called on an incompatible object");
         return false;
     }
 
     static const char start[] = "[object XrayWrapper ";
     static const char end[] = "]";
@@ -1864,31 +1900,16 @@ XrayWrapper<Base, Traits>::getPropertyDe
             if (MOZ_UNLIKELY(!childObj))
                 return xpc::Throw(cx, NS_ERROR_FAILURE);
             FillPropertyDescriptor(desc, wrapper, ObjectValue(*childObj),
                                    /* readOnly = */ true);
             return JS_WrapPropertyDescriptor(cx, desc);
         }
     }
 
-    if (!desc.object() &&
-        id == nsXPConnect::GetRuntimeInstance()->GetStringID(XPCJSRuntime::IDX_TO_STRING))
-    {
-
-        JSFunction *toString = JS_NewFunction(cx, XrayToString, 0, 0, wrapper, "toString");
-        if (!toString)
-            return false;
-
-        desc.object().set(wrapper);
-        desc.setAttributes(0);
-        desc.setGetter(nullptr);
-        desc.setSetter(nullptr);
-        desc.value().setObject(*JS_GetFunctionObject(toString));
-    }
-
     // If we're a special scope for in-content XBL, our script expects to see
     // the bound XBL methods and attributes when accessing content. However,
     // these members are implemented in content via custom-spliced prototypes,
     // and thus aren't visible through Xray wrappers unless we handle them
     // explicitly. So we check if we're running in such a scope, and if so,
     // whether the wrappee is a bound element. If it is, we do a lookup via
     // specialized XBL machinery.
     //