Bug 462428 - Make __lookup[GS]etter__ work on quickstubbed properties by faking it for XPConnect prototype objects only. r=jorendorff sr=brendan
authorBlake Kaplan <mrbkap@gmail.com>
Fri, 23 Jan 2009 15:44:01 -0800
changeset 23018 53090b97eecc38912aee912314d443fff006c4ae
parent 23017 92866cc73bde7de35fd7e542c9fff2ca4aa03188
child 23019 0be8c1baabda6f5b00daba67468ae8174d9db010
push id546
push usermrbkap@mozilla.com
push dateTue, 27 Jan 2009 01:45:25 +0000
reviewersjorendorff, brendan
bugs462428
milestone1.9.1b3pre
Bug 462428 - Make __lookup[GS]etter__ work on quickstubbed properties by faking it for XPConnect prototype objects only. r=jorendorff sr=brendan
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsobj.cpp
js/src/xpconnect/src/qsgen.py
js/src/xpconnect/src/xpcquickstubs.cpp
js/src/xpconnect/tests/mochitest/Makefile.in
js/src/xpconnect/tests/mochitest/test_bug462428.html
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3448,16 +3448,28 @@ JS_GetPropertyAttrsGetterAndSetter(JSCon
 {
     CHECK_REQUEST(cx);
     return GetPropertyAttributes(cx, obj,
                                  js_Atomize(cx, name, strlen(name), 0),
                                  attrsp, foundp, getterp, setterp);
 }
 
 JS_PUBLIC_API(JSBool)
+JS_GetPropertyAttrsGetterAndSetterById(JSContext *cx, JSObject *obj,
+                                       jsid id,
+                                       uintN *attrsp, JSBool *foundp,
+                                       JSPropertyOp *getterp,
+                                       JSPropertyOp *setterp)
+{
+    CHECK_REQUEST(cx);
+    return GetPropertyAttributes(cx, obj, JSID_TO_ATOM(id),
+                                 attrsp, foundp, getterp, setterp);
+}
+
+JS_PUBLIC_API(JSBool)
 JS_SetPropertyAttributes(JSContext *cx, JSObject *obj, const char *name,
                          uintN attrs, JSBool *foundp)
 {
     CHECK_REQUEST(cx);
     return SetPropertyAttributes(cx, obj,
                                  js_Atomize(cx, name, strlen(name), 0),
                                  attrs, foundp);
 }
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -1598,16 +1598,23 @@ JS_GetPropertyAttributes(JSContext *cx, 
  */
 extern JS_PUBLIC_API(JSBool)
 JS_GetPropertyAttrsGetterAndSetter(JSContext *cx, JSObject *obj,
                                    const char *name,
                                    uintN *attrsp, JSBool *foundp,
                                    JSPropertyOp *getterp,
                                    JSPropertyOp *setterp);
 
+extern JS_PUBLIC_API(JSBool)
+JS_GetPropertyAttrsGetterAndSetterById(JSContext *cx, JSObject *obj,
+                                       jsid id,
+                                       uintN *attrsp, JSBool *foundp,
+                                       JSPropertyOp *getterp,
+                                       JSPropertyOp *setterp);
+
 /*
  * Set the attributes of a property on a given object.
  *
  * If the object does not have a property by that name, *foundp will be
  * JS_FALSE and nothing will be altered.
  */
 extern JS_PUBLIC_API(JSBool)
 JS_SetPropertyAttributes(JSContext *cx, JSObject *obj, const char *name,
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1670,18 +1670,18 @@ js_PropertyIsEnumerable(JSContext *cx, J
     ok = OBJ_GET_ATTRIBUTES(cx, pobj, id, prop, &attrs);
     OBJ_DROP_PROPERTY(cx, pobj, prop);
     if (ok)
         *vp = BOOLEAN_TO_JSVAL((attrs & JSPROP_ENUMERATE) != 0);
     return ok;
 }
 
 #if JS_HAS_GETTER_SETTER
-static JSBool
-obj_defineGetter(JSContext *cx, uintN argc, jsval *vp)
+JS_FRIEND_API(JSBool)
+js_obj_defineGetter(JSContext *cx, uintN argc, jsval *vp)
 {
     jsval fval, junk;
     jsid id;
     JSObject *obj;
     uintN attrs;
 
     if (argc <= 1 || JS_TypeOfValue(cx, vp[3]) != JSTYPE_FUNCTION) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
@@ -1705,18 +1705,18 @@ obj_defineGetter(JSContext *cx, uintN ar
     *vp = JSVAL_VOID;
     return OBJ_DEFINE_PROPERTY(cx, obj, id, JSVAL_VOID,
                                (JSPropertyOp) JSVAL_TO_OBJECT(fval),
                                JS_PropertyStub,
                                JSPROP_ENUMERATE | JSPROP_GETTER | JSPROP_SHARED,
                                NULL);
 }
 
-static JSBool
-obj_defineSetter(JSContext *cx, uintN argc, jsval *vp)
+JS_FRIEND_API(JSBool)
+js_obj_defineSetter(JSContext *cx, uintN argc, jsval *vp)
 {
     jsval fval, junk;
     jsid id;
     JSObject *obj;
     uintN attrs;
 
     if (argc <= 1 || JS_TypeOfValue(cx, vp[3]) != JSTYPE_FUNCTION) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
@@ -1851,18 +1851,18 @@ static JSFunctionSpec object_methods[] =
     JS_FN(js_unwatch_str,              obj_unwatch,                 1,0),
 #endif
     JS_TN(js_hasOwnProperty_str,       obj_hasOwnProperty,          1,0,
           obj_hasOwnProperty_trcinfo),
     JS_FN(js_isPrototypeOf_str,        obj_isPrototypeOf,           1,0),
     JS_TN(js_propertyIsEnumerable_str, obj_propertyIsEnumerable,    1,0,
           obj_propertyIsEnumerable_trcinfo),
 #if JS_HAS_GETTER_SETTER
-    JS_FN(js_defineGetter_str,         obj_defineGetter,            2,0),
-    JS_FN(js_defineSetter_str,         obj_defineSetter,            2,0),
+    JS_FN(js_defineGetter_str,         js_obj_defineGetter,         2,0),
+    JS_FN(js_defineSetter_str,         js_obj_defineSetter,         2,0),
     JS_FN(js_lookupGetter_str,         obj_lookupGetter,            1,0),
     JS_FN(js_lookupSetter_str,         obj_lookupSetter,            1,0),
 #endif
     JS_FS_END
 };
 
 static JSFunctionSpec object_static_methods[] = {
     JS_FN("getPrototypeOf",            obj_getPrototypeOf,          1,0),
--- a/js/src/xpconnect/src/qsgen.py
+++ b/js/src/xpconnect/src/qsgen.py
@@ -75,20 +75,16 @@
 # - Quick stubs affect the handling of naming conflicts--that is, which C++
 #   method gets called when a script uses an XPCOM feature that is declared in
 #   more than one of the interfaces the object implements.  Without quick
 #   stubs, XPConnect just walks the interfaces in the order they're listed by
 #   nsClassInfo.  You get the first interface that implements a feature with
 #   that name.  With quick stubs, it's the same except that non-quick-stubbed
 #   features are shadowed.
 #
-# - Quick stub getters and setters are JSPropertyOps-- that is, they do not use
-#   JSPROP_GETTER or JSPROP_SETTER.  This means __lookupGetter__ does not work
-#   on them.  This change is visible to scripts.
-#
 # - Quick stub methods are JSFastNative, which means that when a quick stub
 #   method is called, no JS stack frame is created.  This doesn't affect
 #   Mozilla security checks because they look for scripted JSStackFrames, not
 #   native ones.
 #
 #   It does affect the 'stack' property of JavaScript exceptions, though: the
 #   stubbed member will not appear.  (Note that if the stubbed member itself
 #   fails, the member name will appear in the 'message' property.)
@@ -122,16 +118,19 @@
 #   Components object.
 #
 # - Quick stubs skip a security check that XPConnect does in
 #   XPCWrappedNative::CallMethod.  This means the security manager doesn't have
 #   an opportunity to veto accesses to members for which quick stubs exist.
 #
 # - There are many features of IDL that XPConnect supports but qsgen does not,
 #   including dependent types, arrays, and out parameters.
+#
+# - Since quick stubs are JSPropertyOps, we have to do additional work to make
+#   __lookup[GS]etter__ work on them.
 
 
 import xpidl
 import header
 import os, re
 import sys
 import sets
 
--- a/js/src/xpconnect/src/xpcquickstubs.cpp
+++ b/js/src/xpconnect/src/xpcquickstubs.cpp
@@ -95,45 +95,312 @@ LookupInterfaceOrAncestor(PRUint32 table
             if(p)
                 break;
             info.swap(parent);
         }
     }
     return p;
 }
 
+static JSBool
+PropertyOpForwarder(JSContext *cx, uintN argc, jsval *vp)
+{
+    // Layout:
+    //   this = our this
+    //   property op to call = callee reserved slot 0
+    //   name of the property = callee reserved slot 1
+
+    JSObject *callee = JSVAL_TO_OBJECT(JS_CALLEE(cx, vp));
+    JSObject *obj = JS_THIS_OBJECT(cx, vp);
+    jsval v;
+
+    if(!JS_GetReservedSlot(cx, callee, 0, &v))
+        return JS_FALSE;
+    JSObject *ptrobj = JSVAL_TO_OBJECT(v);
+    JSPropertyOp *popp = static_cast<JSPropertyOp *>(JS_GetPrivate(cx, ptrobj));
+
+    if(!JS_GetReservedSlot(cx, callee, 1, &v))
+        return JS_FALSE;
+
+    JS_SET_RVAL(cx, vp, JS_ARGV(cx, vp)[0]);
+    return (*popp)(cx, obj, v, vp);
+}
+
+static void
+PointerFinalize(JSContext *cx, JSObject *obj)
+{
+    JSPropertyOp *popp = static_cast<JSPropertyOp *>(JS_GetPrivate(cx, obj));
+    delete popp;
+}
+
+static JSClass
+PointerHolderClass = {
+    "Pointer", JSCLASS_HAS_PRIVATE,
+    JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_PropertyStub,
+    JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, PointerFinalize,
+    JSCLASS_NO_OPTIONAL_MEMBERS
+};
+
+static JSObject *
+GeneratePropertyOp(JSContext *cx, JSObject *obj, jsval idval, uintN argc,
+                   const char *name, JSPropertyOp pop)
+{
+    // The JS engine provides two reserved slots on function objects for
+    // XPConnect to use. Use them to stick the necessary info here.
+    JSFunction *fun =
+        JS_NewFunction(cx, reinterpret_cast<JSNative>(PropertyOpForwarder),
+                       argc, JSFUN_FAST_NATIVE, obj, name);
+    if(!fun)
+        return JS_FALSE;
+
+    JSObject *funobj = JS_GetFunctionObject(fun);
+
+    JSAutoTempValueRooter tvr(cx, OBJECT_TO_JSVAL(funobj));
+
+    // Unfortunately, we cannot guarantee that JSPropertyOp is aligned. Use a
+    // second object to work around this.
+    JSObject *ptrobj = JS_NewObject(cx, &PointerHolderClass, nsnull, funobj);
+    if(!ptrobj)
+        return JS_FALSE;
+    JSPropertyOp *popp = new JSPropertyOp;
+    if(!popp)
+        return JS_FALSE;
+    *popp = pop;
+    JS_SetPrivate(cx, ptrobj, popp);
+
+    JS_SetReservedSlot(cx, funobj, 0, OBJECT_TO_JSVAL(ptrobj));
+    JS_SetReservedSlot(cx, funobj, 1, idval);
+    return funobj;
+}
+
+static JSBool
+ReifyPropertyOps(JSContext *cx, JSObject *obj, jsval idval, jsid interned_id,
+                 const char *name, JSPropertyOp getter, JSPropertyOp setter,
+                 JSObject **getterobjp, JSObject **setterobjp)
+{
+    // Generate both getter and setter and stash them in the prototype.
+    jsval roots[2] = { JSVAL_NULL, JSVAL_NULL };
+    JSAutoTempValueRooter tvr(cx, 2, roots);
+
+    uintN attrs = JSPROP_SHARED;
+    JSObject *getterobj;
+    if(getter)
+    {
+        getterobj = GeneratePropertyOp(cx, obj, idval, 0, name, getter);
+        if(!getterobj)
+            return JS_FALSE;
+        roots[0] = OBJECT_TO_JSVAL(getterobj);
+        attrs |= JSPROP_GETTER;
+    }
+    else
+        getterobj = nsnull;
+
+    JSObject *setterobj;
+    if (setter)
+    {
+        setterobj = GeneratePropertyOp(cx, obj, idval, 1, name, setter);
+        if(!setterobj)
+            return JS_FALSE;
+        roots[1] = OBJECT_TO_JSVAL(setterobj);
+        attrs |= JSPROP_SETTER;
+    }
+    else
+        setterobj = nsnull;
+
+    if(getterobjp)
+        *getterobjp = getterobj;
+    if(setterobjp)
+        *setterobjp = setterobj;
+    return JS_DefinePropertyById(cx, obj, interned_id, JSVAL_VOID,
+                                 (JSPropertyOp)getterobj,
+                                 (JSPropertyOp)setterobj,
+                                 attrs);
+}
+
+static JSBool
+LookupGetterOrSetter(JSContext *cx, JSBool wantGetter, jsval *vp)
+{
+    uintN attrs;
+    JSBool found;
+    JSPropertyOp getter, setter;
+    JSObject *obj2;
+    jsid interned_id;
+    jsval v;
+
+    XPC_QS_ASSERT_CONTEXT_OK(cx);
+    JSObject *obj = JS_THIS_OBJECT(cx, vp);
+    if (!obj)
+        return JS_FALSE;
+    jsval idval = JS_ARGV(cx, vp)[0];
+
+    const char *name = JSVAL_IS_STRING(idval)
+                       ? JS_GetStringBytes(JSVAL_TO_STRING(idval))
+                       : nsnull;
+    if(!JS_ValueToId(cx, idval, &interned_id) ||
+       !JS_LookupPropertyWithFlagsById(cx, obj, interned_id,
+                                       JSRESOLVE_QUALIFIED, &obj2, &v) ||
+       (obj2 &&
+        !JS_GetPropertyAttrsGetterAndSetterById(cx, obj2, interned_id, &attrs,
+                                                &found, &getter, &setter)))
+        return JS_FALSE;
+
+    // No property at all means no getters or setters possible.
+    if(!obj2 || !found)
+    {
+        JS_SET_RVAL(cx, vp, JSVAL_VOID);
+        return JS_TRUE;
+    }
+
+    // Inline obj_lookup[GS]etter here.
+    if(wantGetter)
+    {
+        if(attrs & JSPROP_GETTER)
+        {
+            JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(getter));
+            return JS_TRUE;
+        }
+    }
+    else
+    {
+        if(attrs & JSPROP_SETTER)
+        {
+            JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(setter));
+            return JS_TRUE;
+        }
+    }
+
+    // Since XPConnect doesn't use JSPropertyOps in any other contexts,
+    // ensuring that we have an XPConnect prototype object ensures that
+    // we are only going to expose quickstubbed properties to script.
+    // Also be careful not to overwrite existing properties!
+    if(!name ||
+       !IS_PROTO_CLASS(STOBJ_GET_CLASS(obj2)) ||
+       (attrs & (JSPROP_GETTER | JSPROP_SETTER)) ||
+       !(getter || setter))
+    {
+        JS_SET_RVAL(cx, vp, JSVAL_VOID);
+        return JS_TRUE;
+    }
+
+    JSObject *getterobj, *setterobj;
+    if(!ReifyPropertyOps(cx, obj, idval, interned_id, name, getter, setter,
+                         &getterobj, &setterobj))
+        return JS_FALSE;
+
+    JSObject *wantedobj = wantGetter ? getterobj : setterobj;
+    v = wantedobj ? OBJECT_TO_JSVAL(wantedobj) : JSVAL_VOID;
+    JS_SET_RVAL(cx, vp, v);
+    return JS_TRUE;
+}
+
+static JSBool
+SharedLookupGetter(JSContext *cx, uintN argc, jsval *vp)
+{
+    return LookupGetterOrSetter(cx, PR_TRUE, vp);
+}
+
+static JSBool
+SharedLookupSetter(JSContext *cx, uintN argc, jsval *vp)
+{
+    return LookupGetterOrSetter(cx, PR_FALSE, vp);
+}
+
+// XXX Hack! :-/
+JS_FRIEND_API(JSBool) js_obj_defineGetter(JSContext *cx, uintN argc, jsval *vp);
+JS_FRIEND_API(JSBool) js_obj_defineSetter(JSContext *cx, uintN argc, jsval *vp);
+
+static JSBool
+DefineGetterOrSetter(JSContext *cx, uintN argc, JSBool wantGetter, jsval *vp)
+{
+    uintN attrs;
+    JSBool found;
+    JSPropertyOp getter, setter;
+    JSObject *obj2;
+    jsval v;
+    jsid interned_id;
+
+    XPC_QS_ASSERT_CONTEXT_OK(cx);
+    JSObject *obj = JS_THIS_OBJECT(cx, vp);
+    if (!obj)
+        return JS_FALSE;
+    JSFastNative forward = wantGetter ? js_obj_defineGetter : js_obj_defineSetter;
+    jsval id = (argc >= 1) ? JS_ARGV(cx, vp)[0] : JSVAL_VOID;
+    if(!JSVAL_IS_STRING(id))
+        return forward(cx, argc, vp);
+    JSString *str = JSVAL_TO_STRING(id);
+
+    const char *name = JS_GetStringBytes(str);
+    if(!JS_ValueToId(cx, id, &interned_id) ||
+       !JS_LookupPropertyWithFlagsById(cx, obj, interned_id,
+                                       JSRESOLVE_QUALIFIED, &obj2, &v) ||
+       (obj2 &&
+        !JS_GetPropertyAttrsGetterAndSetterById(cx, obj2, interned_id, &attrs,
+                                                &found, &getter, &setter)))
+        return JS_FALSE;
+
+    // The property didn't exist, already has a getter or setter, or is not
+    // our property, then just forward now.
+    if(!obj2 ||
+       (attrs & (JSPROP_GETTER | JSPROP_SETTER)) ||
+       !(getter || setter) ||
+       !IS_PROTO_CLASS(STOBJ_GET_CLASS(obj2)))
+        return forward(cx, argc, vp);
+
+    // Reify the getter and setter...
+    if(!ReifyPropertyOps(cx, obj, id, interned_id, name, getter, setter,
+                         nsnull, nsnull))
+        return JS_FALSE;
+
+    return forward(cx, argc, vp);
+}
+
+static JSBool
+SharedDefineGetter(JSContext *cx, uintN argc, jsval *vp)
+{
+    return DefineGetterOrSetter(cx, argc, PR_TRUE, vp);
+}
+
+static JSBool
+SharedDefineSetter(JSContext *cx, uintN argc, jsval *vp)
+{
+    return DefineGetterOrSetter(cx, argc, PR_FALSE, vp);
+}
+
+
 JSBool
 xpc_qsDefineQuickStubs(JSContext *cx, JSObject *proto, uintN flags,
                        PRUint32 ifacec, const nsIID **interfaces,
                        PRUint32 tableSize, const xpc_qsHashEntry *table)
 {
     /*
      * Walk interfaces in reverse order to behave like XPConnect when a
      * feature is defined in more than one of the interfaces.
      *
      * XPCNativeSet::FindMethod returns the first matching feature it finds,
      * searching the interfaces forward.  Here, definitions toward the
      * front of 'interfaces' overwrite those toward the back.
      */
+    PRBool definedProperty = PR_FALSE;
     for(uint32 i = ifacec; i-- != 0;)
     {
         const nsID &iid = *interfaces[i];
         const xpc_qsHashEntry *entry =
             LookupInterfaceOrAncestor(tableSize, table, iid);
 
         if(entry)
         {
             for(;;)
             {
                 // Define quick stubs for attributes.
                 const xpc_qsPropertySpec *ps = entry->properties;
                 if(ps)
                 {
                     for(; ps->name; ps++)
                     {
+                        definedProperty = PR_TRUE;
                         if(!JS_DefineProperty(cx, proto, ps->name, JSVAL_VOID,
                                               ps->getter, ps->setter,
                                               flags | JSPROP_SHARED))
                             return JS_FALSE;
                     }
                 }
 
                 // Define quick stubs for methods.
@@ -153,16 +420,28 @@ xpc_qsDefineQuickStubs(JSContext *cx, JS
                 // Next.
                 size_t j = entry->parentInterface;
                 if(j == XPC_QS_NULL_INDEX)
                     break;
                 entry = table + j;
             }
         }
     }
+
+    static JSFunctionSpec getterfns[] = {
+        JS_FN("__lookupGetter__", SharedLookupGetter, 1, 0),
+        JS_FN("__lookupSetter__", SharedLookupSetter, 1, 0),
+        JS_FN("__defineGetter__", SharedDefineGetter, 2, 0),
+        JS_FN("__defineSetter__", SharedDefineSetter, 2, 0),
+        JS_FS_END
+    };
+
+    if(definedProperty && !JS_DefineFunctions(cx, proto, getterfns))
+        return JS_FALSE;
+
     return JS_TRUE;
 }
 
 JSBool
 xpc_qsThrow(JSContext *cx, nsresult rv)
 {
     XPCThrower::Throw(rv, cx);
     return JS_FALSE;
--- a/js/src/xpconnect/tests/mochitest/Makefile.in
+++ b/js/src/xpconnect/tests/mochitest/Makefile.in
@@ -48,12 +48,13 @@ include $(topsrcdir)/config/rules.mk
 		test_bug361111.xul \
 		test_bug390488.html \
 		test_bug393269.html \
 		test_bug396851.html \
 		test_bug428021.html \
 		test_bug448587.html \
 		test_wrappers.html \
 		test_bug446584.html \
+		test_bug462428.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/test_bug462428.html
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=462428
+-->
+<head>
+  <title>Test for Bug 462428</title>
+  <script type="application/javascript" src="/MochiKit/MochiKit.js"></script>
+  <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=462428">Mozilla Bug 462428</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 462428 **/
+var getter = document.__lookupGetter__('documentElement');
+ok(getter !== undefined, "But able to look it up the normal way");
+
+is(getter.call(document), document.documentElement, "the getter actually works");
+
+document.__defineSetter__('documentElement', function() {});
+is(getter.call(document), document.documentElement, "the getter works after defineSetter");
+
+var oldTitle = document.title;
+try {
+    var setter = document.__lookupSetter__('title');
+    setter.call(document, "title 1");
+    is(document.title, "title 1", "the setter is bound correctly");
+} finally {
+    document.title = oldTitle
+}
+
+</script>
+</pre>
+</body>
+</html>