Bug 972987 - Implement Xrays to self-hosted methods and properties. r=till,gabor
authorBobby Holley <bobbyholley@gmail.com>
Mon, 02 Jun 2014 13:55:20 -0700
changeset 186207 39034630898e26c2e214b1562138cc7aa54f6a23
parent 186193 d74a2f6f81eea17282835eb8d8a85ce10c2fa0d6
child 186208 ae6bd0223b532929c93a9b3ee9aaed4f134b513a
push id26884
push usercbook@mozilla.com
push dateTue, 03 Jun 2014 12:40:39 +0000
treeherdermozilla-central@caff98d085ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill, gabor
bugs972987
milestone32.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 972987 - Implement Xrays to self-hosted methods and properties. r=till,gabor
js/xpconnect/tests/chrome/test_xrayToJS.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/xpconnect/tests/chrome/test_xrayToJS.xul
+++ b/js/xpconnect/tests/chrome/test_xrayToJS.xul
@@ -152,19 +152,24 @@ https://bugzilla.mozilla.org/show_bug.cg
     xrayProto.expando = 42;
     is(xray.expando, 42, "Xrayed instances see proto expandos");
     is(xray2.expando, 42, "Xrayed instances see proto expandos");
   }
 
   function testDate() {
     // toGMTString is handled oddly in the engine. We don't bother to support
     // it over Xrays.
-    //
-    // We don't yet support self-hosted functions on Xrays. See bug 972987.
-    let propsToSkip = ['toGMTString', 'toLocaleString',
-                       'toLocaleDateString', 'toLocaleTimeString'];
+    let propsToSkip = ['toGMTString'];
+
     testXray('Date', new iwin.Date(), new iwin.Date(), propsToSkip);
+
+    // Test the self-hosted toLocaleString.
+    var d = new iwin.Date();
+    isnot(d.toLocaleString, Cu.unwaiveXrays(d.wrappedJSObject.toLocaleString), "Different function identities");
+    is(Cu.getGlobalForObject(d.toLocaleString), window, "Xray global is correct");
+    is(Cu.getGlobalForObject(d.wrappedJSObject.toLocaleString), iwin, "Underlying global is correct");
+    is(d.toLocaleString('de-DE'), d.wrappedJSObject.toLocaleString('de-DE'), "Results match");
   }
 
   ]]>
   </script>
   <iframe id="ifr" onload="go();" src="http://example.org/tests/js/xpconnect/tests/mochitest/file_empty.html" />
 </window>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -447,65 +447,87 @@ JSXrayTraits::resolveOwnProperty(JSConte
     // properties when we start supporting arrays.
     if (!JSID_IS_STRING(id))
         return true;
     Rooted<JSFlatString*> str(cx, JSID_TO_FLAT_STRING(id));
 
     // Scan through the functions.
     const JSFunctionSpec *fsMatch = nullptr;
     for (const JSFunctionSpec *fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) {
-        // We don't support self-hosted functions yet. See bug 972987.
-        if (fs->selfHostedName)
-            continue;
         if (JS_FlatStringEqualsAscii(str, fs->name)) {
             fsMatch = fs;
             break;
         }
     }
     if (fsMatch) {
         // Generate an Xrayed version of the method.
-        Rooted<JSFunction*> fun(cx, JS_NewFunctionById(cx, fsMatch->call.op, fsMatch->nargs,
-                                                       0, wrapper, id));
+        RootedFunction fun(cx);
+        if (fsMatch->selfHostedName) {
+            fun = JS::GetSelfHostedFunction(cx, fsMatch->selfHostedName, id, fsMatch->nargs);
+        } else {
+            fun = JS_NewFunctionById(cx, fsMatch->call.op, fsMatch->nargs,
+                                     0, wrapper, id);
+        }
         if (!fun)
             return false;
 
         // The generic Xray machinery only defines non-own properties on the holder.
         // This is broken, and will be fixed at some point, but for now we need to
         // cache the value explicitly. See the corresponding call to
         // JS_GetPropertyById at the top of this function.
         RootedObject funObj(cx, JS_GetFunctionObject(fun));
         return JS_DefinePropertyById(cx, holder, id, funObj, 0) &&
                JS_GetPropertyDescriptorById(cx, holder, id, desc);
     }
 
     // Scan through the properties.
     const JSPropertySpec *psMatch = nullptr;
     for (const JSPropertySpec *ps = clasp->spec.prototypeProperties; ps && ps->name; ++ps) {
-        // We don't support self-hosted accessors yet (see bug 972987). And given
-        // the confusion outlined in bug 992977, we can't support JSPropertyOp-
-        // backed entries either (which in practice is fine).
-        if (!(ps->flags & JSPROP_NATIVE_ACCESSORS))
-            continue;
         if (JS_FlatStringEqualsAscii(str, ps->name)) {
             psMatch = ps;
             break;
         }
     }
     if (psMatch) {
+        desc.value().setUndefined();
+        // Note that this is also kind of an abuse of JSPROP_NATIVE_ACCESSORS.
+        // See bug 992977.
+        RootedFunction getterObj(cx);
+        RootedFunction setterObj(cx);
+        unsigned flags = psMatch->flags;
+        if (flags & JSPROP_NATIVE_ACCESSORS) {
+            desc.setGetter(psMatch->getter.propertyOp.op);
+            desc.setSetter(psMatch->setter.propertyOp.op);
+        } else {
+            MOZ_ASSERT(flags & JSPROP_GETTER);
+            getterObj = JS::GetSelfHostedFunction(cx, psMatch->getter.selfHosted.funname, id, 0);
+            if (!getterObj)
+                return false;
+            desc.setGetterObject(JS_GetFunctionObject(getterObj));
+            if (psMatch->setter.selfHosted.funname) {
+                MOZ_ASSERT(flags & JSPROP_SETTER);
+                setterObj = JS::GetSelfHostedFunction(cx, psMatch->setter.selfHosted.funname, id, 0);
+                if (!setterObj)
+                    return false;
+                desc.setSetterObject(JS_GetFunctionObject(setterObj));
+            }
+        }
+        desc.setAttributes(flags);
+
         // The generic Xray machinery only defines non-own properties on the holder.
         // This is broken, and will be fixed at some point, but for now we need to
         // cache the value explicitly. See the corresponding call to
         // JS_GetPropertyById at the top of this function.
         //
         // Note also that the public-facing API here doesn't give us a way to
         // pass along JITInfo. It's probably ok though, since Xrays are already
         // pretty slow.
         return JS_DefinePropertyById(cx, holder, id,
-                                     UndefinedHandleValue, psMatch->flags,
-                                     psMatch->getter.propertyOp.op, psMatch->setter.propertyOp.op) &&
+                                     UndefinedHandleValue, desc.attributes(),
+                                     desc.getter(), desc.setter()) &&
                JS_GetPropertyDescriptorById(cx, holder, id, desc);
     }
 
     return true;
 }
 
 bool
 JSXrayTraits::enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags,
@@ -522,31 +544,30 @@ JSXrayTraits::enumerateNames(JSContext *
     // Grab the JSClass. We require all Xrayable classes to have a ClassSpec.
     RootedObject target(cx, getTargetObject(wrapper));
     const js::Class *clasp = js::GetObjectClass(target);
     MOZ_ASSERT(JSCLASS_CACHED_PROTO_KEY(clasp) == getProtoKey(holder));
     MOZ_ASSERT(clasp->spec.defined());
 
     // Intern all the strings, and pass theme to the caller.
     for (const JSFunctionSpec *fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) {
-        // We don't support self-hosted functions yet. See bug 972987.
-        if (fs->selfHostedName)
-            continue;
         RootedString str(cx, JS_InternString(cx, fs->name));
         if (!str)
             return false;
         if (!props.append(INTERNED_STRING_TO_JSID(cx, str)))
             return false;
     }
     for (const JSPropertySpec *ps = clasp->spec.prototypeProperties; ps && ps->name; ++ps) {
-        // We don't support self-hosted functions yet. See bug 972987.
-        // Note that this is also kind of an abuse of JSPROP_NATIVE_ACCESSORS.
-        // See bug 992977.
-        if (!(ps->flags & JSPROP_NATIVE_ACCESSORS))
-            continue;
+        // We have code to Xray self-hosted accessors. But at present, there don't appear
+        // to be any self-hosted accessors anywhere in SpiderMonkey, let alone in on an
+        // Xrayable class, so we can't test it. Assert against it to make sure that we get
+        // test coverage in test_XrayToJS.xul when the time comes.
+        MOZ_ASSERT(ps->flags & JSPROP_NATIVE_ACCESSORS,
+                   "Self-hosted accessor added to Xrayable class - ping the XPConnect "
+                   "module owner about adding test coverage");
         RootedString str(cx, JS_InternString(cx, ps->name));
         if (!str)
             return false;
         if (!props.append(INTERNED_STRING_TO_JSID(cx, str)))
             return false;
     }
 
     // Add the 'constructor' property.