Bug 738244 - Supporting DOM specific collection properties through xray wrappers; r=mrbkap
authorGabor Krizsanits <gkrizsanits@mozilla.com>
Mon, 27 Aug 2012 15:06:34 +0200
changeset 105594 febe1ad166da318e6965ae129e8c44872048115a
parent 105593 d985281cad1377334f72e5d3c85f6c42a38bc85e
child 105595 d36a98542b01fee2c7469eb466c27099f1e727b1
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersmrbkap
bugs738244
milestone17.0a1
Bug 738244 - Supporting DOM specific collection properties through xray wrappers; r=mrbkap
dom/base/nsDOMClassInfo.cpp
dom/base/nsDOMClassInfo.h
js/xpconnect/tests/chrome/Makefile.in
js/xpconnect/tests/chrome/test_bug738244.xul
js/xpconnect/tests/mochitest/Makefile.in
js/xpconnect/tests/mochitest/file_bug738244.html
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -7192,17 +7192,18 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
 
     return NS_OK;
   }
 
   // Hmm, we do an awful lot of QIs here; maybe we should add a
   // method on an interface that would let us just call into the
   // window code directly...
 
-  if (!ObjectIsNativeWrapper(cx, obj)) {
+  if (!ObjectIsNativeWrapper(cx, obj) ||
+      xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id)) {
     nsCOMPtr<nsIDocShellTreeNode> dsn(do_QueryInterface(win->GetDocShell()));
 
     int32_t count = 0;
 
     if (dsn) {
       dsn->GetChildCount(&count);
     }
 
@@ -9216,17 +9217,18 @@ nsHTMLDocumentSH::NewResolve(nsIXPConnec
   // nsDocumentSH::NewResolve() we're ok, since once those properties
   // are accessed, we'll do the necessary security check.
 
   if (!(flags & JSRESOLVE_ASSIGNING)) {
     // For native wrappers, do not resolve random names on document
 
     JSAutoRequest ar(cx);
 
-    if (!ObjectIsNativeWrapper(cx, obj)) {
+    if (!ObjectIsNativeWrapper(cx, obj) ||
+        xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id)) {
       nsCOMPtr<nsISupports> result;
       nsWrapperCache *cache;
       nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result),
                                 &cache);
       NS_ENSURE_SUCCESS(rv, rv);
 
       if (result) {
         JSBool ok = *_retval =
@@ -9308,33 +9310,30 @@ nsHTMLDocumentSH::NewResolve(nsIXPConnec
   return nsDocumentSH::NewResolve(wrapper, cx, obj, id, flags, objp, _retval);
 }
 
 NS_IMETHODIMP
 nsHTMLDocumentSH::GetProperty(nsIXPConnectWrappedNative *wrapper,
                               JSContext *cx, JSObject *obj, jsid id,
                               jsval *vp, bool *_retval)
 {
-  // For native wrappers, do not get random names on document
-  if (!ObjectIsNativeWrapper(cx, obj)) {
-    nsCOMPtr<nsISupports> result;
-
-    JSAutoRequest ar(cx);
-
-    nsWrapperCache *cache;
-    nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), &cache);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    if (result) {
-      rv = WrapNative(cx, obj, result, cache, true, vp);
-      if (NS_SUCCEEDED(rv)) {
-        rv = NS_SUCCESS_I_DID_SOMETHING;
-      }
-      return rv;
-    }
+  nsCOMPtr<nsISupports> result;
+
+  JSAutoRequest ar(cx);
+
+  nsWrapperCache *cache;
+  nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result), &cache);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (result) {
+    rv = WrapNative(cx, obj, result, cache, true, vp);
+    if (NS_SUCCEEDED(rv)) {
+      rv = NS_SUCCESS_I_DID_SOMETHING;
+    }
+    return rv;
   }
 
   return NS_OK;
 }
 
 // HTMLFormElement helper
 
 // static
@@ -9366,17 +9365,18 @@ nsHTMLFormElementSH::FindNamedItem(nsIFo
 NS_IMETHODIMP
 nsHTMLFormElementSH::NewResolve(nsIXPConnectWrappedNative *wrapper,
                                 JSContext *cx, JSObject *obj, jsid id,
                                 uint32_t flags, JSObject **objp,
                                 bool *_retval)
 {
   // For native wrappers, do not resolve random names on form
   if ((!(JSRESOLVE_ASSIGNING & flags)) && JSID_IS_STRING(id) &&
-      !ObjectIsNativeWrapper(cx, obj)) {
+      (!ObjectIsNativeWrapper(cx, obj) ||
+       xpc::WrapperFactory::XrayWrapperNotShadowing(obj, id))) {
     nsCOMPtr<nsIForm> form(do_QueryWrappedNative(wrapper, obj));
     nsCOMPtr<nsISupports> result;
     nsWrapperCache *cache;
 
     FindNamedItem(form, id, getter_AddRefs(result), &cache);
 
     if (result) {
       JSAutoRequest ar(cx);
@@ -9397,28 +9397,26 @@ NS_IMETHODIMP
 nsHTMLFormElementSH::GetProperty(nsIXPConnectWrappedNative *wrapper,
                                  JSContext *cx, JSObject *obj, jsid id,
                                  jsval *vp, bool *_retval)
 {
   nsCOMPtr<nsIForm> form(do_QueryWrappedNative(wrapper, obj));
 
   if (JSID_IS_STRING(id)) {
     // For native wrappers, do not get random names on form
-    if (!ObjectIsNativeWrapper(cx, obj)) {
-      nsCOMPtr<nsISupports> result;
-      nsWrapperCache *cache;
-
-      FindNamedItem(form, id, getter_AddRefs(result), &cache);
-
-      if (result) {
-        // Wrap result, result can be either an element or a list of
-        // elements
-        nsresult rv = WrapNative(cx, obj, result, cache, true, vp);
-        return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
-      }
+    nsCOMPtr<nsISupports> result;
+    nsWrapperCache *cache;
+
+    FindNamedItem(form, id, getter_AddRefs(result), &cache);
+
+    if (result) {
+      // Wrap result, result can be either an element or a list of
+      // elements
+      nsresult rv = WrapNative(cx, obj, result, cache, true, vp);
+      return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
     }
   } else {
     int32_t n = GetArrayIndexFromId(cx, id);
 
     if (n >= 0) {
       nsIFormControl* control = form->GetElementAt(n);
 
       if (control) {
--- a/dom/base/nsDOMClassInfo.h
+++ b/dom/base/nsDOMClassInfo.h
@@ -127,16 +127,21 @@ public:
    * work. In order to allow scriptable helpers to define non-IDL defined but
    * still "safe" properties for Xray wrappers, we call into the scriptable
    * helper with |obj| being the wrapper.
    *
    * Ideally, that would be the end of the story, however due to complications
    * dealing with document.domain, it's possible to end up in a scriptable
    * helper with a wrapper, even though we should be treating the lookup as a
    * transparent one.
+   *
+   * Note: So ObjectIsNativeWrapper(cx, obj) check usually means "through xray
+   * wrapper this part is not visible" while combined with
+   * || xpc::WrapperFactory::XrayWrapperNotShadowing(obj) it means "through
+   * xray wrapper it is visible only if it does not hide any native property."
    */
   static bool ObjectIsNativeWrapper(JSContext* cx, JSObject* obj);
 
   static nsISupports *GetNative(nsIXPConnectWrappedNative *wrapper, JSObject *obj);
 
   static nsIXPConnect *XPConnect()
   {
     return sXPConnect;
--- a/js/xpconnect/tests/chrome/Makefile.in
+++ b/js/xpconnect/tests/chrome/Makefile.in
@@ -27,16 +27,17 @@ MOCHITEST_CHROME_FILES = \
 		test_bug618176.xul \
 		file_bug618176.xul \
 		test_bug654370.xul \
 		test_bug658560.xul \
 		test_bug664689.xul \
 		test_bug679861.xul \
 		test_bug706301.xul \
 		test_bug726949.xul \
+		test_bug738244.xul \
 		test_bug743843.xul \
 		test_bug760076.xul \
 		test_bug760109.xul \
 		test_bug763343.xul \
 		test_bug771429.xul \
 		test_bug773962.xul \
 		test_APIExposer.xul \
 		test_chrometoSource.xul \
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug738244.xul
@@ -0,0 +1,50 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=533596
+-->
+<window title="Mozilla Bug 533596"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+
+  <iframe src="http://example.org/tests/js/xpconnect/tests/mochitest/file_bug738244.html"
+          onload="xrayTest(this)">
+  </iframe>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+
+      SimpleTest.waitForExplicitFinish();
+
+      function xrayTest(ifr) {
+        var win = ifr.contentWindow;
+        var doc = ifr.contentDocument;
+
+        doc.getElementById = 42;
+        is(doc.getElementById, 42,
+           "Native property cannot be shadowed on the xray");
+
+        is(doc.form1.name, "form1",
+           "Form elements cannot be found by name on the document through xray");
+
+        is(doc.form1.input1.name, "input1",
+           "Input element cannot be found by name on a form element through xray");
+
+        is(typeof doc.form1.appendChild, "function",
+           "Input element shadows native property with its name through xray");
+
+        is(win.frame1.name, "frame1",
+           "IFrames cannot be found by name on the window through xray");
+
+        SimpleTest.finish();
+      }
+
+  ]]></script>
+</window>
--- a/js/xpconnect/tests/mochitest/Makefile.in
+++ b/js/xpconnect/tests/mochitest/Makefile.in
@@ -67,16 +67,17 @@ MOCHITEST_FILES =	bug500931_helper.html 
 		test_bug781476.html \
 		file_bug781476.html \
 		file_nodelists.html \
 		file_exnstack.html \
 		file_expandosharing.html \
 		file_empty.html \
 		file_documentdomain.html \
 		test_lookupMethod.html \
+		file_bug738244.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES	= \
 		test_bug361111.xul \
 		test_bug760131.html \
 		$(NULL)
 
 ifneq ($(OS_TARGET),Android)
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/mochitest/file_bug738244.html
@@ -0,0 +1,10 @@
+<html>
+<body>
+<form name="form1">
+  <input name="input1" />
+  <input name="appendChild" />
+</form>
+<iframe name="frame1">
+</iframe>
+</body>
+</html>
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -620,16 +620,24 @@ WrapperFactory::WrapForSameCompartmentXr
         JSObject *xrayHolder = XrayUtils::createHolder(cx, obj, parent);
         if (!xrayHolder)
             return nullptr;
         js::SetProxyExtra(wrapperObj, 0, js::ObjectValue(*xrayHolder));
     }
     return wrapperObj;
 }
 
+
+bool
+WrapperFactory::XrayWrapperNotShadowing(JSObject *wrapper, jsid id)
+{
+    ResolvingId *rid = ResolvingId::getResolvingIdFromWrapper(wrapper);
+    return rid->isXrayShadowing(id);
+}
+
 /*
  * Calls to JS_TransplantObject* should go through these helpers here so that
  * waivers get fixed up properly.
  */
 
 static bool
 FixWaiverAfterTransplant(JSContext *cx, JSObject *oldWaiver, JSObject *newobj)
 {
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -88,15 +88,18 @@ class WrapperFactory {
     // Return true if this is a Components object.
     static bool IsComponentsObject(JSObject *obj);
 
     // Wrap a (same compartment) Components object.
     static JSObject *WrapComponentsObject(JSContext *cx, JSObject *obj);
 
     // Wrap a same-compartment object for Xray inspection.
     static JSObject *WrapForSameCompartmentXray(JSContext *cx, JSObject *obj);
+
+    // Returns true if the wrapper is in not shadowing mode for the id.
+    static bool XrayWrapperNotShadowing(JSObject *wrapper, jsid id);
 };
 
 extern js::DirectWrapper XrayWaiver;
 
 }
 
 #endif /* _xpc_WRAPPERFACTORY_H */
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -274,16 +274,78 @@ createHolder(JSContext *cx, JSObject *wr
     js::SetReservedSlot(holder, JSSLOT_RESOLVING, PrivateValue(NULL));
     return holder;
 }
 
 }
 
 using namespace XrayUtils;
 
+ResolvingId::ResolvingId(JSObject *wrapper, jsid id)
+    : mId(id),
+    mHolder(getHolderObject(wrapper)),
+    mPrev(getResolvingId(mHolder)),
+    mXrayShadowing(false)
+{
+    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(this));
+}
+
+ResolvingId::~ResolvingId()
+{
+    MOZ_ASSERT(getResolvingId(mHolder) == this, "unbalanced ResolvingIds");
+    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(mPrev));
+}
+
+bool
+ResolvingId::isXrayShadowing(jsid id)
+{
+    if (!mXrayShadowing)
+        return false;
+
+    return mId == id;
+}
+
+bool
+ResolvingId::isResolving(jsid id)
+{
+    for (ResolvingId *cur = this; cur; cur = cur->mPrev) {
+        if (cur->mId == id)
+            return true;
+    }
+
+    return false;
+}
+
+ResolvingId *
+ResolvingId::getResolvingId(JSObject *holder)
+{
+    MOZ_ASSERT(strcmp(JS_GetClass(holder)->name, "NativePropertyHolder") == 0);
+    return (ResolvingId *)js::GetReservedSlot(holder, JSSLOT_RESOLVING).toPrivate();
+}
+
+JSObject *
+ResolvingId::getHolderObject(JSObject *wrapper)
+{
+    return &js::GetProxyExtra(wrapper, 0).toObject();
+}
+
+ResolvingId *
+ResolvingId::getResolvingIdFromWrapper(JSObject *wrapper)
+{
+    return getResolvingId(getHolderObject(wrapper));
+}
+
+class ResolvingIdDummy
+{
+public:
+    ResolvingIdDummy(JSObject *wrapper, jsid id)
+    {
+    }
+};
+
 class XPCWrappedNativeXrayTraits
 {
 public:
     static bool resolveNativeProperty(JSContext *cx, JSObject *wrapper, JSObject *holder, jsid id,
                                       bool set, JSPropertyDescriptor *desc);
     static bool resolveOwnProperty(JSContext *cx, js::Wrapper &jsWrapper, JSObject *wrapper,
                                    JSObject *holder, jsid id, bool set,
                                    JSPropertyDescriptor *desc);
@@ -293,40 +355,28 @@ public:
     static bool enumerateNames(JSContext *cx, JSObject *wrapper, unsigned flags,
                                JS::AutoIdVector &props);
     static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper)
     {
         return getHolderObject(wrapper);
     }
     static JSObject* getInnerObject(JSObject *wrapper);
 
-    class ResolvingId
-    {
-    public:
-        ResolvingId(JSObject *holder, jsid id);
-        ~ResolvingId();
+    static bool isResolving(JSContext *cx, JSObject *holder, jsid id);
 
-    private:
-        friend class XPCWrappedNativeXrayTraits;
+    static bool resolveDOMCollectionProperty(JSContext *cx, JSObject *wrapper, JSObject *holder,
+                                             jsid id, bool set, PropertyDescriptor *desc);
 
-        jsid mId;
-        JSObject *mHolder;
-        ResolvingId *mPrev;
-    };
-    static bool isResolving(JSContext *cx, JSObject *holder, jsid id);
+    typedef ResolvingId ResolvingIdImpl;
 
 private:
     static JSObject* getHolderObject(JSObject *wrapper)
     {
         return &js::GetProxyExtra(wrapper, 0).toObject();
     }
-    static ResolvingId* getResolvingId(JSObject *holder)
-    {
-        return (ResolvingId *)js::GetReservedSlot(holder, JSSLOT_RESOLVING).toPrivate();
-    }
 };
 
 class ProxyXrayTraits
 {
 public:
     static bool resolveNativeProperty(JSContext *cx, JSObject *wrapper, JSObject *holder, jsid id,
                                       bool set, JSPropertyDescriptor *desc);
     static bool resolveOwnProperty(JSContext *cx, js::Wrapper &jsWrapper, JSObject *wrapper,
@@ -341,28 +391,23 @@ public:
     {
         return getHolderObject(cx, wrapper, true);
     }
     static JSObject* getInnerObject(JSObject *wrapper)
     {
         return &js::GetProxyPrivate(wrapper).toObject();
     }
 
-    class ResolvingId
-    {
-    public:
-        ResolvingId(JSObject *holder, jsid id)
-        {
-        }
-    };
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id)
     {
-      return false;
+        return false;
     }
 
+    typedef ResolvingIdDummy ResolvingIdImpl;
+
 private:
     static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper,
                                      bool createHolder)
     {
         if (!js::GetProxyExtra(wrapper, 0).isUndefined())
             return &js::GetProxyExtra(wrapper, 0).toObject();
 
         if (!createHolder)
@@ -390,28 +435,23 @@ public:
     {
         return getHolderObject(cx, wrapper, true);
     }
     static JSObject* getInnerObject(JSObject *wrapper)
     {
         return &js::GetProxyPrivate(wrapper).toObject();
     }
 
-    class ResolvingId
-    {
-    public:
-        ResolvingId(JSObject *holder, jsid id)
-        {
-        }
-    };
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id)
     {
-      return false;
+        return false;
     }
 
+    typedef ResolvingIdDummy ResolvingIdImpl;
+
 private:
     static JSObject* getHolderObject(JSContext *cx, JSObject *wrapper,
                                      bool createHolder)
     {
         if (!js::GetProxyExtra(wrapper, 0).isUndefined())
             return &js::GetProxyExtra(wrapper, 0).toObject();
 
         if (!createHolder)
@@ -467,40 +507,21 @@ FindWrapper(JSObject *wrapper)
 }
 
 JSObject*
 XPCWrappedNativeXrayTraits::getInnerObject(JSObject *wrapper)
 {
     return GetWrappedNativeObjectFromHolder(getHolderObject(wrapper));
 }
 
-XPCWrappedNativeXrayTraits::ResolvingId::ResolvingId(JSObject *wrapper, jsid id)
-  : mId(id),
-    mHolder(getHolderObject(wrapper)),
-    mPrev(getResolvingId(mHolder))
-{
-    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, PrivateValue(this));
-}
-
-XPCWrappedNativeXrayTraits::ResolvingId::~ResolvingId()
-{
-    NS_ASSERTION(getResolvingId(mHolder) == this, "unbalanced ResolvingIds");
-    js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, PrivateValue(mPrev));
-}
-
 bool
 XPCWrappedNativeXrayTraits::isResolving(JSContext *cx, JSObject *holder,
                                         jsid id)
 {
-    for (ResolvingId *cur = getResolvingId(holder); cur; cur = cur->mPrev) {
-        if (cur->mId == id)
-            return true;
-    }
-
-    return false;
+    return ResolvingId::getResolvingId(holder)->isResolving(id);
 }
 
 // Some DOM objects have shared properties that don't have an explicit
 // getter/setter and rely on the class getter/setter. We install a
 // class getter/setter on the holder object to trigger them.
 JSBool
 holder_get(JSContext *cx, JSHandleObject wrapper_, JSHandleId id, JSMutableHandleValue vp)
 {
@@ -543,45 +564,112 @@ holder_set(JSContext *cx, JSHandleObject
             if (retval)
                 XPCThrower::Throw(rv, cx);
             return false;
         }
     }
     return true;
 }
 
+class AutoSetWrapperNotShadowing
+{
+public:
+    AutoSetWrapperNotShadowing(JSObject *wrapper MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+        MOZ_ASSERT(wrapper);
+        mResolvingId = ResolvingId::getResolvingIdFromWrapper(wrapper);
+        MOZ_ASSERT(mResolvingId);
+        mResolvingId->mXrayShadowing = true;
+    }
+
+    ~AutoSetWrapperNotShadowing()
+    {
+        mResolvingId->mXrayShadowing = false;
+    }
+
+private:
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+    ResolvingId *mResolvingId;
+};
+
+// This is called after the resolveNativeProperty could not find any property
+// with the given id. At this point we can check for DOM specific collections
+// like document["formName"] because we already know that it is not shadowing
+// any native property.
+bool
+XPCWrappedNativeXrayTraits::resolveDOMCollectionProperty(JSContext *cx, JSObject *wrapper,
+                                                         JSObject *holder, jsid id, bool set,
+                                                         PropertyDescriptor *desc)
+{
+    // If we are not currently resolving this id and resolveNative is called
+    // we don't do anything. (see defineProperty in case of shadowing is forbidden).
+    ResolvingId *rid = ResolvingId::getResolvingId(holder);
+    if (!rid || rid->mId != id)
+        return true;
+
+    XPCWrappedNative *wn = GetWrappedNativeFromHolder(holder);
+    if (!NATIVE_HAS_FLAG(wn, WantNewResolve))
+        return true;
+
+    // Setting the current ResolvingId in non-shadowing mode. So for this id
+    // Xray won't ignore DOM specific collection properties temporarily.
+    AutoSetWrapperNotShadowing asw(wrapper);
+
+    bool retval = true;
+    JSObject *pobj = NULL;
+    unsigned flags = (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED;
+    nsresult rv = wn->GetScriptableInfo()->GetCallback()->NewResolve(wn, cx, wrapper, id,
+                                                                     flags, &pobj, &retval);
+    if (NS_FAILED(rv)) {
+        if (retval)
+            XPCThrower::Throw(rv, cx);
+        return false;
+    }
+
+    if (pobj && !JS_GetPropertyDescriptorById(cx, holder, id,
+                                              JSRESOLVE_QUALIFIED, desc))
+    {
+        return false;
+    }
+
+    return true;
+}
+
 bool
 XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapper,
                                                   JSObject *holder, jsid id, bool set,
                                                   JSPropertyDescriptor *desc)
 {
     desc->obj = NULL;
 
     MOZ_ASSERT(js::GetObjectJSClass(holder) == &HolderClass);
     JSObject *wnObject = GetWrappedNativeObjectFromHolder(holder);
     XPCWrappedNative *wn = GetWrappedNative(wnObject);
 
     // This will do verification and the method lookup for us.
     XPCCallContext ccx(JS_CALLER, cx, wnObject, nullptr, id);
 
-    // There are no native numeric properties, so we can shortcut here. We will not
-    // find the property.
+    // There are no native numeric properties, so we can shortcut here. We will
+    // not find the property. However we want to support non shadowing dom
+    // specific collection properties like window.frames, so we still have to
+    // check for those.
     if (!JSID_IS_STRING(id)) {
         /* Not found */
-        return true;
+        return resolveDOMCollectionProperty(cx, wrapper, holder, id, set, desc);
     }
 
     XPCNativeInterface *iface;
     XPCNativeMember *member;
     if (ccx.GetWrapper() != wn ||
         !wn->IsValid()  ||
         !(iface = ccx.GetInterface()) ||
         !(member = ccx.GetMember())) {
         /* Not found */
-        return true;
+        return resolveDOMCollectionProperty(cx, wrapper, holder, id, set, desc);
     }
 
     desc->obj = holder;
     desc->attrs = JSPROP_ENUMERATE;
     desc->getter = NULL;
     desc->setter = NULL;
     desc->shortid = 0;
     desc->value = JSVAL_VOID;
@@ -1230,17 +1318,17 @@ XrayWrapper<Base, Traits>::getPropertyDe
     }
 
     bool status;
     Wrapper::Action action = set ? Wrapper::SET : Wrapper::GET;
     desc->obj = NULL; // default value
     if (!this->enter(cx, wrapper, id, action, &status))
         return status;
 
-    typename Traits::ResolvingId resolving(wrapper, id);
+    typename Traits::ResolvingIdImpl resolving(wrapper, id);
 
     // Redirect access straight to the wrapper if we should be transparent.
     if (XrayUtils::IsTransparent(cx, wrapper)) {
         JSObject *obj = Traits::getInnerObject(wrapper);
         {
             JSAutoCompartment ac(cx, obj);
             if (!JS_GetPropertyDescriptorById(cx, obj, id,
                                               (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED,
@@ -1329,17 +1417,17 @@ XrayWrapper<Base, Traits>::getOwnPropert
     }
 
     bool status;
     Wrapper::Action action = set ? Wrapper::SET : Wrapper::GET;
     desc->obj = NULL; // default value
     if (!this->enter(cx, wrapper, id, action, &status))
         return status;
 
-    typename Traits::ResolvingId resolving(wrapper, id);
+    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)) {
         JSObject *obj = Traits::getInnerObject(wrapper);
         {
             JSAutoCompartment ac(cx, obj);
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -2,16 +2,17 @@
  * vim: set ts=4 sw=4 et tw=99 ft=cpp:
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jsapi.h"
 #include "jswrapper.h"
+#include "mozilla/GuardObjects.h"
 
 // Xray wrappers re-resolve the original native properties on the native
 // object and always directly access to those properties.
 // Because they work so differently from the rest of the wrapper hierarchy,
 // we pull them out of the Wrapper inheritance hierarchy and create a
 // little world around them.
 
 class XPCWrappedNative;
@@ -95,9 +96,35 @@ public:
     virtual bool getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id,
                                        bool set, js::PropertyDescriptor *desc);
     virtual bool getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy,
                                           jsid id, bool set,
                                           js::PropertyDescriptor *desc);
 };
 
 extern SandboxProxyHandler sandboxProxyHandler;
+
+class AutoSetWrapperNotShadowing;
+class XPCWrappedNativeXrayTraits;
+
+class ResolvingId {
+public:
+    ResolvingId(JSObject *wrapper, jsid id);
+    ~ResolvingId();
+
+    bool isXrayShadowing(jsid id);
+    bool isResolving(jsid id);
+    static ResolvingId* getResolvingId(JSObject *holder);
+    static JSObject* getHolderObject(JSObject *wrapper);
+    static ResolvingId *getResolvingIdFromWrapper(JSObject *wrapper);
+
+private:
+    friend class AutoSetWrapperNotShadowing;
+    friend class XPCWrappedNativeXrayTraits;
+
+    jsid mId;
+    JSObject *mHolder;
+    ResolvingId *mPrev;
+    bool mXrayShadowing;
+};
+
 }
+