Bug 1041731 - Unforgeable Xrayed methods aren't getting cached on the holder. r=bholley, a=ledru.
authorPeter Van der Beken <peterv@propagandism.org>
Tue, 02 Sep 2014 14:11:38 +0200
changeset 217751 073eb01d99b754cb417d581183c7ce3ae32c23f4
parent 217750 d62fe065592e61d772542173db1216de2ecd2f01
child 217752 56bd998462222a6e5650c298be9e0a756b31e9b6
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, ledru
bugs1041731
milestone33.0a2
Bug 1041731 - Unforgeable Xrayed methods aren't getting cached on the holder. r=bholley, a=ledru.
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/test/chrome.ini
dom/bindings/test/test_document_location_via_xray_cached.html
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -964,76 +964,84 @@ GetNativePropertyHooks(JSContext *cx, JS
 
 // Try to resolve a property as an unforgeable property from the given
 // NativeProperties, if it's there.  nativeProperties is allowed to be null (in
 // which case we of course won't resolve anything).
 static bool
 XrayResolveUnforgeableProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                                JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
                                JS::MutableHandle<JSPropertyDescriptor> desc,
+                               bool& cacheOnHolder,
                                const NativeProperties* nativeProperties);
 
 static bool
 XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           const NativePropertyHooks* nativePropertyHooks,
                           DOMObjectType type, JS::Handle<JSObject*> obj,
                           JS::Handle<jsid> id,
-                          JS::MutableHandle<JSPropertyDescriptor> desc);
+                          JS::MutableHandle<JSPropertyDescriptor> desc,
+                          bool& cacheOnHolder);
 
 bool
 XrayResolveOwnProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
-                       JS::MutableHandle<JSPropertyDescriptor> desc)
+                       JS::MutableHandle<JSPropertyDescriptor> desc,
+                       bool& cacheOnHolder)
 {
+  cacheOnHolder = false;
+
   DOMObjectType type;
   const NativePropertyHooks *nativePropertyHooks =
     GetNativePropertyHooks(cx, obj, type);
 
   if (type != eInstance) {
     // For prototype objects and interface objects, just return their
     // normal set of properties.
     return XrayResolveNativeProperty(cx, wrapper, nativePropertyHooks, type,
-                                     obj, id, desc);
+                                     obj, id, desc, cacheOnHolder);
   }
 
   // Check for unforgeable properties before doing mResolveOwnProperty weirdness
   const NativePropertiesHolder& nativeProperties =
     nativePropertyHooks->mNativeProperties;
-  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc,
+  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc, cacheOnHolder,
                                       nativeProperties.regular)) {
     return false;
   }
   if (desc.object()) {
     return true;
   }
-  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc,
+  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc, cacheOnHolder,
                                       nativeProperties.chromeOnly)) {
     return false;
   }
   if (desc.object()) {
     return true;
   }
 
   return !nativePropertyHooks->mResolveOwnProperty ||
          nativePropertyHooks->mResolveOwnProperty(cx, wrapper, obj, id, desc);
 }
 
 static bool
 XrayResolveAttribute(JSContext* cx, JS::Handle<JSObject*> wrapper,
                      JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
                      const Prefable<const JSPropertySpec>* attributes, jsid* attributeIds,
-                     const JSPropertySpec* attributeSpecs, JS::MutableHandle<JSPropertyDescriptor> desc)
+                     const JSPropertySpec* attributeSpecs, JS::MutableHandle<JSPropertyDescriptor> desc,
+                     bool& cacheOnHolder)
 {
   for (; attributes->specs; ++attributes) {
     if (attributes->isEnabled(cx, obj)) {
       // Set i to be the index into our full list of ids/specs that we're
       // looking at now.
       size_t i = attributes->specs - attributeSpecs;
       for ( ; attributeIds[i] != JSID_VOID; ++i) {
         if (id == attributeIds[i]) {
+          cacheOnHolder = true;
+
           const JSPropertySpec& attrSpec = attributeSpecs[i];
           // Because of centralization, we need to make sure we fault in the
           // JitInfos as well. At present, until the JSAPI changes, the easiest
           // way to do this is wrap them up as functions ourselves.
           desc.setAttributes(attrSpec.flags & ~JSPROP_NATIVE_ACCESSORS);
           // They all have getters, so we can just make it.
           JS::Rooted<JSFunction*> fun(cx,
                                       JS_NewFunctionById(cx, (JSNative)attrSpec.getter.propertyOp.op,
@@ -1067,26 +1075,29 @@ XrayResolveAttribute(JSContext* cx, JS::
 }
 
 static bool
 XrayResolveMethod(JSContext* cx, JS::Handle<JSObject*> wrapper,
                   JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
                   const Prefable<const JSFunctionSpec>* methods,
                   jsid* methodIds,
                   const JSFunctionSpec* methodSpecs,
-                  JS::MutableHandle<JSPropertyDescriptor> desc)
+                  JS::MutableHandle<JSPropertyDescriptor> desc,
+                  bool& cacheOnHolder)
 {
   const Prefable<const JSFunctionSpec>* method;
   for (method = methods; method->specs; ++method) {
     if (method->isEnabled(cx, obj)) {
       // Set i to be the index into our full list of ids/specs that we're
       // looking at now.
       size_t i = method->specs - methodSpecs;
       for ( ; methodIds[i] != JSID_VOID; ++i) {
         if (id == methodIds[i]) {
+          cacheOnHolder = true;
+
           const JSFunctionSpec& methodSpec = methodSpecs[i];
           JSFunction *fun;
           if (methodSpec.selfHostedName) {
             fun = JS::GetSelfHostedFunction(cx, methodSpec.selfHostedName, id, methodSpec.nargs);
             if (!fun) {
               return false;
             }
             MOZ_ASSERT(!methodSpec.call.op, "Bad FunctionSpec declaration: non-null native");
@@ -1111,99 +1122,103 @@ XrayResolveMethod(JSContext* cx, JS::Han
   }
   return true;
 }
 
 /* static */ bool
 XrayResolveUnforgeableProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                                JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
                                JS::MutableHandle<JSPropertyDescriptor> desc,
+                               bool& cacheOnHolder,
                                const NativeProperties* nativeProperties)
 {
   if (!nativeProperties) {
     return true;
   }
 
   if (nativeProperties->unforgeableAttributes) {
     if (!XrayResolveAttribute(cx, wrapper, obj, id,
                               nativeProperties->unforgeableAttributes,
                               nativeProperties->unforgeableAttributeIds,
                               nativeProperties->unforgeableAttributeSpecs,
-                              desc)) {
+                              desc, cacheOnHolder)) {
       return false;
     }
 
     if (desc.object()) {
       return true;
     }
   }
 
   if (nativeProperties->unforgeableMethods) {
     if (!XrayResolveMethod(cx, wrapper, obj, id,
                            nativeProperties->unforgeableMethods,
                            nativeProperties->unforgeableMethodIds,
                            nativeProperties->unforgeableMethodSpecs,
-                           desc)) {
+                           desc, cacheOnHolder)) {
       return false;
     }
 
     if (desc.object()) {
       return true;
     }
   }
 
   return true;
 }
 
 static bool
 XrayResolveProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                     JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
-                    JS::MutableHandle<JSPropertyDescriptor> desc, DOMObjectType type,
+                    JS::MutableHandle<JSPropertyDescriptor> desc,
+                    bool& cacheOnHolder, DOMObjectType type,
                     const NativeProperties* nativeProperties)
 {
   const Prefable<const JSFunctionSpec>* methods;
   jsid* methodIds;
   const JSFunctionSpec* methodSpecs;
   if (type == eInterface) {
     methods = nativeProperties->staticMethods;
     methodIds = nativeProperties->staticMethodIds;
     methodSpecs = nativeProperties->staticMethodSpecs;
   } else {
     methods = nativeProperties->methods;
     methodIds = nativeProperties->methodIds;
     methodSpecs = nativeProperties->methodSpecs;
   }
   if (methods) {
     if (!XrayResolveMethod(cx, wrapper, obj, id, methods, methodIds,
-                           methodSpecs, desc)) {
+                           methodSpecs, desc, cacheOnHolder)) {
       return false;
     }
     if (desc.object()) {
       return true;
     }
   }
 
   if (type == eInterface) {
     if (nativeProperties->staticAttributes) {
       if (!XrayResolveAttribute(cx, wrapper, obj, id,
                                 nativeProperties->staticAttributes,
                                 nativeProperties->staticAttributeIds,
-                                nativeProperties->staticAttributeSpecs, desc)) {
+                                nativeProperties->staticAttributeSpecs, desc,
+                                cacheOnHolder)) {
         return false;
       }
       if (desc.object()) {
         return true;
       }
     }
   } else {
     if (nativeProperties->attributes) {
       if (!XrayResolveAttribute(cx, wrapper, obj, id,
                                 nativeProperties->attributes,
                                 nativeProperties->attributeIds,
-                                nativeProperties->attributeSpecs, desc)) {
+                                nativeProperties->attributeSpecs, desc,
+                                cacheOnHolder)) {
         return false;
       }
       if (desc.object()) {
         return true;
       }
     }
   }
 
@@ -1211,16 +1226,18 @@ XrayResolveProperty(JSContext* cx, JS::H
     const Prefable<const ConstantSpec>* constant;
     for (constant = nativeProperties->constants; constant->specs; ++constant) {
       if (constant->isEnabled(cx, obj)) {
         // Set i to be the index into our full list of ids/specs that we're
         // looking at now.
         size_t i = constant->specs - nativeProperties->constantSpecs;
         for ( ; nativeProperties->constantIds[i] != JSID_VOID; ++i) {
           if (id == nativeProperties->constantIds[i]) {
+            cacheOnHolder = true;
+
             desc.setAttributes(JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT);
             desc.object().set(wrapper);
             desc.value().set(nativeProperties->constantSpecs[i].value);
             return true;
           }
         }
       }
     }
@@ -1228,110 +1245,118 @@ XrayResolveProperty(JSContext* cx, JS::H
 
   return true;
 }
 
 static bool
 ResolvePrototypeOrConstructor(JSContext* cx, JS::Handle<JSObject*> wrapper,
                               JS::Handle<JSObject*> obj,
                               size_t protoAndIfaceCacheIndex, unsigned attrs,
-                              JS::MutableHandle<JSPropertyDescriptor> desc)
+                              JS::MutableHandle<JSPropertyDescriptor> desc,
+                              bool& cacheOnHolder)
 {
   JS::Rooted<JSObject*> global(cx, js::GetGlobalForObjectCrossCompartment(obj));
   {
     JSAutoCompartment ac(cx, global);
     ProtoAndIfaceCache& protoAndIfaceCache = *GetProtoAndIfaceCache(global);
     JSObject* protoOrIface =
       protoAndIfaceCache.EntrySlotIfExists(protoAndIfaceCacheIndex);
     if (!protoOrIface) {
       return false;
     }
+
+    cacheOnHolder = true;
+
     desc.object().set(wrapper);
     desc.setAttributes(attrs);
     desc.setGetter(JS_PropertyStub);
     desc.setSetter(JS_StrictPropertyStub);
     desc.value().set(JS::ObjectValue(*protoOrIface));
   }
   return JS_WrapPropertyDescriptor(cx, desc);
 }
 
 /* static */ bool
 XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           const NativePropertyHooks* nativePropertyHooks,
                           DOMObjectType type, JS::Handle<JSObject*> obj,
                           JS::Handle<jsid> id,
-                          JS::MutableHandle<JSPropertyDescriptor> desc)
+                          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);
+                                         desc, cacheOnHolder);
   }
 
   if (type == eInterfacePrototype && IdEquals(id, "constructor")) {
     return nativePropertyHooks->mConstructorID == constructors::id::_ID_Count ||
            ResolvePrototypeOrConstructor(cx, wrapper, obj,
                                          nativePropertyHooks->mConstructorID,
-                                         0, desc);
+                                         0, desc, cacheOnHolder);
   }
 
   const NativePropertiesHolder& nativeProperties =
     nativePropertyHooks->mNativeProperties;
 
   if (nativeProperties.regular &&
-      !XrayResolveProperty(cx, wrapper, obj, id, desc, type,
+      !XrayResolveProperty(cx, wrapper, obj, id, desc, cacheOnHolder, type,
                            nativeProperties.regular)) {
     return false;
   }
 
   if (!desc.object() &&
       nativeProperties.chromeOnly &&
       xpc::AccessCheck::isChrome(js::GetObjectCompartment(wrapper)) &&
-      !XrayResolveProperty(cx, wrapper, obj, id, desc, type,
+      !XrayResolveProperty(cx, wrapper, obj, id, desc, cacheOnHolder, type,
                            nativeProperties.chromeOnly)) {
     return false;
   }
 
   return true;
 }
 
 bool
 XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           JS::Handle<JSObject*> obj,
-                          JS::Handle<jsid> id, JS::MutableHandle<JSPropertyDescriptor> desc)
+                          JS::Handle<jsid> id, JS::MutableHandle<JSPropertyDescriptor> desc,
+                          bool& cacheOnHolder)
 {
+  cacheOnHolder = false;
+
   DOMObjectType type;
   const NativePropertyHooks* nativePropertyHooks =
     GetNativePropertyHooks(cx, obj, type);
 
   if (type == eInstance) {
     // Force the type to be eInterfacePrototype, since we need to walk the
     // prototype chain.
     type = eInterfacePrototype;
   }
 
   if (type == eInterfacePrototype) {
     do {
       if (!XrayResolveNativeProperty(cx, wrapper, nativePropertyHooks, type,
-                                     obj, id, desc)) {
+                                     obj, id, desc, cacheOnHolder)) {
         return false;
       }
 
       if (desc.object()) {
         return true;
       }
     } while ((nativePropertyHooks = nativePropertyHooks->mProtoHooks));
 
     return true;
   }
 
   return XrayResolveNativeProperty(cx, wrapper, nativePropertyHooks, type, obj,
-                                   id, desc);
+                                   id, desc, cacheOnHolder);
 }
 
 bool
 XrayDefineProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                    JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
                    JS::MutableHandle<JSPropertyDescriptor> desc, bool* defined)
 {
   if (!js::IsProxy(obj))
@@ -1660,17 +1685,18 @@ NativeToString(JSContext* cx, JS::Handle
   JS::Rooted<JSPropertyDescriptor> toStringDesc(cx);
   toStringDesc.object().set(nullptr);
   toStringDesc.setAttributes(0);
   toStringDesc.setGetter(nullptr);
   toStringDesc.setSetter(nullptr);
   toStringDesc.value().set(JS::UndefinedValue());
   JS::Rooted<jsid> id(cx,
     nsXPConnect::GetRuntimeInstance()->GetStringID(XPCJSRuntime::IDX_TO_STRING));
-  if (!XrayResolveNativeProperty(cx, wrapper, obj, id, &toStringDesc)) {
+  bool unused;
+  if (!XrayResolveNativeProperty(cx, wrapper, obj, id, &toStringDesc, unused)) {
     return false;
   }
 
   JS::Rooted<JSString*> str(cx);
   {
     JSAutoCompartment ac(cx, obj);
     if (toStringDesc.object()) {
       JS::Rooted<JS::Value> toString(cx, toStringDesc.value());
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -2269,29 +2269,31 @@ AddStringToIDVector(JSContext* cx, JS::A
  * wrapper is the Xray JS object.
  * obj is the target object of the Xray, a binding's instance object or a
  *     interface or interface prototype object.
  */
 bool
 XrayResolveOwnProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                        JS::Handle<JSObject*> obj,
                        JS::Handle<jsid> id,
-                       JS::MutableHandle<JSPropertyDescriptor> desc);
+                       JS::MutableHandle<JSPropertyDescriptor> desc,
+                       bool& cacheOnHolder);
 
 /**
  * This resolves operations, attributes and constants of the interfaces for obj.
  *
  * wrapper is the Xray JS object.
  * obj is the target object of the Xray, a binding's instance object or a
  *     interface or interface prototype object.
  */
 bool
 XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           JS::Handle<JSObject*> obj,
-                          JS::Handle<jsid> id, JS::MutableHandle<JSPropertyDescriptor> desc);
+                          JS::Handle<jsid> id, JS::MutableHandle<JSPropertyDescriptor> desc,
+                          bool& cacheOnHolder);
 
 /**
  * Define a property on obj through an Xray wrapper.
  *
  * wrapper is the Xray JS object.
  * obj is the target object of the Xray, a binding's instance object or a
  *     interface or interface prototype object.
  * defined will be set to true if a property was set as a result of this call.
--- a/dom/bindings/test/chrome.ini
+++ b/dom/bindings/test/chrome.ini
@@ -1,5 +1,6 @@
 [DEFAULT]
 
 [test_bug707564-chrome.html]
 [test_bug775543.html]
 [test_document_location_set_via_xray.html]
+[test_document_location_via_xray_cached.html]
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_document_location_via_xray_cached.html
@@ -0,0 +1,36 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1041731
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1041731</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1041731">Mozilla Bug 1041731</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe id="t" src="http://example.org/tests/dom/bindings/test/file_document_location_set_via_xray.html"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 1041731 **/
+
+function test()
+{
+  var loc = document.getElementById("t").contentWindow.document.location;
+  ise(loc.toString, loc.toString, "Unforgeable method on the Xray should be cached");
+  SimpleTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(test);
+
+</script>
+</pre>
+</body>
+</html>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -2052,18 +2052,19 @@ XPCWrappedNativeXrayTraits::construct(JS
 
 }
 
 bool
 DOMXrayTraits::resolveNativeProperty(JSContext *cx, HandleObject wrapper,
                                      HandleObject holder, HandleId id,
                                      MutableHandle<JSPropertyDescriptor> desc)
 {
+    bool unused;
     RootedObject obj(cx, getTargetObject(wrapper));
-    if (!XrayResolveNativeProperty(cx, wrapper, obj, id, desc))
+    if (!XrayResolveNativeProperty(cx, wrapper, obj, id, desc, unused))
         return false;
 
     MOZ_ASSERT(!desc.object() || desc.object() == wrapper, "What did we resolve this on?");
 
     return true;
 }
 
 bool
@@ -2094,23 +2095,36 @@ DOMXrayTraits::resolveOwnProperty(JSCont
                 }
                 desc.value().setObject(*obj);
                 FillPropertyDescriptor(desc, wrapper, true);
                 return JS_WrapPropertyDescriptor(cx, desc);
             }
         }
     }
 
+    if (!JS_GetPropertyDescriptorById(cx, holder, id, desc))
+        return false;
+    if (desc.object()) {
+        desc.object().set(wrapper);
+        return true;
+    }
+
     RootedObject obj(cx, getTargetObject(wrapper));
-    if (!XrayResolveOwnProperty(cx, wrapper, obj, id, desc))
+    bool cacheOnHolder;
+    if (!XrayResolveOwnProperty(cx, wrapper, obj, id, desc, cacheOnHolder))
         return false;
 
     MOZ_ASSERT(!desc.object() || desc.object() == wrapper, "What did we resolve this on?");
 
-    return true;
+    if (!desc.object() || !cacheOnHolder)
+        return true;
+
+    return JS_DefinePropertyById(cx, holder, id, desc.value(), desc.attributes(),
+                                 desc.getter(), desc.setter()) &&
+           JS_GetPropertyDescriptorById(cx, holder, id, desc);
 }
 
 bool
 DOMXrayTraits::defineProperty(JSContext *cx, HandleObject wrapper, HandleId id,
                               MutableHandle<JSPropertyDescriptor> desc,
                               Handle<JSPropertyDescriptor> existingDesc, bool *defined)
 {
     // Check for an indexed property on a Window.  If that's happening, do