Fix Iterator assertion, add tests, bug 589112. r=dmandelin

js/src/jsiter.cpp

@@ -123,16 +123,11 @@ NativeIterator::mark(JSTracer *trc)
         MarkObject(trc, obj, "obj");
 }
 
-/*
- * Shared code to close iterator's state either through an explicit call or
- * when GC detects that the iterator is no longer reachable.
- */
 static void
 iterator_finalize(JSContext *cx, JSObject *obj)
 {
     JS_ASSERT(obj->getClass() == &js_IteratorClass);
 
-    /* Avoid double work if the iterator was closed by JSOP_ENDITER. */
     NativeIterator *ni = obj->getNativeIterator();
     if (ni) {
         cx->free(ni);
@@ -511,13 +506,13 @@ NativeIterator::init(JSObject *obj, uintN flags, uint32 slength, uint32 key)
 static inline void
 RegisterEnumerator(JSContext *cx, JSObject *iterobj, NativeIterator *ni)
 {
-    JS_ASSERT(!(ni->flags & JSITER_ACTIVE));
-    ni->flags |= JSITER_ACTIVE;
-
     /* Register non-escaping native enumerators (for-in) with the current context. */
     if (ni->flags & JSITER_ENUMERATE) {
         ni->next = cx->enumerators;
         cx->enumerators = iterobj;
+
+        JS_ASSERT(!(ni->flags & JSITER_ACTIVE));
+        ni->flags |= JSITER_ACTIVE;
     }
 }
 
@@ -849,17 +844,19 @@ js_CloseIterator(JSContext *cx, JSObject *obj)
         /* Remove enumerators from the active list, which is a stack. */
         NativeIterator *ni = obj->getNativeIterator();
 
-        JS_ASSERT(ni->flags & JSITER_ACTIVE);
-        ni->flags &= ~JSITER_ACTIVE;
-
         if (ni->flags & JSITER_ENUMERATE) {
             JS_ASSERT(cx->enumerators == obj);
             cx->enumerators = ni->next;
-        }
 
-        /* Reset the enumerator; it may still be in the cached iterators
-         * for this thread, and can be reused. */
-        ni->props_cursor = ni->props_array;
+            JS_ASSERT(ni->flags & JSITER_ACTIVE);
+            ni->flags &= ~JSITER_ACTIVE;
+
+            /*
+             * Reset the enumerator; it may still be in the cached iterators
+             * for this thread, and can be reused.
+             */
+            ni->props_cursor = ni->props_array;
+        }
     }
 #if JS_HAS_GENERATORS
     else if (clasp == &js_GeneratorClass) {

js/src/jsiter.h

@@ -58,7 +58,10 @@
 #define JSITER_OWNONLY    0x8   /* iterate over obj's own properties only */
 #define JSITER_HIDDEN     0x10  /* also enumerate non-enumerable properties */
 
-/* Whether the iterator is currently active. Not serialized by XDR. */
+/*
+ * For cacheable native iterators, whether the iterator is currently active.
+ * Not serialized by XDR.
+ */
 #define JSITER_ACTIVE     0x1000
 
 struct NativeIterator {

js/src/tests/js1_7/extensions/jstests.list

@@ -65,3 +65,5 @@ script regress-474771-01.js
 script regress-474771-02.js
 script regress-476257.js
 script regress-477048.js
+script regress-589112.js
+script regress-590813.js

js/src/tests/js1_7/extensions/regress-589112.js

@@ -0,0 +1,6 @@
+
+f = eval("(function(){return x=Iterator(/x/)})")
+for (a in f()) {}
+for (d in x) {}
+
+reportCompare(0, 0, "");

js/src/tests/js1_7/extensions/regress-590813.js

@@ -0,0 +1,23 @@
+
+var actual = '';
+var expected = 'A0B1B2C0C1C2';
+
+var x = Iterator([1,2,3], true);
+
+for (var a in x) {
+  actual += 'A' + a;
+  for (var b in x) {
+    actual += 'B' + b;
+  }
+}
+
+var y = Iterator([1,2,3], true);
+
+for (var c in y) {
+  actual += 'C' + c;
+}
+for (var d in y) {
+  actual += 'D' + d;
+}
+
+reportCompare(expected, actual, "Handle nested Iterator iteration right");