Bug 823228 part 2. Move indexed property access on windows from nsWindowSH::GetProperty to the outer window proxy. r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 17 Jan 2013 12:30:37 -0500
changeset 119161 a46302d16d2006567836b20f5611d541652b3aa5
parent 119160 43511e31bf92e8613d0772fbfbab1b6e90ffccd6
child 119162 247a5773190bf3379cf802f92af441ed28ffc056
push id24195
push userMs2ger@gmail.com
push dateSat, 19 Jan 2013 16:10:11 +0000
treeherderautoland@02e12a80aef9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs823228
milestone21.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 823228 part 2. Move indexed property access on windows from nsWindowSH::GetProperty to the outer window proxy. r=bholley
dom/base/nsDOMClassInfo.cpp
dom/base/nsGlobalWindow.cpp
dom/base/test/Makefile.in
dom/base/test/test_window_indexing.html
js/xpconnect/tests/chrome/test_bug738244.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -5027,17 +5027,17 @@ nsWindowSH::InstallGlobalScopePolluter(J
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWindowSH::GetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                         JSObject *obj, jsid id, jsval *vp, bool *_retval)
 {
-  nsGlobalWindow *win = nsGlobalWindow::FromWrapper(wrapper);
+  DebugOnly<nsGlobalWindow*> win = nsGlobalWindow::FromWrapper(wrapper);
 
   JSAutoRequest ar(cx);
 
 #ifdef DEBUG_SH_FORWARDING
   {
     JSString *jsstr = ::JS_ValueToString(cx, id);
     if (jsstr) {
       nsDependentJSString str(jsstr);
@@ -5054,58 +5054,16 @@ nsWindowSH::GetProperty(nsIXPConnectWrap
     }
   }
 #endif
 
   // The order in which things are done in this method are a bit
   // whacky, that's because this method is *extremely* performace
   // critical. Don't touch this unless you know what you're doing.
 
-  if (JSID_IS_INT(id) && JSID_TO_INT(id) >= 0) {
-    // If we're accessing a numeric property we'll treat that as if
-    // window.frames[n] is accessed (since window.frames === window),
-    // if window.frames[n] is a child frame, wrap the frame and return
-    // it without doing a security check.
-    uint32_t index = uint32_t(JSID_TO_INT(id));
-    bool found = false;
-    if (nsCOMPtr<nsIDOMWindow> frame = win->IndexedGetter(index, found)) {
-      // A numeric property accessed and the numeric property is a
-      // child frame, wrap the child frame without doing a security
-      // check and return.
-
-      nsGlobalWindow *frameWin = (nsGlobalWindow *)frame.get();
-      NS_ASSERTION(frameWin->IsOuterWindow(), "IndexedGetter gave us an inner?");
-
-      frameWin->EnsureInnerWindow();
-      JSObject *global = frameWin->GetGlobalJSObject();
-
-      // This null check fixes a hard-to-reproduce crash that occurs when we
-      // get here when we're mid-call to nsDocShell::Destroy. See bug 640904
-      // comment 105.
-      if (MOZ_UNLIKELY(!global)) {
-        return NS_ERROR_FAILURE;
-      }
-
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      jsval v;
-      nsresult rv = WrapNative(cx, xpc_UnmarkGrayObject(global), frame,
-                               &NS_GET_IID(nsIDOMWindow), true, &v,
-                               getter_AddRefs(holder));
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      if (!JS_WrapValue(cx, &v)) {
-        return NS_ERROR_FAILURE;
-      }
-
-      *vp = v;
-    }
-
-    return NS_SUCCESS_I_DID_SOMETHING;
-  }
-
   if (JSID_IS_STRING(id) && !JSVAL_IS_PRIMITIVE(*vp) &&
       ::JS_TypeOfValue(cx, *vp) != JSTYPE_FUNCTION) {
     // A named property accessed which could have been resolved to a
     // child frame in nsWindowSH::NewResolve() (*vp will tell us if
     // that's the case). If *vp is a window object (i.e. a child
     // frame), return without doing a security check.
     //
     // Calling GetWrappedNativeOfJSObject() is not all that cheap, so
@@ -6595,44 +6553,23 @@ LocationSetterUnwrapper(JSContext *cx, J
 NS_IMETHODIMP
 nsWindowSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                        JSObject *obj_, jsid id_, uint32_t flags,
                        JSObject **objp, bool *_retval)
 {
   js::RootedObject obj(cx, obj_);
   js::RootedId id(cx, id_);
 
+  if (!JSID_IS_STRING(id)) {
+    return NS_OK;
+  }
+
   nsGlobalWindow *win = nsGlobalWindow::FromWrapper(wrapper);
   MOZ_ASSERT(win->IsInnerWindow());
 
-  if (!JSID_IS_STRING(id)) {
-    if (JSID_IS_INT(id) && JSID_TO_INT(id) >= 0 && !(flags & JSRESOLVE_ASSIGNING)) {
-      // If we're resolving a numeric property, treat that as if
-      // window.frames[n] is resolved (since window.frames ===
-      // window), if window.frames[n] is a child frame, define a
-      // property for this index.
-      uint32_t index = uint32_t(JSID_TO_INT(id));
-      bool found;
-      nsCOMPtr<nsIDOMWindow> frame = win->IndexedGetter(index, found);
-      if (found) {
-        // A numeric property accessed and the numeric property is a
-        // child frame. Define a property for this index.
-
-        *_retval = ::JS_DefineElement(cx, obj, index, JSVAL_VOID,
-                                      nullptr, nullptr, JSPROP_SHARED);
-
-        if (*_retval) {
-          *objp = obj;
-        }
-      }
-    }
-
-    return NS_OK;
-  }
-
   nsIScriptContext *my_context = win->GetContextInternal();
 
   // Resolve standard classes on my_context's JSContext (or on cx,
   // if we don't have a my_context yet), in case the two contexts
   // have different origins.  We want lazy standard class
   // initialization to behave as if it were done eagerly, on each
   // window's own context (not on some other window-caller's
   // context).
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -529,56 +529,320 @@ class nsOuterWindowProxy : public js::Wr
 public:
   nsOuterWindowProxy() : js::Wrapper(0) { setSafeToUnwrap(false); }
 
   virtual bool isOuterWindow() {
     return true;
   }
   virtual JSString *obj_toString(JSContext *cx, JSObject *wrapper) MOZ_OVERRIDE;
   virtual void finalize(JSFreeOp *fop, JSObject *proxy) MOZ_OVERRIDE;
+
+  // Fundamental traps
+  virtual bool getPropertyDescriptor(JSContext* cx, JSObject* proxy,
+                                     jsid id, JSPropertyDescriptor* desc,
+                                     unsigned flags) MOZ_OVERRIDE;
+  virtual bool getOwnPropertyDescriptor(JSContext* cx, JSObject* proxy,
+                                        jsid id, JSPropertyDescriptor* desc,
+                                        unsigned flags) MOZ_OVERRIDE;
+  virtual bool defineProperty(JSContext* cx, JSObject* proxy,
+                              jsid id, JSPropertyDescriptor* desc) MOZ_OVERRIDE;
+  virtual bool getOwnPropertyNames(JSContext *cx, JSObject *proxy,
+                                   JS::AutoIdVector &props) MOZ_OVERRIDE;
+  virtual bool delete_(JSContext *cx, JSObject *proxy, jsid id,
+                       bool *bp) MOZ_OVERRIDE;
+  virtual bool enumerate(JSContext *cx, JSObject *proxy,
+                         JS::AutoIdVector &props) MOZ_OVERRIDE;
+
+  // Derived traps
+  virtual bool has(JSContext *cx, JSObject *proxy, jsid id,
+                   bool *bp) MOZ_OVERRIDE;
+  virtual bool hasOwn(JSContext *cx, JSObject *proxy, jsid id,
+                      bool *bp) MOZ_OVERRIDE;
   virtual bool get(JSContext *cx, JSObject *wrapper, JSObject *receiver,
-                   jsid id, js::Value *vp) MOZ_OVERRIDE;
+                   jsid id, JS::Value *vp) MOZ_OVERRIDE;
+  virtual bool set(JSContext *cx, JSObject *proxy, JSObject *receiver,
+                   jsid id, bool strict, JS::Value *vp) MOZ_OVERRIDE;
+  virtual bool keys(JSContext *cx, JSObject *proxy,
+                    JS::AutoIdVector &props) MOZ_OVERRIDE;
+  virtual bool iterate(JSContext *cx, JSObject *proxy, unsigned flags,
+                       JS::Value *vp) MOZ_OVERRIDE;
 
   static nsOuterWindowProxy singleton;
+
+protected:
+  nsGlobalWindow* GetWindow(JSObject *proxy)
+  {
+    return nsGlobalWindow::FromSupports(
+      static_cast<nsISupports*>(js::GetProxyExtra(proxy, 0).toPrivate()));
+  }
+
+  // False return value means we threw an exception.  True return value
+  // but false "found" means we didn't have a subframe at that index.
+  bool GetSubframeWindow(JSContext *cx, JSObject *proxy, jsid id,
+                         JS::Value *vp, bool &found);
+
+  // Returns a non-null window only if id is an index and we have a
+  // window at that index.
+  already_AddRefed<nsIDOMWindow> GetSubframeWindow(JSContext *cx,
+                                                   JSObject *proxy,
+                                                   jsid id);
+
+  bool AppendIndexedPropertyNames(JSContext *cx, JSObject *proxy,
+                                  JS::AutoIdVector &props);
 };
 
 
 JSString *
 nsOuterWindowProxy::obj_toString(JSContext *cx, JSObject *proxy)
 {
     MOZ_ASSERT(js::IsProxy(proxy));
 
     return JS_NewStringCopyZ(cx, "[object Window]");
 }
 
 void
 nsOuterWindowProxy::finalize(JSFreeOp *fop, JSObject *proxy)
 {
-  nsISupports *global =
-    static_cast<nsISupports*>(js::GetProxyExtra(proxy, 0).toPrivate());
+  nsGlobalWindow* global = GetWindow(proxy);
   if (global) {
-    nsWrapperCache *cache;
-    CallQueryInterface(global, &cache);
-    cache->ClearWrapper();
+    global->ClearWrapper();
   }
 }
 
 bool
+nsOuterWindowProxy::getPropertyDescriptor(JSContext* cx, JSObject* proxy,
+                                          jsid id,
+                                          JSPropertyDescriptor* desc,
+                                          unsigned flags)
+{
+  // The only thing we can do differently from js::Wrapper is shadow stuff with
+  // our indexed properties, so we can just try getOwnPropertyDescriptor and if
+  // that gives us nothing call on through to js::Wrapper.
+  desc->obj = nullptr;
+  if (!getOwnPropertyDescriptor(cx, proxy, id, desc, flags)) {
+    return false;
+  }
+
+  if (desc->obj) {
+    return true;
+  }
+
+  return js::Wrapper::getPropertyDescriptor(cx, proxy, id, desc, flags);
+}
+
+bool
+nsOuterWindowProxy::getOwnPropertyDescriptor(JSContext* cx, JSObject* proxy,
+                                             jsid id,
+                                             JSPropertyDescriptor* desc,
+                                             unsigned flags)
+{
+  bool found;
+  if (!GetSubframeWindow(cx, proxy, id, &desc->value, found)) {
+    return false;
+  }
+  if (found) {
+    FillPropertyDescriptor(desc, proxy, true);
+    return true;
+  }
+  // else fall through to js::Wrapper
+
+  return js::Wrapper::getOwnPropertyDescriptor(cx, proxy, id, desc, flags);
+}
+
+bool
+nsOuterWindowProxy::defineProperty(JSContext* cx, JSObject* proxy,
+                                   jsid id, JSPropertyDescriptor* desc)
+{
+  if (nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id)) {
+    // Don't define anything; we're done here, since the spec requires
+    // that we treat our indexed properties as readonly.
+    return true;
+  }
+
+  return js::Wrapper::defineProperty(cx, proxy, id, desc);
+}
+
+bool
+nsOuterWindowProxy::getOwnPropertyNames(JSContext *cx, JSObject *proxy,
+                                        JS::AutoIdVector &props)
+{
+  // Just our indexed stuff followed by our "normal" own property names.
+  if (!AppendIndexedPropertyNames(cx, proxy, props)) {
+    return false;
+  }
+
+  JS::AutoIdVector innerProps(cx);
+  if (!js::Wrapper::getOwnPropertyNames(cx, proxy, innerProps)) {
+    return false;
+  }
+  return js::AppendUnique(cx, props, innerProps);
+}
+
+bool
+nsOuterWindowProxy::delete_(JSContext *cx, JSObject *proxy, jsid id,
+                            bool *bp)
+{
+  if (nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id)) {
+    // Reject (which means throw if and only if strict) the set.
+    // Except we don't even know whether we're strict.  See bug 803157.
+    *bp = false;
+    return true;
+  }
+
+  return js::Wrapper::delete_(cx, proxy, id, bp);
+}
+
+bool
+nsOuterWindowProxy::enumerate(JSContext *cx, JSObject *proxy,
+                              JS::AutoIdVector &props)
+{
+  // Just our indexed stuff followed by our "normal" own property names.
+  if (!AppendIndexedPropertyNames(cx, proxy, props)) {
+    return false;
+  }
+
+  JS::AutoIdVector innerProps(cx);
+  if (!js::Wrapper::enumerate(cx, proxy, innerProps)) {
+    return false;
+  }
+  return js::AppendUnique(cx, props, innerProps);
+}
+
+bool
+nsOuterWindowProxy::has(JSContext *cx, JSObject *proxy, jsid id, bool *bp)
+{
+  if (nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id)) {
+    *bp = true;
+    return true;
+  }
+
+  return js::Wrapper::has(cx, proxy, id, bp);
+}
+
+bool
+nsOuterWindowProxy::hasOwn(JSContext *cx, JSObject *proxy, jsid id, bool *bp)
+{
+  if (nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id)) {
+    *bp = true;
+    return true;
+  }
+
+  return js::Wrapper::hasOwn(cx, proxy, id, bp);
+}
+
+bool
 nsOuterWindowProxy::get(JSContext *cx, JSObject *wrapper, JSObject *receiver,
-                        jsid id, js::Value *vp)
+                        jsid id, JS::Value *vp)
 {
   if (id == nsDOMClassInfo::sWrappedJSObject_id &&
       xpc::AccessCheck::isChrome(js::GetContextCompartment(cx))) {
     *vp = JS::ObjectValue(*wrapper);
     return true;
   }
 
+  bool found;
+  if (!GetSubframeWindow(cx, wrapper, id, vp, found)) {
+    return false;
+  }
+  if (found) {
+    return true;
+  }
+  // Else fall through to js::Wrapper
+
   return js::Wrapper::get(cx, wrapper, receiver, id, vp);
 }
 
+bool
+nsOuterWindowProxy::set(JSContext *cx, JSObject *proxy, JSObject *receiver,
+                        jsid id, bool strict, JS::Value *vp)
+{
+  if (nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id)) {
+    // Reject (which means throw if and only if strict) the set.
+    if (strict) {
+      // XXXbz This needs to throw, but see bug 828137.
+    }
+    return true;
+  }
+
+  return js::Wrapper::set(cx, proxy, receiver, id, strict, vp);
+}
+
+bool
+nsOuterWindowProxy::keys(JSContext *cx, JSObject *proxy,
+                         JS::AutoIdVector &props)
+{
+  // BaseProxyHandler::keys seems to do what we want here: call
+  // getOwnPropertyNames and then filter out the non-enumerable properties.
+  return js::BaseProxyHandler::keys(cx, proxy, props);
+}
+
+bool
+nsOuterWindowProxy::iterate(JSContext *cx, JSObject *proxy, unsigned flags,
+                            JS::Value *vp)
+{
+  // BaseProxyHandler::iterate seems to do what we want here: fall
+  // back on the property names returned from keys() and enumerate().
+  return js::BaseProxyHandler::iterate(cx, proxy, flags, vp);
+}
+
+bool
+nsOuterWindowProxy::GetSubframeWindow(JSContext *cx, JSObject *proxy,
+                                      jsid id, JS::Value* vp,
+                                      bool& found)
+{
+  nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id);
+  if (!frame) {
+    found = false;
+    return true;
+  }
+
+  found = true;
+  // Just return the window's global
+  nsGlobalWindow* global = static_cast<nsGlobalWindow*>(frame.get());
+  global->EnsureInnerWindow();
+  JSObject* obj = global->FastGetGlobalJSObject();
+  // This null check fixes a hard-to-reproduce crash that occurs when we
+  // get here when we're mid-call to nsDocShell::Destroy. See bug 640904
+  // comment 105.
+  if (MOZ_UNLIKELY(!obj)) {
+    return xpc::Throw(cx, NS_ERROR_FAILURE);
+  }
+  *vp = JS::ObjectValue(*obj);
+  return JS_WrapValue(cx, vp);
+}
+
+already_AddRefed<nsIDOMWindow>
+nsOuterWindowProxy::GetSubframeWindow(JSContext *cx, JSObject *proxy, jsid id)
+{
+  int32_t index = GetArrayIndexFromId(cx, id);
+  if (!IsArrayIndex(index)) {
+    return nullptr;
+  }
+
+  nsGlobalWindow* win = GetWindow(proxy);
+  bool unused;
+  return win->IndexedGetter(index, unused);
+}
+
+bool
+nsOuterWindowProxy::AppendIndexedPropertyNames(JSContext *cx, JSObject *proxy,
+                                               JS::AutoIdVector &props)
+{
+  uint32_t length = GetWindow(proxy)->GetLength();
+  MOZ_ASSERT(int32_t(length) >= 0);
+  if (!props.reserve(props.length() + length)) {
+    return false;
+  }
+  for (int32_t i = 0; i < int32_t(length); ++i) {
+    props.append(INT_TO_JSID(i));
+  }
+
+  return true;
+}
+
 nsOuterWindowProxy
 nsOuterWindowProxy::singleton;
 
 class nsChromeOuterWindowProxy : public nsOuterWindowProxy
 {
 public:
   nsChromeOuterWindowProxy() : nsOuterWindowProxy() {}
 
--- a/dom/base/test/Makefile.in
+++ b/dom/base/test/Makefile.in
@@ -18,16 +18,17 @@ MOCHITEST_FILES = \
   test_e4x_for_each.html \
   test_gsp-standards.html \
   test_gsp-quirks.html \
   test_gsp-qualified.html \
   test_nondomexception.html \
   test_screen_orientation.html \
   test_window_constructor.html \
   test_window_enumeration.html \
+  test_window_indexing.html \
   test_writable-replaceable.html \
   $(NULL)
 
 MOCHITEST_CHROME_FILES = \
    test_bug715041.xul \
    test_bug715041_removal.xul \
    $(NULL)
 
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_window_indexing.html
@@ -0,0 +1,116 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=823228
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 823228</title>
+  <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=823228">Mozilla Bug 823228</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <iframe name="x" id="x"></iframe>
+  <iframe name="y" id="y"></iframe>
+</div>
+<pre id="test">
+</pre>
+  <script type="application/javascript">
+
+  /** Test for Bug 823228 **/
+  is(window, window.frames, "These better be equal");
+  ok("0" in window, "We have a subframe");
+  ok("1" in window, "We have two subframes");
+  ok(!("2" in window), "But we don't have three subframes");
+  window[2] = "myString";
+  ok("2" in window, "Should be able to set expando");
+  Object.getPrototypeOf(window)[3] = "Hey there";
+  ok("3" in window, "Should be walking up the proto chain");
+
+  is(window[0].name, "x", "First frame is x");
+  is(window[1].name, "y", "Second frame is y");
+  is(window[2], "myString", "We should still have our expando");
+  is(window[3], "Hey there", "We should still have our prop on the proto chain");
+
+  var x = $("x");
+  var y = $("y");
+
+  is(x.contentWindow, window[0], "First frame should have correct window");
+  is(y.contentWindow, window[1], "Second frame should have correct window");
+
+  // set() hook test
+  window[1] = "FAIL";
+  is(window[1].name, "y", "Second frame is still y");
+  y.parentNode.removeChild(y);
+  ok(!("1" in window), "We no longer have two subframes");
+  is(window[1], undefined, "We should not have a value here");
+
+  // defineProperty() hook test
+  x.parentNode.appendChild(y);
+  Object.defineProperty(window, "1", { value: "FAIL2", configurable: true,
+				       writable: true })
+  y.parentNode.removeChild(y);
+  ok(!("1" in window), "We no longer have two subframes, again");
+  is(window[1], undefined, "We should not have a value here either");
+
+  // More set() hook test
+  window[1] = "PASS";
+  is(window[1], "PASS", "Should be able to set expando on window[1] now");
+  var desc = Object.getOwnPropertyDescriptor(window, "1");
+  ok(desc.configurable, "Expando should be configurable");
+  ok(desc.enumerable, "Expando should be configurable");
+  ok(desc.writable, "Expando should be writable");
+  is(desc.value, "PASS", "Expando should have correct value");
+
+  // Sneaky shadowing (get() hook)
+  x.parentNode.appendChild(y);
+  is(window[1], y.contentWindow, "Second frame should now shadow expando");
+  desc = Object.getOwnPropertyDescriptor(window, "1");
+  ok(desc.configurable, "Subframe should be configurable");
+  ok(desc.enumerable, "Subframe should be configurable");
+  ok(!desc.writable, "Subframe should not be writable");
+  is(desc.value, y.contentWindow, "Subframe should have correct value");
+
+  // And unshadowing
+  y.parentNode.removeChild(y);
+  is(window[1], "PASS", "And now we should be able to see the expando again");
+
+  // And more defineProperty()
+  Object.defineProperty(window, "1", { value: "PASS2", configurable: true,
+				       writable: true })
+  is(window[1], "PASS2", "Defining past end of list should work");
+
+  // Enumeration tests
+  x.parentNode.appendChild(y);
+
+  var names = Object.getOwnPropertyNames(window);
+  is(names[0], "0", "Must start with 0");
+  is(names[1], "1", "Must continue with 1");
+  isnot(names.indexOf("2"), -1, "And then 2, defined earlier, should be in there");
+  is(names.indexOf("3"), -1, "But no 3; that's on the proto");
+
+  names = [];
+  for (var name in window) {
+    names.push(name);
+  }
+  is(names[0], "0", "Enumeration must start with 0");
+  is(names[1], "1", "Enumeration must continue with 1");
+  isnot(names.indexOf("2"), -1, "Enumeration: and then 2, defined earlier");
+  isnot(names.indexOf("3"), -1, "Enumeration: and then 3, defined on the proto");
+  is(names.indexOf("4"), -1, "But no 4 around");
+
+  // Delete tests
+  delete window[1];
+  is(window[1], y.contentWindow, "Shouldn't be able to delete a supported index");
+  y.parentNode.removeChild(y);
+  is(window[1], "PASS2",
+     "And shouldn't have deleted the thing we were shadowing either");
+  delete window[1];
+  is(window[1], undefined,
+     "But should be able to delete unshadowed things");
+  </script>
+</body>
+</html>
--- a/js/xpconnect/tests/chrome/test_bug738244.xul
+++ b/js/xpconnect/tests/chrome/test_bug738244.xul
@@ -38,13 +38,22 @@ https://bugzilla.mozilla.org/show_bug.cg
            "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");
 
+        is(win[0].name, "frame1",
+           "IFrames cannot be found by index on the window through xray");
+
+        win["1000"] = "foopy";
+        ok(!("1000" in win), "Shouldn't be able to add indexed expandos to xray");
+        
+        win["1000a"] = "foopy";
+        ok("1000a" in win, "Should be able to add named expandos to xray");
+
         SimpleTest.finish();
       }
 
   ]]></script>
 </window>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -17,16 +17,17 @@
 
 #include "XPCWrapper.h"
 #include "xpcprivate.h"
 
 #include "jsapi.h"
 #include "nsJSUtils.h"
 
 #include "mozilla/dom/BindingUtils.h"
+#include "nsGlobalWindow.h"
 
 using namespace mozilla::dom;
 
 namespace xpc {
 
 using namespace js;
 
 static const uint32_t JSSLOT_RESOLVING = 0;
@@ -648,22 +649,29 @@ XPCWrappedNativeXrayTraits::resolveDOMCo
 
     if (pobj && !JS_GetPropertyDescriptorById(cx, holder, id, 0, desc))
         return false;
 
     return true;
 }
 
 template <typename T>
+static T*
+As(JSObject *wrapper)
+{
+    XPCWrappedNative *wn = XPCWrappedNativeXrayTraits::getWN(wrapper);
+    nsCOMPtr<T> native = do_QueryWrappedNative(wn);
+    return native;
+}
+
+template <typename T>
 static bool
 Is(JSObject *wrapper)
 {
-    XPCWrappedNative *wn = XPCWrappedNativeXrayTraits::getWN(wrapper);
-    nsCOMPtr<T> native = do_QueryWrappedNative(wn);
-    return !!native;
+    return !!As<T>(wrapper);
 }
 
 static nsQueryInterface
 do_QueryInterfaceNative(JSContext* cx, JSObject* wrapper);
 
 // Helper function to work around some limitations of the current XPC 
 // calling mechanism. See: bug 763897.
 // The idea is that we unwrap the 'this' object, and find the wrapped
@@ -940,16 +948,40 @@ XPCWrappedNativeXrayTraits::resolveOwnPr
                                                PropertyDescriptor *desc, unsigned flags)
 {
     // Call the common code.
     bool ok = XrayTraits::resolveOwnProperty(cx, jsWrapper, wrapper, holder,
                                              id, desc, flags);
     if (!ok || desc->obj)
         return ok;
 
+    // Check for indexed access on a window.
+    int32_t index = GetArrayIndexFromId(cx, id);
+    if (IsArrayIndex(index)) {
+        nsGlobalWindow* win =
+            static_cast<nsGlobalWindow*>(As<nsIDOMWindow>(wrapper));
+        // Note: As() unwraps outer windows to get to the inner window.
+        if (win) {
+            bool unused;
+            nsCOMPtr<nsIDOMWindow> subframe = win->IndexedGetter(index, unused);
+            if (subframe) {
+                nsGlobalWindow* global = static_cast<nsGlobalWindow*>(subframe.get());
+                global->EnsureInnerWindow();
+                JSObject* obj = global->FastGetGlobalJSObject();
+                if (MOZ_UNLIKELY(!obj)) {
+                    // It's gone?
+                    return xpc::Throw(cx, NS_ERROR_FAILURE);
+                }
+                desc->value = JS::ObjectValue(*obj);
+                mozilla::dom::FillPropertyDescriptor(desc, wrapper, true);
+                return JS_WrapPropertyDescriptor(cx, desc);
+            }
+        }
+    }
+
     // Xray wrappers don't use the regular wrapper hierarchy, so we should be
     // in the wrapper's compartment here, not the wrappee.
     MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     XPCJSRuntime* rt = nsXPConnect::GetRuntimeInstance();
     if (AccessCheck::isChrome(wrapper) &&
         (((id == rt->GetStringID(XPCJSRuntime::IDX_BASEURIOBJECT) ||
            id == rt->GetStringID(XPCJSRuntime::IDX_NODEPRINCIPAL)) &&
           Is<nsINode>(wrapper)) ||
@@ -1003,32 +1035,41 @@ XPCWrappedNativeXrayTraits::resolveOwnPr
 #endif
     }
 
     return true;
 }
 
 bool
 XPCWrappedNativeXrayTraits::defineProperty(JSContext *cx, JSObject *wrapper, jsid id,
-                                      PropertyDescriptor *desc, bool *defined)
+                                           PropertyDescriptor *desc, bool *defined)
 {
     *defined = false;
     JSObject *holder = singleton.ensureHolder(cx, wrapper);
     if (isResolving(cx, holder, id)) {
         if (!(desc->attrs & (JSPROP_GETTER | JSPROP_SETTER))) {
             if (!desc->getter)
                 desc->getter = holder_get;
             if (!desc->setter)
                 desc->setter = holder_set;
         }
 
         *defined = true;
         return JS_DefinePropertyById(cx, holder, id, desc->value, desc->getter, desc->setter,
                                      desc->attrs);
     }
+
+    // Check for an indexed property on a Window.  If that's happening, do
+    // nothing but claim we defined it so it won't get added as an expando.
+    int32_t index = GetArrayIndexFromId(cx, id);
+    if (IsArrayIndex(index) && Is<nsIDOMWindow>(wrapper)) {
+        *defined = true;
+        return true;
+    }
+
     return true;
 }
 
 bool
 XPCWrappedNativeXrayTraits::enumerateNames(JSContext *cx, JSObject *wrapper, unsigned flags,
                                            JS::AutoIdVector &props)
 {
     // Force all native properties to be materialized onto the wrapped native.