Fix ZoneAwarePromise.all to resolve at the correct time

[trevorade]

For ZoneAwarePromise.all, as implemented, the count variable serves three purposes:

1. Count the number of values passed-in
2. Specify the index of a resolved value in resolvedValues
3. Track when all the promises have been resolved.

In the event that value.then is immediately called, count will be decremented at the incorrect time resulting in a potentially negative value or reaching 0 when not all values have actually been resolved.

The fix is to be more meticulous about using the correct indices at the correct time and to not overload the count and number of resolved promises. This updated version is more explicit about the purpose of the unresolvedCount and valueIndex variables. unresolvedCount needs to start at 2 to prevent resolve from being called too soon in the event that value.then calls the callback immediately. The scoping of the index for use in resolvedValues is made constant to guarantee the correct index is used.

This buggy behavior specifically manifested as an issue in the Monaco editor but most likely occurs elsewhere as well in cases where promises may immediately call into .then.

Related issue:
microsoft/monaco-editor#790 (comment)

This fixes #1077

[evmar]

cc @rkirov

[evmar]

Maybe mention this fixes #1077

[trevorade]

The continuous-integration/travis-ci/pr failure appears to be a disconnect. I didn't see any failing tests. I don't know how to re-run those tests.

[JiaLiPassion]

I remembered some other guy get the same issue before, I think this is the bug of Monaco editor.
The callback should not be immediately called. It will obey the promise spec if my understandings are correct.
#1077

[evmar]

Igor says that @mhevery is the right person to review this

[evmar]

Also at a higher level, why do we need to reimplement these? Why can't we have our monkeypatches reuse the builtins?

[JiaLiPassion]

should we add some test cases about this? I can do that in another PR.

[JiaLiPassion]

@evmar, yeah, I think if we only need to keep the zone information, we can just monkey-patch the constructor and then. I believe the main reason is we want to have our own microTaskQueue, so we can run the microTask inside this queue after other macroTask(setTimeout) is invoked, is that correct, @mhevery?

[trevorade]

@mhevery, this appears ready to go. Not sure what the next step is for this.