Add Future.timeout.

Add Future.timeout.

R=floitsch@google.com, nweiz@google.com

Committed: https://code.google.com/p/dart/source/detail?r=30745

[Lasse Reichstein Nielsen, 2013-11-27 10:27:34]

WDYT?

[floitsch, 2013-11-27 12:31:15]

LGTM.

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

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future.dart#new...
sdk/lib/async/future.dart:417: * If this future does not complete before
`timeLimit` has passed,
New line before?

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future.dart#new...
sdk/lib/async/future.dart:419: * returns or thorws) is used as the result of the
returned future.
throws

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future.dart#new...
sdk/lib/async/future.dart:421: Future timeout(Duration timeLimit, onTimeout());
I think onTimeout should be optional.

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

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future_impl.dar...
sdk/lib/async/future_impl.dart:549: zone.run(() {
I think it would be nicer if you passed "onTimeout" to the zone.
so:
try {
  result._complete(zone.run(onTimeout));
} catch (e, s) {
  result._completeError(e, s);
}

This way the zone gets the function that was registered.

[Lasse Reichstein Nielsen, 2013-11-27 12:54:20]

PTAL

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

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future.dart#new...
sdk/lib/async/future.dart:417: * If this future does not complete before
`timeLimit` has passed,
On 2013/11/27 12:31:16, floitsch wrote:
> New line before?

Done.

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future.dart#new...
sdk/lib/async/future.dart:419: * returns or thorws) is used as the result of the
returned future.
On 2013/11/27 12:31:16, floitsch wrote:
> throws

Done.

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future.dart#new...
sdk/lib/async/future.dart:421: Future timeout(Duration timeLimit, onTimeout());
On 2013/11/27 12:31:16, floitsch wrote:
> I think onTimeout should be optional.

Done.

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

https://codereview.chromium.org/91213002/diff/1/sdk/lib/async/future_impl.dar...
sdk/lib/async/future_impl.dart:549: zone.run(() {
Good point. Done.

[floitsch, 2013-11-27 13:05:48]

Still LGTM.
Add CCs for the affected files.

[Lasse Reichstein Nielsen, 2013-11-27 13:08:48]

Adding nweiz.

[nweiz, 2013-11-27 20:50:36]

It seems a little weird to me to make this an instance method on Future rather
than a static method, but the code changes lgtm.

https://codereview.chromium.org/91213002/diff/70001/pkg/barback/lib/src/cance...
File pkg/barback/lib/src/cancelable_future.dart (right):

https://codereview.chromium.org/91213002/diff/70001/pkg/barback/lib/src/cance...
pkg/barback/lib/src/cancelable_future.dart:36:
_completer.future.timeout(timeLimit, onTimeout);
Style nit: indent two spaces. Same for other files.

[Lasse Reichstein Nielsen, 2013-11-28 08:11:23]

https://codereview.chromium.org/91213002/diff/70001/pkg/barback/lib/src/cance...
File pkg/barback/lib/src/cancelable_future.dart (right):

https://codereview.chromium.org/91213002/diff/70001/pkg/barback/lib/src/cance...
pkg/barback/lib/src/cancelable_future.dart:36:
_completer.future.timeout(timeLimit, onTimeout);
Done, for consistency, but I'm fairly sure the style guide wants four spaces
here. It's a continued line, and not a nested code block. The one example that
uses the syntax is:
bobLikes() =>
    isDeepFried || (hasPieCrust && !vegan) || containsBacon;
and it uses four spaces.

[Lasse Reichstein Nielsen, 2013-11-28 08:16:12]

Committed patchset #7 manually as r30745 (presubmit successful).