Fix for bug 693258 (Fixes for test failures when turning off new list DOM bindings). r=bz/Waldo.
authorPeter Van der Beken <peterv@propagandism.org>
Thu, 13 Oct 2011 15:36:09 +0200
changeset 79012 da852a7882b5240265b4ab27c2ad084c99669616
parent 79011 f7ed9d64a8d7f1fe382830e201c21d06c0585347
child 79013 159a1ce7cabc8a2ead7a1ee33312059251268705
push id2805
push userpvanderbeken@mozilla.com
push dateThu, 20 Oct 2011 09:43:38 +0000
treeherdermozilla-inbound@da852a7882b5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, Waldo
bugs693258
milestone10.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
Fix for bug 693258 (Fixes for test failures when turning off new list DOM bindings). r=bz/Waldo.
dom/base/nsDOMClassInfo.cpp
dom/base/nsDOMClassInfo.h
dom/base/nsWrapperCache.h
dom/tests/mochitest/bugs/test_bug633133.html
js/src/jsarray.cpp
js/src/jsarray.h
js/src/jsfriendapi.h
js/xpconnect/src/dombindings.cpp
js/xpconnect/tests/chrome/test_nodelists.xul
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -7976,16 +7976,41 @@ nsNamedNodeMapSH::GetNamedItem(nsISuppor
   *aCache = attr = map->GetNamedItem(aName, aResult);
   return attr;
 }
 
 
 // HTMLCollection helper
 
 nsresult
+nsHTMLCollectionSH::PreCreate(nsISupports *nativeObj, JSContext *cx,
+                              JSObject *globalObj, JSObject **parentObj)
+{
+  nsIHTMLCollection* list = static_cast<nsIHTMLCollection*>(nativeObj);
+#ifdef DEBUG
+  {
+    nsCOMPtr<nsIHTMLCollection> list_qi = do_QueryInterface(nativeObj);
+
+    // If this assertion fires the QI implementation for the object in
+    // question doesn't use the nsIHTMLCollection pointer as the nsISupports
+    // pointer. That must be fixed, or we'll crash...
+    NS_ASSERTION(list_qi == list, "Uh, fix QI!");
+  }
+#endif
+
+  nsINode* native_parent = list->GetParentObject();
+
+  nsresult rv =
+    WrapNativeParent(cx, globalObj, native_parent, native_parent, parentObj);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_SUCCESS_ALLOW_SLIM_WRAPPERS;
+}
+
+nsresult
 nsHTMLCollectionSH::GetLength(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                               JSObject *obj, PRUint32 *length)
 {
   nsIHTMLCollection* collection =
     static_cast<nsIHTMLCollection*>(GetNative(wrapper, obj));
 #ifdef DEBUG
   {
     nsCOMPtr<nsIHTMLCollection> collection_qi =
--- a/dom/base/nsDOMClassInfo.h
+++ b/dom/base/nsDOMClassInfo.h
@@ -727,16 +727,19 @@ protected:
 
   // Override nsNamedArraySH::GetNamedItem()
   virtual nsISupports* GetNamedItem(nsISupports *aNative,
                                     const nsAString& aName,
                                     nsWrapperCache **cache,
                                     nsresult *aResult);
 
 public:
+  NS_IMETHOD PreCreate(nsISupports *nativeObj, JSContext *cx,
+                       JSObject *globalObj, JSObject **parentObj);
+
   static nsIClassInfo *doCreate(nsDOMClassInfoData* aData)
   {
     return new nsHTMLCollectionSH(aData);
   }
 };
 
 
 // ContentList helper
--- a/dom/base/nsWrapperCache.h
+++ b/dom/base/nsWrapperCache.h
@@ -153,17 +153,25 @@ public:
 
   bool PreservingWrapper()
   {
     return (mWrapperPtrBits & WRAPPER_BIT_PRESERVED) != 0;
   }
 
   void SetIsProxy()
   {
-    mWrapperPtrBits |= WRAPPER_IS_PROXY;
+    NS_ASSERTION(!mWrapperPtrBits,
+                 "This flag should be set before creating any wrappers.");
+    mWrapperPtrBits = WRAPPER_IS_PROXY;
+  }
+  void ClearIsProxy()
+  {
+    NS_ASSERTION(!mWrapperPtrBits || mWrapperPtrBits == WRAPPER_IS_PROXY,
+                 "This flag should be cleared before creating any wrappers.");
+    mWrapperPtrBits = 0;
   }
 
   bool IsProxy() const
   {
     return (mWrapperPtrBits & WRAPPER_IS_PROXY) != 0;
   }
 
 
--- a/dom/tests/mochitest/bugs/test_bug633133.html
+++ b/dom/tests/mochitest/bugs/test_bug633133.html
@@ -23,17 +23,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 <script type="application/javascript">
 
 /** Test for Bug 633133 **/
 
 var divCollection = document.getElementsByTagName('div');
 
 ok("foo" in divCollection, "'foo' should be in the div collection");
 ok("bar" in divCollection, "'bar' should be in the div collection");
-ok("" in divCollection, "empty string should be in the div collection");
+ok(!("" in divCollection), "empty string shouldn't be in the div collection");
 ok(!("foobar" in divCollection), "'foobar' shouldn't be in the div collection");
 
 var select = $('select');
 is(select[0].text, "option1", "select elements work");
 Math.sin();
 ok(1 in select, "in works");
 is(select[1].text, "option2", "can get it too");
 ok(!(2 in select), "in works for elements out of range");
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -192,17 +192,17 @@ namespace js {
  * 
  * In our implementation, it would be sufficient to check for JSVAL_IS_INT(id)
  * except that by using signed 31-bit integers we miss the top half of the
  * valid range. This function checks the string representation itself; note
  * that calling a standard conversion routine might allow strings such as
  * "08" or "4.0" as array indices, which they are not.
  *
  */
-bool
+JS_FRIEND_API(bool)
 StringIsArrayIndex(JSLinearString *str, jsuint *indexp)
 {
     const jschar *s = str->chars();
     uint32 length = str->length();
     const jschar *end = s + length;
 
     if (length == 0 || length > (sizeof("4294967294") - 1) || !JS7_ISDEC(*s))
         return false;
--- a/js/src/jsarray.h
+++ b/js/src/jsarray.h
@@ -63,20 +63,16 @@ JSObject::isPackedDenseArray()
 {
     JS_ASSERT(isDenseArray());
     return flags & PACKED_ARRAY;
 }
 
 namespace js {
 /* 2^32-2, inclusive */
 const uint32 MAX_ARRAY_INDEX = 4294967294u;
-    
-extern bool
-StringIsArrayIndex(JSLinearString *str, jsuint *indexp);
-    
 }
 
 inline JSBool
 js_IdIsIndex(jsid id, jsuint *indexp)
 {
     if (JSID_IS_INT(id)) {
         jsint i;
         i = JSID_TO_INT(id);
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -329,16 +329,19 @@ static inline js::StrictPropertyOp
 CastAsJSStrictPropertyOp(JSObject *object)
 {
     return JS_DATA_TO_FUNC_PTR(js::StrictPropertyOp, object);
 }
 
 JS_FRIEND_API(bool)
 GetPropertyNames(JSContext *cx, JSObject *obj, uintN flags, js::AutoIdVector *props);
 
+JS_FRIEND_API(bool)
+StringIsArrayIndex(JSLinearString *str, jsuint *indexp);
+
 /*
  * NB: these flag bits are encoded into the bytecode stream in the immediate
  * operand of JSOP_ITER, so don't change them without advancing jsxdrapi.h's
  * JSXDR_BYTECODE_VERSION.
  */
 #define JSITER_ENUMERATE  0x1   /* for-in compatible hidden default iterator */
 #define JSITER_FOREACH    0x2   /* return [key, value] pair rather than key */
 #define JSITER_KEYVALUE   0x4   /* destructuring for-in wants [key, value] */
--- a/js/xpconnect/src/dombindings.cpp
+++ b/js/xpconnect/src/dombindings.cpp
@@ -482,16 +482,18 @@ ListBase<LC>::create(JSContext *cx, XPCW
     if (js::GetObjectGlobal(parent) != scope->GetGlobalJSObject()) {
         if (!ac.enter(cx, parent))
             return NULL;
 
         scope = XPCWrappedNativeScope::FindInJSObjectScope(cx, parent);
     }
 
     JSObject *proto = getPrototype(cx, scope, triedToWrap);
+    if (!proto && !*triedToWrap)
+        aWrapperCache->ClearIsProxy();
     if (!proto)
         return NULL;
     JSObject *obj = NewProxyObject(cx, &ListBase<LC>::instance,
                                    PrivateValue(aList), proto, parent);
     if (!obj)
         return NULL;
 
     NS_ADDREF(aList);
@@ -534,16 +536,19 @@ GetArrayIndexFromId(JSContext *cx, jsid 
         return JSID_TO_INT(id);
     if (NS_LIKELY(id == s_length_id))
         return -1;
     if (NS_LIKELY(JSID_IS_ATOM(id))) {
         JSAtom *atom = JSID_TO_ATOM(id);
         jschar s = *atom->chars();
         if (NS_LIKELY((unsigned)s >= 'a' && (unsigned)s <= 'z'))
             return -1;
+
+        jsuint i;
+        return js::StringIsArrayIndex(JSID_TO_ATOM(id), &i) ? i : -1;
     }
     return IdToInt32(cx, id);
 }
 
 static void
 FillPropertyDescriptor(PropertyDescriptor *desc, JSObject *obj, jsval v, bool readonly)
 {
     desc->obj = obj;
--- a/js/xpconnect/tests/chrome/test_nodelists.xul
+++ b/js/xpconnect/tests/chrome/test_nodelists.xul
@@ -25,17 +25,20 @@
         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");
+        if (SpecialPowers.getBoolPref("dom.new_bindings"))
+            ok(!("2" in list), "in operator doesn't see phantom element");
+        else
+            todo(!("2" in list), "in operator doesn't see phantom element");
         is(list[2], undefined, "no node there!");
         SimpleTest.finish();
       }
   ]]></script>
 
   <iframe id="ifr"
           src="http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/file_nodelists.html"
           onload="go()" />