Simplify future-propagation for whenComplete.

Simplify future-propagation for whenComplete.

R=floitsch@google.com

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

[Lasse Reichstein Nielsen, 2015-12-09 08:55:30]

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

[Lasse Reichstein Nielsen, 2015-12-09 12:45:26]

This clean-up is in anticipation of trying, once again, to handle the
long-future-chain memory leak. I had to understand the code first.

[floitsch, 2015-12-09 22:52:02]

LGTM.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:475: // doesn't have callbacks, so this is a
significant optimization.
don't

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:492: bool handleValueCallback() {
doesn't return anything anymore.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:504: bool handleError() {
doesn't return anything anymore.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:536: return;
no need for the return here.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:564: // We have to wait for the compelteResult
future to complete
completeResult

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:595: // can be expensive, we're trying to avoid
it.
Not sure if is-checks really are that expensive.
is-tests on Future are everywhere, so might as well just do an is-check here.

[Lasse Reichstein Nielsen, 2015-12-10 06:52:27]

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:492: bool handleValueCallback() {
On 2015/12/09 22:52:01, floitsch wrote:
> doesn't return anything anymore.

Done.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:504: bool handleError() {
On 2015/12/09 22:52:02, floitsch wrote:
> doesn't return anything anymore.

Done.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:536: return;
On 2015/12/09 22:52:02, floitsch wrote:
> no need for the return here.

Done.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:564: // We have to wait for the compelteResult
future to complete
Good catch. Done.

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:595: // can be expensive, we're trying to avoid
it.
On 2015/12/09 22:52:02, floitsch wrote:
> Not sure if is-checks really are that expensive.
> is-tests on Future are everywhere, so might as well just do an is-check here.

Done.

[floitsch, 2015-12-10 07:24:41]

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/1510053004/diff/60001/sdk/lib/async/future_im...
sdk/lib/async/future_impl.dart:564: // We have to wait for the compelteResult
future to complete
On 2015/12/10 06:52:26, Lasse Reichstein Nielsen wrote:
> Good catch. Done.

Your bar is low for "good catches" ;)

[Lasse Reichstein Nielsen, 2015-12-10 08:15:48]

Description was changed from

==========
Simplify future-propagation for whenComplete.
==========

to

==========
Simplify future-propagation for whenComplete.

R=floitsch@google.com

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

[Lasse Reichstein Nielsen, 2015-12-10 08:15:49]

Committed patchset #5 (id:80001) manually as
e038407e27bd41cfba095267614c58958f2c218e (presubmit successful).

[eernst, 2015-12-10 15:07:23]

On 2015/12/10 08:15:49, Lasse Reichstein Nielsen wrote:
> Committed patchset #5 (id:80001) manually as
> e038407e27bd41cfba095267614c58958f2c218e (presubmit successful).

After a short discussion in meat space, Lasse and I agreed that I should finish
reading this CL, understanding it as far as I could and checking for possible
glitches. Did that, did not find anything suspicious.