Bug 857356 - Remove XBL field auto-waiving. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Thu, 09 May 2013 09:16:01 -0700
changeset 131397 52bd3c5ba8fda5baf01e9344fc883b9aacc8d45a
parent 131396 a23a23055e1370d6420649ee491ec1d6415602a5
child 131398 d8d4867f71c1725b193f9d6d20e5eea084d6459e
push id24658
push useremorley@mozilla.com
push dateFri, 10 May 2013 08:13:26 +0000
treeherdermozilla-central@08be63954b6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs857356
milestone23.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 857356 - Remove XBL field auto-waiving. r=bz
content/base/public/nsContentUtils.h
content/base/src/nsContentUtils.cpp
content/xbl/src/nsXBLProtoImplField.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@@ -179,18 +179,16 @@ public:
 
   static bool     IsCallerChrome();
   static bool     IsCallerXBL();
 
   static bool     IsImageSrcSetDisabled();
 
   static bool LookupBindingMember(JSContext* aCx, nsIContent *aContent,
                                   JS::HandleId aId, JSPropertyDescriptor* aDesc);
-  static bool IsBindingField(JSContext* aCx, nsIContent* aContent,
-                             JS::HandleId aId);
 
   /**
    * Returns the parent node of aChild crossing document boundaries.
    */
   static nsINode* GetCrossDocParentNode(nsINode* aChild);
 
   /**
    * Do not ever pass null pointers to this method.  If one of your
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -1753,33 +1753,16 @@ nsContentUtils::LookupBindingMember(JSCo
   nsXBLBinding* binding = aContent->OwnerDoc()->BindingManager()
                                   ->GetBinding(aContent);
   if (!binding)
     return true;
   return binding->LookupMember(aCx, aId, aDesc);
 }
 
 // static
-bool
-nsContentUtils::IsBindingField(JSContext* aCx, nsIContent* aContent,
-                               JS::HandleId aId)
-{
-  nsXBLBinding* binding = aContent->OwnerDoc()->BindingManager()
-                                  ->GetBinding(aContent);
-  if (!binding)
-    return false;
-
-  if (!JSID_IS_STRING(aId))
-    return false;
-  nsDependentJSString name(aId);
-
-  return binding->HasField(name);
-}
-
-// static
 nsINode*
 nsContentUtils::GetCrossDocParentNode(nsINode* aChild)
 {
   NS_PRECONDITION(aChild, "The child is null!");
 
   nsINode* parent = aChild->GetParentNode();
   if (parent || !aChild->IsNodeOfType(nsINode::eDOCUMENT))
     return parent;
--- a/content/xbl/src/nsXBLProtoImplField.cpp
+++ b/content/xbl/src/nsXBLProtoImplField.cpp
@@ -247,46 +247,18 @@ FieldGetterImpl(JSContext *cx, JS::CallA
   }
   args.rval().set(v);
   return true;
 }
 
 static JSBool
 FieldGetter(JSContext *cx, unsigned argc, JS::Value *vp)
 {
-  // FieldGetter generally lives in the XBL scope, and is defined as a cross-
-  // compartment wrapper on the in-content XBL prototype object. When content
-  // accesses the field for the first time, it ends up invoking the wrapped
-  // FieldGetter on the prototype, which enters the XBL scope, landing us here.
-  // We then use the nativeCall machinery to re-enter the content compartment
-  // (unwrapping |this|), define the field on the in-content |this|, and return
-  // the value of the field to the caller.
-  //
-  // There's one hitch, though. When code in the XBL scope accesses a field on
-  // the content object, we waive the usual Xray vision granted to XBL scopes
-  // in order to do the access, because there isn't really anything else sane to
-  // do. In this sequence of events, the chrome caller invokes a get() for the
-  // field on the Xrayed element. XrayWrapper::get bounces to BaseProxyHandler::get,
-  // Which invokes XrayWrapper::getPropertyDescriptor. This detects the field
-  // access, creates a waived version of the wrapper, and does a lookup for the
-  // property on the waived wrapper. This would normally result in the resulting
-  // getter being transitively waived, which would cause said getter to properly
-  // waive Xray on its return value when it is eventually invoked (by the XBL
-  // scope) further down in BaseProxyHandler::get. However, this getter is
-  // FieldGetter, which actually lives in the XBL scope, meaning that we end up
-  // stripping all the wrappers off, effectively losing track of the fact that
-  // we meant to be waiving Xray here.
-  //
-  // Since fields are already doing this special Xray waiving stuff, the simplest
-  // solution seems to be to waive Xray on the |this| object before invoking
-  // CallNonGenericMethod. This means that the nativeCall trap of WaiveXrayWrapper
-  // will properly waive the result on the way back. Whew.
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  return xpc::WrapperFactory::WaiveXrayAndWrap(cx, args.mutableThisv().address()) &&
-         JS::CallNonGenericMethod<ValueHasISupportsPrivate, FieldGetterImpl>
+  return JS::CallNonGenericMethod<ValueHasISupportsPrivate, FieldGetterImpl>
                                  (cx, args);
 }
 
 bool
 FieldSetterImpl(JSContext *cx, JS::CallArgs args)
 {
   const JS::Value &thisv = args.thisv();
   MOZ_ASSERT(ValueHasISupportsPrivate(thisv));
@@ -315,22 +287,17 @@ FieldSetterImpl(JSContext *cx, JS::CallA
   args.rval().setUndefined();
   return true;
 }
 
 static JSBool
 FieldSetter(JSContext *cx, unsigned argc, JS::Value *vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  // It's probably not actually necessary to waive Xray here given that
-  // FieldSetter doesn't return everything, but it's good to maintain
-  // consistency with FieldGetter. See the comment there for more details on
-  // why we do this.
-  return xpc::WrapperFactory::WaiveXrayAndWrap(cx, args.mutableThisv().address()) &&
-         JS::CallNonGenericMethod<ValueHasISupportsPrivate, FieldSetterImpl>
+  return JS::CallNonGenericMethod<ValueHasISupportsPrivate, FieldSetterImpl>
                                  (cx, args);
 }
 
 nsresult
 nsXBLProtoImplField::InstallAccessors(JSContext* aCx,
                                       JS::Handle<JSObject*> aTargetClassObject)
 {
   MOZ_ASSERT(js::IsObjectInContextCompartment(aTargetClassObject, aCx));
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1325,32 +1325,16 @@ XrayWrapper<Base, Traits>::XrayWrapper(u
 
 template <typename Base, typename Traits>
 XrayWrapper<Base, Traits>::~XrayWrapper()
 {
 }
 
 namespace XrayUtils {
 
-bool
-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)))
-    {
-        if (nsContentUtils::IsBindingField(cx, content, id))
-            return true;
-    }
-    return false;
-}
-
 JSObject *
 GetNativePropertiesObject(JSContext *cx, JSObject *wrapper)
 {
     NS_ASSERTION(js::IsWrapper(wrapper) && WrapperFactory::IsXrayWrapper(wrapper),
                  "bad object passed in");
 
     JSObject *holder = GetHolder(wrapper);
     NS_ASSERTION(holder, "uninitialized wrapper being used?");
@@ -1518,23 +1502,16 @@ 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(cx, wrapper, id);
 
-    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.
     XPCJSRuntime* rt = nsXPConnect::GetRuntimeInstance();
     if (AccessCheck::wrapperSubsumes(wrapper) &&
@@ -1672,47 +1649,29 @@ XrayWrapper<Base, Traits>::getOwnPropert
         return true;
     }
 
     typename Traits::ResolvingIdImpl resolving(cx, wrapper, id);
 
     // NB: Nothing we do here acts on the wrapped native itself, so we don't
     // enter our policy.
 
-    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;
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::defineProperty(JSContext *cx, HandleObject wrapper,
                                           HandleId id, PropertyDescriptor *desc)
 {
     assertEnteredPolicy(cx, wrapper, id);
-    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, 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.
     Rooted<PropertyDescriptor> existing_desc(cx);
     if (!getOwnPropertyDescriptor(cx, wrapper, id, existing_desc.address(), JSRESOLVE_ASSIGNING))
         return false;
 
     if (existing_desc.object() && existing_desc.isPermanent())