Add missing null-tests to async error functions.

Add missing null-tests to async error functions.

Also treat null errors comming out of Zone.errorCallback as
NullThrownError.

This should prevent, as was always the intention, any async error
from having a null value.
This is important for async/await syntax, where the distinction
between sync and async errors is removed.

(For Dart 2.0, we could just make null throwable).

R=sgjesse@google.com

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

[Lasse Reichstein Nielsen, 2014-09-24 12:28:39]

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

[floitsch, 2014-09-24 12:59:13]

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

[floitsch, 2014-09-24 12:59:14]

DBC.

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
File sdk/lib/async/broadcast_stream_controller.dart (right):

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
sdk/lib/async/broadcast_stream_controller.dart:241: if (error == null) throw new
ArgumentError("Error must not be null");
Or make it an asynchronous NullThrownError?

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

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/future.dart#ne...
sdk/lib/async/future.dart:195: if (error == null) throw new ArgumentError("Error
must not be null");
Or make it an asynchronous "NullThrownError" ?

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

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/zone.dart#newc...
sdk/lib/async/zone.dart:45: if (error == null) throw new ArgumentError("Error
must not be null");
Or make it a NullThrownError?

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/zone.dart#newc...
sdk/lib/async/zone.dart:260: /** The root zone that is implicitly created. */
why this change?

[Søren Gjesse, 2014-09-24 13:29:31]

lgtm,

As we can't totally avoid nulls Florians suggestion might be the right thing to
do.

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
File sdk/lib/async/broadcast_stream_controller.dart (right):

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
sdk/lib/async/broadcast_stream_controller.dart:243: AsyncError replacement =
Zone.current.errorCallback(error, stackTrace);
The five lines seems to be duplicated in a number of places, refactor?

AsyncError replacement = Zone.current.errorCallback(error, stackTrace);
if (replacement != null) {
  error = replacement.error;
  if (error == null) error = new NullThrownError();
  stackTrace = replacement.stackTrace;
}

[Lasse Reichstein Nielsen, 2014-09-24 13:49:58]

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
File sdk/lib/async/broadcast_stream_controller.dart (right):

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
sdk/lib/async/broadcast_stream_controller.dart:241: if (error == null) throw new
ArgumentError("Error must not be null");
I can do that everywhere. It would be consistent behavior, the only problem
would be that it wouldn't throw early if the value was null by error.

Let's do it.

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/broadcast_stre...
sdk/lib/async/broadcast_stream_controller.dart:243: AsyncError replacement =
Zone.current.errorCallback(error, stackTrace);
I wish that I could, but it really just conditionally unpacks into local
variables, which is not something that can be abstracted away.
The only reusable option part would be a 'getAsyncError(AsyncError e) => e.error
== null ? new NullThrownError() : e.error;' to save one line each place. I'm not
sure it's worth it - but let's try and see.

I can't assume that the AsyncError is the current implementation, so it can't
prevent null errors, I have to check after extracting it.

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

https://codereview.chromium.org/598993002/diff/1/sdk/lib/async/zone.dart#newc...
sdk/lib/async/zone.dart:260: /** The root zone that is implicitly created. */
Just because the comment style was inconsistent with the rest of the file.

[Lasse Reichstein Nielsen, 2014-09-25 11:01:00]

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