Remove .writeStream, .consume and rewrite IOSink to correctly implement a (sane) well-defined behav…

Remove .writeStream, .consume and rewrite IOSink to correctly implement a (sane) well-defined behaviour of the .done future.

BUG=

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

[Søren Gjesse, 2013-04-15 06:56:30]

LGTM!

https://codereview.chromium.org/14028017/diff/2001/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/scheduled_server/safe_http_server.dart (right):

https://codereview.chromium.org/14028017/diff/2001/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/scheduled_server/safe_http_server.dart:1: //
iopyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
Accidental edit

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:516: .then((v) {
v -> _

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:593: var data = _buffer.readBytes();
Why this change?

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:631: // propegated to the websocket.
propagated

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart
File sdk/lib/io/io_sink.dart (right):

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:10: * [IOSink] buffers the input given by [write],
[writeAll], [writeln],
[write], [writeAll], [writeln], [writeCharCode] => all [StringSink] methods

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:15: * to the [IOSink] will throw a [StateError].
Maybe mention that when the addStream future completes the IOSink is no longer
bound to a stream and it is again possible to call methods.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:135: _isBound = true;
This looks good. Maybe add a comment like "wait for any buffers sync operations
to complete".

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:153: throw new StateError("IOSink is already bound to a
stream");
Maybe the "already" in this message is confusing.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:187: throw new StateError("IOSink is already bound to a
stream");
Remove "already" here as well.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:190: throw new StateError("IOSink is already closed");
And here.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:192: if (_controllerInstance == null) {
This also looks nice!

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:199: // A new stream takes over - forward errors to that
stream.
I think this comment belongs in the onError case.

[Anders Johnsen, 2013-04-15 07:35:19]

https://codereview.chromium.org/14028017/diff/2001/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/scheduled_server/safe_http_server.dart (right):

https://codereview.chromium.org/14028017/diff/2001/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/scheduled_server/safe_http_server.dart:1: //
iopyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> Accidental edit

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:516: .then((v) {
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> v -> _

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:593: var data = _buffer.readBytes();
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> Why this change?

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/http_impl.dart#...
sdk/lib/io/http_impl.dart:631: // propegated to the websocket.
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> propagated

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart
File sdk/lib/io/io_sink.dart (right):

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:10: * [IOSink] buffers the input given by [write],
[writeAll], [writeln],
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> [write], [writeAll], [writeln], [writeCharCode] => all [StringSink] methods

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:15: * to the [IOSink] will throw a [StateError].
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> Maybe mention that when the addStream future completes the IOSink is no longer
> bound to a stream and it is again possible to call methods.

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:135: _isBound = true;
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> This looks good. Maybe add a comment like "wait for any buffers sync
operations
> to complete".

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:153: throw new StateError("IOSink is already bound to a
stream");
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> Maybe the "already" in this message is confusing.

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:187: throw new StateError("IOSink is already bound to a
stream");
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> Remove "already" here as well.

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:190: throw new StateError("IOSink is already closed");
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> And here.

Done.

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:192: if (_controllerInstance == null) {
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> This also looks nice!

Thanks :)

https://codereview.chromium.org/14028017/diff/2001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:199: // A new stream takes over - forward errors to that
stream.
On 2013/04/15 06:56:30, Søren Gjesse wrote:
> I think this comment belongs in the onError case.

Done.

[Anders Johnsen, 2013-04-15 07:36:07]

Committed patchset #3 manually as r21416 (presubmit successful).