Make _Future._chainForeignFuture not cause an assert.

Make _Future._chainForeignFuture not cause an assert.

If the foreign future completes with a future, the following call to
_Future._completeWithValue would hit an assert in checked mode
(value is! Future). This change makes the _Future._completeWithValue flatten
incoming nesting futures before completing the _Future, ensuring that a
_Future never has a Future as value.

An alternative is to throw if the value of a future is another future.
That's what the assert does in checked mode, but we shouldn't be able to hit
an assert.

R=floitsch@google.com

Committed: https://github.com/dart-lang/sdk/commit/63b71b4c241e8dbfa62fbb10aec71afe8e592694

[Lasse Reichstein Nielsen, 2016-05-10 08:41:55]

lrn@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2016-05-10 15:39:51]

LGTM.

https://codereview.chromium.org/1966523003/diff/1/sdk/lib/async/future_impl.dart
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/1966523003/diff/1/sdk/lib/async/future_impl.d...
sdk/lib/async/future_impl.dart:415: // so use _complete in stead of
_completeWithValue.
instead

https://codereview.chromium.org/1966523003/diff/1/tests/lib/async/future_test...
File tests/lib/async/future_test.dart (right):

https://codereview.chromium.org/1966523003/diff/1/tests/lib/async/future_test...
tests/lib/async/future_test.dart:1001: f.then((v) { Expect.isTrue(v is Future);
});  // Feature is designed that way.
long line.

https://codereview.chromium.org/1966523003/diff/1/tests/lib/async/future_test...
tests/lib/async/future_test.dart:1126: class Feature implements Future<dynamic>
{
Please rename. It took me several attempts to read the comments for the test.

[Lasse Reichstein Nielsen, 2016-05-11 10:19:37]

https://codereview.chromium.org/1966523003/diff/1/sdk/lib/async/future_impl.dart
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/1966523003/diff/1/sdk/lib/async/future_impl.d...
sdk/lib/async/future_impl.dart:415: // so use _complete in stead of
_completeWithValue.
On 2016/05/10 15:39:51, floitsch wrote:
> instead

Done.

https://codereview.chromium.org/1966523003/diff/1/tests/lib/async/future_test...
File tests/lib/async/future_test.dart (right):

https://codereview.chromium.org/1966523003/diff/1/tests/lib/async/future_test...
tests/lib/async/future_test.dart:1001: f.then((v) { Expect.isTrue(v is Future);
});  // Feature is designed that way.
On 2016/05/10 15:39:51, floitsch wrote:
> long line.

Done.

https://codereview.chromium.org/1966523003/diff/1/tests/lib/async/future_test...
tests/lib/async/future_test.dart:1126: class Feature implements Future<dynamic>
{
Done. Now being clever in a more easily distinguishable way.

[Lasse Reichstein Nielsen, 2016-05-11 11:14:10]

Description was changed from

==========
Make _Future._chainForeignFuture not cause an assert.

If the foreign future completes with a future, the following call to
_Future._completeWithValue would hit an assert in checked mode
(value is! Future). This change makes the _Future._completeWithValue flatten
incoming nesting futures before completing the _Future, ensuring that a
_Future never has a Future as value.

An alternative is to throw if the value of a future is another future.
That's what the assert does in checked mode, but we shouldn't be able to hit
an assert.
==========

to

==========
Make _Future._chainForeignFuture not cause an assert.

If the foreign future completes with a future, the following call to
_Future._completeWithValue would hit an assert in checked mode
(value is! Future). This change makes the _Future._completeWithValue flatten
incoming nesting futures before completing the _Future, ensuring that a
_Future never has a Future as value.

An alternative is to throw if the value of a future is another future.
That's what the assert does in checked mode, but we shouldn't be able to hit
an assert.

R=floitsch@google.com

Committed:
https://github.com/dart-lang/sdk/commit/63b71b4c241e8dbfa62fbb10aec71afe8e592694
==========

[Lasse Reichstein Nielsen, 2016-05-11 11:14:12]

Committed patchset #2 (id:20001) manually as
63b71b4c241e8dbfa62fbb10aec71afe8e592694 (presubmit successful).