Bug 843829 - Stop using IsTransparent for XBL field access, and explicitly waive instead. r=mrbkap
☠☠ backed out by e60919ded783 ☠ ☠
authorBobby Holley <bobbyholley@gmail.com>
Tue, 02 Apr 2013 18:51:20 -0700
changeset 127131 57652d8f0827
parent 127130 2e889cd77a48
child 127132 64f001fe04fb
push id117
push usertomi.aarnio@nokia.com
push dateWed, 03 Apr 2013 12:07:07 +0000
reviewersmrbkap
bugs843829
milestone23.0a1
Bug 843829 - Stop using IsTransparent for XBL field access, and explicitly waive instead. r=mrbkap
content/xbl/test/file_bug821850.xhtml
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/content/xbl/test/file_bug821850.xhtml
+++ b/content/xbl/test/file_bug821850.xhtml
@@ -39,18 +39,18 @@ https://bugzilla.mozilla.org/show_bug.cg
           var bound = document.getElementById('bound');
           ok(bound, "bound is non-null");
           is(bound.method('baz'), "method:baz", "Xray methods work");
           is(bound.prop, "propVal", "Property Xrays work");
           is(bound.primitiveField, 2, "Field Xrays work");
           is(bound.objectField.bar.a, 1, "Field Xrays work on objects");
           is(bound.contentField.foo, 10, "Field Xrays work on content objects");
           var hole = bound.contentField.rabbit.hole;
-          ok(hole.win === window, "We gain back Xray vision when hitting a native object");
-          ok(Cu.isXrayWrapper(hole.win), "Really is Xray");
+          ok(hole.win === XPCNativeWrapper.unwrap(window), "Xray vision remains waived when hitting a native object");
+          ok(!Cu.isXrayWrapper(hole.win), "Xray is waived");
 
           // This gets invoked by an event handler.
           window.finish = function() {
             // Content messed with stuff. Make sure we still see the right thing.
             is(bound.method('bay'), "method:bay", "Xray methods work");
             is(bound.wrappedJSObject.method('bay'), "hah", "Xray waived methods work");
             is(bound.prop, "set:someOtherVal", "Xray props work");
             is(bound.wrappedJSObject.prop, "redefined", "Xray waived props work");
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1298,17 +1298,17 @@ XrayWrapper<Base, Traits>::XrayWrapper(u
 template <typename Base, typename Traits>
 XrayWrapper<Base, Traits>::~XrayWrapper()
 {
 }
 
 namespace XrayUtils {
 
 bool
-IsTransparent(JSContext *cx, HandleObject wrapper, HandleId id)
+NeedsWaive(JSContext *cx, HandleObject wrapper, HandleId id)
 {
     // We dynamically waive Xray vision for XBL bindings accessing fields
     // on bound elements, since there's no way to access such things sanely
     // over Xray.
     nsCOMPtr<nsIContent> content;
     if (EnsureCompartmentPrivate(wrapper)->scope->IsXBLScope() &&
         (content = do_QueryInterfaceNative(cx, wrapper)))
     {
@@ -1451,28 +1451,21 @@ XrayWrapper<Base, Traits>::getPropertyDe
     RootedObject holder(cx, Traits::singleton.ensureHolder(cx, wrapper));
     if (Traits::isResolving(cx, holder, id)) {
         desc->obj = NULL;
         return true;
     }
 
     typename Traits::ResolvingIdImpl resolving(wrapper, id);
 
-    // Redirect access straight to the wrapper if we should be transparent.
-    if (XrayUtils::IsTransparent(cx, wrapper, id)) {
-        RootedObject obj(cx, Traits::getTargetObject(wrapper));
-        {
-            JSAutoCompartment ac(cx, obj);
-            if (!JS_GetPropertyDescriptorById(cx, obj, id, flags, desc))
-                return false;
-        }
-
-        if (desc->obj)
-            desc->obj = wrapper;
-        return JS_WrapPropertyDescriptor(cx, desc);
+    if (XrayUtils::NeedsWaive(cx, wrapper, id)) {
+        RootedObject waived(cx, WrapperFactory::WaiveXray(cx, wrapper));
+        if (!waived || !JS_WrapObject(cx, waived.address()))
+            return false;
+        return JS_GetPropertyDescriptorById(cx, waived, id, flags, desc);
     }
 
     if (!holder)
         return false;
 
     // Only chrome wrappers and same-origin xrays (used by jetpack sandboxes)
     // get .wrappedJSObject. We can check this by determining if the compartment
     // of the wrapper subsumes that of the wrappee.
@@ -1594,27 +1587,26 @@ XrayWrapper<Base, Traits>::getOwnPropert
         desc->obj = NULL;
         return true;
     }
 
     typename Traits::ResolvingIdImpl resolving(wrapper, id);
 
     // NB: Nothing we do here acts on the wrapped native itself, so we don't
     // enter our policy.
-    // Redirect access straight to the wrapper if we should be transparent.
-    if (XrayUtils::IsTransparent(cx, wrapper, id)) {
-        RootedObject obj(cx, Traits::getTargetObject(wrapper));
-        {
-            JSAutoCompartment ac(cx, obj);
-            if (!JS_GetPropertyDescriptorById(cx, obj, id, flags, desc))
-                return false;
-        }
 
-        desc->obj = (desc->obj == obj) ? wrapper.get() : nullptr; // XXX
-        return JS_WrapPropertyDescriptor(cx, desc);
+    if (XrayUtils::NeedsWaive(cx, wrapper, id)) {
+        RootedObject waived(cx, WrapperFactory::WaiveXray(cx, wrapper));
+        if (!waived || !JS_WrapObject(cx, waived.address()))
+            return false;
+        if (!JS_GetPropertyDescriptorById(cx, waived, id, flags, desc))
+            return false;
+        if (desc->obj != waived)
+            desc->obj = nullptr;
+        return true;
     }
 
     if (!Traits::singleton.resolveOwnProperty(cx, *this, wrapper, holder, id, desc, flags))
         return false;
 
     if (desc->obj) {
         desc->obj = wrapper;
         return true;
@@ -1631,24 +1623,21 @@ XrayWrapper<Base, Traits>::getOwnPropert
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::defineProperty(JSContext *cx, HandleObject wrapper,
                                           HandleId id, PropertyDescriptor *desc)
 {
     assertEnteredPolicy(cx, wrapper, id);
-    // Redirect access straight to the wrapper if we should be transparent.
-    if (XrayUtils::IsTransparent(cx, wrapper, id)) {
-        RootedObject obj(cx, Traits::getTargetObject(wrapper));
-        JSAutoCompartment ac(cx, obj);
-        if (!JS_WrapPropertyDescriptor(cx, desc))
+    if (XrayUtils::NeedsWaive(cx, wrapper, id)) {
+        RootedObject waived(cx, WrapperFactory::WaiveXray(cx, wrapper));
+        if (!waived || !JS_WrapObject(cx, waived.address()))
             return false;
-
-        return JS_DefinePropertyById(cx, obj, id, desc->value, desc->getter, desc->setter,
+        return JS_DefinePropertyById(cx, waived, id, desc->value, desc->getter, desc->setter,
                                      desc->attrs);
     }
 
     // NB: We still need JSRESOLVE_ASSIGNING here for the time being, because it
     // tells things like nodelists whether they should create the property or not.
     PropertyDescriptor existing_desc;
     if (!getOwnPropertyDescriptor(cx, wrapper, id, &existing_desc, JSRESOLVE_ASSIGNING))
         return false;
@@ -1693,29 +1682,16 @@ XrayWrapper<Base, Traits>::getOwnPropert
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::delete_(JSContext *cx, HandleObject wrapper,
                                    HandleId id, bool *bp)
 {
     assertEnteredPolicy(cx, wrapper, id);
-    // Redirect access straight to the wrapper if we should be transparent.
-    if (XrayUtils::IsTransparent(cx, wrapper, id)) {
-        RootedObject obj(cx, Traits::getTargetObject(wrapper));
-
-        JSAutoCompartment ac(cx, obj);
-
-        JSBool b;
-        RootedValue v(cx);
-        if (!JS_DeletePropertyById2(cx, obj, id, v.address()) || !JS_ValueToBoolean(cx, v, &b))
-            return false;
-        *bp = !!b;
-        return true;
-    }
 
     // Check the expando object.
     RootedObject target(cx, Traits::getTargetObject(wrapper));
     RootedObject expando(cx, Traits::singleton.getExpandoObject(cx, target, wrapper));
     JSBool b = true;
     if (expando) {
         JSAutoCompartment ac(cx, expando);
         RootedValue v(cx);
@@ -1730,23 +1706,16 @@ XrayWrapper<Base, Traits>::delete_(JSCon
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::enumerate(JSContext *cx, HandleObject wrapper, unsigned flags,
                                      AutoIdVector &props)
 {
     assertEnteredPolicy(cx, wrapper, JSID_VOID);
-    // Redirect access straight to the wrapper if we should be transparent.
-    if (XrayUtils::IsTransparent(cx, wrapper, JSID_VOIDHANDLE)) {
-        RootedObject obj(cx, Traits::getTargetObject(wrapper));
-        JSAutoCompartment ac(cx, obj);
-        return js::GetPropertyNames(cx, obj, flags, &props);
-    }
-
     if (!AccessCheck::wrapperSubsumes(wrapper)) {
         JS_ReportError(cx, "Not allowed to enumerate cross origin objects");
         return false;
     }
 
     // Enumerate expando properties first. Note that the expando object lives
     // in the target compartment.
     RootedObject target(cx, Traits::singleton.getTargetObject(wrapper));