Bug 648801 (new DOM list bindings) - Add holder to proxy Xray and make the new nodelist deal with string IDs. r=bz/jst/mrbkap.

--HG--
extra : rebase_source : e311fe95456fb253313e3849e7bc926be385e17b

dom/base/nsDOMClassInfo.h

@@ -194,12 +194,14 @@ protected:
   nsresult ResolveConstructor(JSContext *cx, JSObject *obj,
                               JSObject **objp);
 
+public:
   // Checks if id is a number and returns the number, if aIsNumber is
   // non-null it's set to true if the id is a number and false if it's
   // not a number. If id is not a number this method returns -1
   static PRInt32 GetArrayIndexFromId(JSContext *cx, jsid id,
                                      bool *aIsNumber = nsnull);
 
+protected:
   static inline bool IsReadonlyReplaceable(jsid id)
   {
     return (id == sParent_id       ||

js/src/xpconnect/src/dombindings.cpp

@@ -310,8 +310,9 @@ NodeList<T>::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, b
                                       PropertyDescriptor *desc)
 {
     // FIXME: expandos
-    int32 index;
-    if (JSID_IS_INT(id) && ((index = JSID_TO_INT(id)) >= 0)) {
+    bool isNumber;
+    int32 index = nsDOMClassInfo::GetArrayIndexFromId(cx, id, &isNumber);
+    if (isNumber && index >= 0) {
         T *nodeList = getNodeList(proxy);
         nsIContent *result = nodeList->GetNodeAt(PRUint32(index));
         if (result) {
@@ -399,8 +400,9 @@ bool
 NodeList<T>::hasOwn(JSContext *cx, JSObject *proxy, jsid id, bool *bp)
 {
     // FIXME: expandos
-    int32 index;
-    if (JSID_IS_INT(id) && ((index = JSID_TO_INT(id)) >= 0)) {
+    bool isNumber;
+    int32 index = nsDOMClassInfo::GetArrayIndexFromId(cx, id, &isNumber);
+    if (isNumber && index >= 0) {
         if (getNodeList(proxy)->GetNodeAt(PRUint32(index))) {
             *bp = true;
             return true;
@@ -505,8 +507,9 @@ bool
 NodeList<T>::get(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, Value *vp)
 {
     // FIXME: expandos
-    int32 index;
-    if (JSID_IS_INT(id) && ((index = JSID_TO_INT(id)) >= 0)) {
+    bool isNumber;
+    int32 index = nsDOMClassInfo::GetArrayIndexFromId(cx, id, &isNumber);
+    if (isNumber && index >= 0) {
         T *nodeList = getNodeList(proxy);
         nsIContent *result = nodeList->GetNodeAt(PRUint32(index));
         if (result)

js/src/xpconnect/tests/chrome/test_nodelists.xul

@@ -22,6 +22,16 @@
         var list = win.document.getElementsByTagName('p');
         is(list.length, 3, "can get the length");
         ok(list[0].toString().indexOf("[object HTMLParagraphElement") >= 0, "can get list[0]");
+        is(list[0], list.item(0), "list.item works");
+        is(list.item, list.item, "don't recreate functions for each get");
+
+        var list2 = list[2];
+        ok(list[2].toString().indexOf("[object HTMLParagraphElement"), "list[2] exists");
+        ok("2" in list, "in operator works");
+
+        is(win.document.body.removeChild(win.document.body.lastChild), list2, "remove last paragraph element");
+        ok(!("2" in list), "in operator doesn't see phantom element");
+        is(list[2], undefined, "no node there!");
         SimpleTest.finish();
       }
   ]]></script>

js/src/xpconnect/wrappers/XrayWrapper.cpp

@@ -1060,11 +1060,43 @@ XrayProxy::~XrayProxy()
 {
 }
 
+static JSObject *
+GetHolderObject(JSContext *cx, JSObject *wrapper)
+{
+    if (!js::GetProxyExtra(wrapper, 0).isUndefined())
+        return &js::GetProxyExtra(wrapper, 0).toObject();
+
+    JSObject *obj = JS_NewObjectWithGivenProto(cx, nsnull, nsnull, js::GetObjectGlobal(wrapper));
+    if (!obj)
+        return nsnull;
+    js::SetProxyExtra(wrapper, 0, ObjectValue(*obj));
+    return obj;
+}
+
 bool
 XrayProxy::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id,
                                  bool set, js::PropertyDescriptor *desc)
 {
-    return Proxy::getPropertyDescriptor(cx, wrapper, id, set, desc);
+    JSObject *holder = GetHolderObject(cx, wrapper);
+    if (!holder)
+        return false;
+
+    if (!JS_GetPropertyDescriptorById(cx, holder, id, JSRESOLVE_QUALIFIED, desc))
+        return false;
+    if (desc->obj) {
+        desc->obj = wrapper;
+        return true;
+    }
+
+    if (!Proxy::getPropertyDescriptor(cx, wrapper, id, set, desc))
+        return false;
+
+    if (!desc->obj)
+        return true;
+
+    desc->obj = wrapper;
+    return JS_DefinePropertyById(cx, holder, id, desc->value, desc->getter, desc->setter,
+                                 desc->attrs);
 }
 
 bool