Change singleSubscription/multiSubscription to normal/broadcast.

Change singleSubscription/multiSubscription to normal/broadcast.

Also make StreamController not a Stream.

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

[floitsch, 2013-01-22 14:37:56]

LGTM.

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
File sdk/lib/async/stream_controller.dart (left):

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
sdk/lib/async/stream_controller.dart:24: class StreamController<T> extends
Stream<T> implements StreamSink<T> {
This looks like an unrelated change.
At least update the description?

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
File sdk/lib/async/stream_controller.dart (right):

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
sdk/lib/async/stream_controller.dart:25: _StreamImpl<T> _stream;
Should we make this simply final and initialize it in the constructor?

https://codereview.chromium.org/12049013/diff/1/utils/pub/error_group.dart
File utils/pub/error_group.dart (right):

https://codereview.chromium.org/12049013/diff/1/utils/pub/error_group.dart#ne...
utils/pub/error_group.dart:236: : _controller = inner.isMulticast ?
isBroadcast?

https://codereview.chromium.org/12049013/diff/1/utils/pub/error_group.dart#ne...
utils/pub/error_group.dart:237: new StreamController.multiSubscription() :
Didn't see any change to this, but just to make sure: we kept the
multiSubscription constructor name?
Do we intend to change that one?

[Lasse Reichstein Nielsen, 2013-01-22 15:33:37]

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
File sdk/lib/async/stream_controller.dart (left):

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
sdk/lib/async/stream_controller.dart:24: class StreamController<T> extends
Stream<T> implements StreamSink<T> {
Description updated.

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
File sdk/lib/async/stream_controller.dart (right):

https://codereview.chromium.org/12049013/diff/1/sdk/lib/async/stream_controll...
sdk/lib/async/stream_controller.dart:25: _StreamImpl<T> _stream;
Yes. We can do that now that the state-change-callbacks are not instance
methods.

https://codereview.chromium.org/12049013/diff/1/utils/pub/error_group.dart
File utils/pub/error_group.dart (right):

https://codereview.chromium.org/12049013/diff/1/utils/pub/error_group.dart#ne...
utils/pub/error_group.dart:236: : _controller = inner.isMulticast ?
Fixed *blush*.

https://codereview.chromium.org/12049013/diff/1/utils/pub/error_group.dart#ne...
utils/pub/error_group.dart:237: new StreamController.multiSubscription() :
Ack no, it should be changed too.

[Lasse Reichstein Nielsen, 2013-01-22 15:35:30]

PTAL (and feel free to commit it before I get back, if you want to)

[floitsch, 2013-01-24 13:03:42]

Still LGTM.

https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream_contr...
File sdk/lib/async/stream_controller.dart (right):

https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream_contr...
sdk/lib/async/stream_controller.dart:12: * A controller with the stream it
controls.
containing?

https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream_contr...
sdk/lib/async/stream_controller.dart:27: * A controller with a broadcast
[stream]..
remove second ".".

[Bob Nystrom, 2013-01-24 17:29:14]

https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream.dart
File sdk/lib/async/stream.dart (right):

https://codereview.chromium.org/12049013/diff/5001/sdk/lib/async/stream.dart#...
sdk/lib/async/stream.dart:83: bool get isBroadcast => false;
Have you considered having an actual distinct type for broadcast streams?

I ask because I know Nathan got burned a couple of times trying to add a
listener to a single-cast stream that already had one. It would be nice if the
type system could help us statically avoid those bugs.