Bug 1067501 - Make stringification of DOM Xrays use Object.prototype.toString. r=bholley.

--HG--
extra : rebase_source : 7ba38f2b2625d0ff5405eda2fda6bad9608efa34

dom/bindings/BindingUtils.cpp

@@ -343,53 +343,40 @@ DefineUnforgeableAttributes(JSContext* cx, JS::Handle<JSObject*> obj,
 // 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
@@ -465,25 +452,12 @@ CreateInterfaceObject(JSContext* cx, JS::Handle<JSObject*> global,
     // 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;
@@ -1283,12 +1257,30 @@ XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           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")) {

dom/tests/mochitest/chrome/test_sandbox_bindings.xul

@@ -70,9 +70,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
       }
       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);
@@ -83,8 +83,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest.prototype", sandbox);
         xhr.constructor = "ok";
-      } catch (e) {
         is(xhr.constructor, "ok", "'XMLHttpRequest.prototype.constructor' should be writeable");
+      } catch (e) {
+        ok(false, "'XMLHttpRequest.prototype.constructor' should be writeable");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest.prototype", sandbox);
@@ -109,7 +110,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
       }
       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)");
       }
@@ -136,14 +137,13 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
       } 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);
@@ -155,7 +155,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
         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);

js/xpconnect/tests/chrome/test_nodelists.xul

@@ -21,17 +21,27 @@
         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>

js/xpconnect/wrappers/XrayWrapper.cpp

@@ -1066,6 +1066,9 @@ XPCWrappedNativeXrayTraits::preserveWrapper(JSObject *target)
         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,
@@ -1139,8 +1142,21 @@ XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, HandleObject wr
         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);
@@ -1448,6 +1464,18 @@ DOMXrayTraits::resolveNativeProperty(JSContext *cx, HandleObject 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;
@@ -1702,8 +1730,16 @@ XrayToString(JSContext *cx, unsigned argc, Value *vp)
 
     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");
@@ -1869,21 +1905,6 @@ XrayWrapper<Base, Traits>::getPropertyDescriptor(JSContext *cx, HandleObject wra
         }
     }
 
-    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,