Futures.wait should call then, even if some(or all) of the futures are done

corelib/src/future.dart

Index: corelib/src/future.dart
===================================================================
--- corelib/src/future.dart	(revision 3022)
+++ corelib/src/future.dart	(working copy)
@@ -132,16 +132,27 @@
     // As each future completes, put its value into the corresponding
     // position in the list of values.
     for (int i = 0; i < futures.length; i++) {
-      // TODO(mattsh) - remove this after bug
-      // http://code.google.com/p/dart/issues/detail?id=333 is fixed.
-      int pos = i;
-      futures[pos].then((Object value) {
-        values[pos] = value;
-        if (--remaining == 0) {
-          completer.complete(values);
-        }
-      });
+      if (futures[i].isComplete) {

[Siggi Cherem (dart-lang), 2012/01/10 18:56:43]

(sorry for the slow reply I was OOO until today)

We might be able to simplify this change slightly.

We need overall more tests for this code (thanks for the new tests!), but I
believe many of the tests you added, except [testWaitEmpty] worked before your
change too. I believe all other test were passing before because [Future.then]
internally calls the complete handler immediately if the future is already
completed (basically the same code you have inlined here in this branch).

I believe we can simplify this by removing this conditional (reverting the body
of the loop as before), but changing line 152 below to say 'futures.length == 0'
rather than 'remaining == 0'.

[Anders Johnsen, 2012/01/10 19:21:39]

I was having problems with futures being complete not responding, so I'll
investigate that a bit further.

And yes, that is a nicer way to write it.

Thank you for the review!

+        values[i] = futures[i].value;
+        remaining--;
+      } else {
+        // TODO(mattsh) - remove this after bug
+        // http://code.google.com/p/dart/issues/detail?id=333 is fixed.
+        int pos = i;
+        futures[pos].then((Object value) {
+          values[pos] = value;
+          if (--remaining == 0) {
+            completer.complete(values);
+          }
+        });
+      }
     }
+    // Special case where all the futures are already completed,
+    // trigger the value now.
+    if (remaining == 0) {
+      completer.complete(values);
+    }
+
     return completer.future;
   }
 }