Update ZLib/GZip in dart:io to be a Codec/Converter from dart:convert.

Update ZLib/GZip in dart:io to be a Codec/Converter from dart:convert.

BUG=
R=floitsch@google.com, sgjesse@google.com

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

[floitsch, 2013-08-22 15:04:04]

LGTM.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.dart
File sdk/lib/io/data_transformer.dart (right):

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:8: const ZLIB = const ZLibCodec();
add comments.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:11: class ZLibCodec extends Codec<List<int>,
List<int>> {
add comments and for all other classes / methods.

Don't forget to explain the difference between ZLIB and GZIP.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:31: class ZLibEncoder extends
Converter<List<int>, List<int>> {
Still misses the synchronous convert function.
You can just use the startChunkedConversion to do it.

Add some synchronous tests.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:41: ByteConversionSink startChunkedConversion(
Comment explaining that the function is more efficient with a
ByteConversionSink.

ditto for other.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:113: if (_empty) _filter.process(const [], 0,
0);
can't you replace this and the following lines with: addSlice([], 0, 0, false)

Add comment that you need to call _filter.process at least once (at least that's
what I understand).

[Søren Gjesse, 2013-08-22 15:10:37]

lgtm

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.dart
File sdk/lib/io/data_transformer.dart (right):

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:8: const ZLIB = const ZLibCodec();
Documentation for the ZLIB object?

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:11: class ZLibCodec extends Codec<List<int>,
List<int>> {
And for the codec.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:20: const GZIP = const GZipCodec();
Ditto.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:23: class GZipCodec extends Codec<List<int>,
List<int>> {
Ditto.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:51: class ZLibDecoder extends
Converter<List<int>, List<int>> {
Add comment for this constructor as well. Currently dartdoc will put in the doc
from the super constructor is there is no doc, which is pretty confusing.

[Anders Johnsen, 2013-08-22 15:58:01]

PTAL

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.dart
File sdk/lib/io/data_transformer.dart (right):

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:8: const ZLIB = const ZLibCodec();
On 2013/08/22 15:04:05, floitsch wrote:
> add comments.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:8: const ZLIB = const ZLibCodec();
On 2013/08/22 15:10:37, Søren Gjesse wrote:
> Documentation for the ZLIB object?

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:11: class ZLibCodec extends Codec<List<int>,
List<int>> {
On 2013/08/22 15:04:05, floitsch wrote:
> add comments and for all other classes / methods.
> 
> Don't forget to explain the difference between ZLIB and GZIP.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:11: class ZLibCodec extends Codec<List<int>,
List<int>> {
On 2013/08/22 15:10:37, Søren Gjesse wrote:
> And for the codec.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:20: const GZIP = const GZipCodec();
On 2013/08/22 15:10:37, Søren Gjesse wrote:
> Ditto.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:23: class GZipCodec extends Codec<List<int>,
List<int>> {
On 2013/08/22 15:10:37, Søren Gjesse wrote:
> Ditto.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:31: class ZLibEncoder extends
Converter<List<int>, List<int>> {
On 2013/08/22 15:04:05, floitsch wrote:
> Still misses the synchronous convert function.
> You can just use the startChunkedConversion to do it.
> 
> Add some synchronous tests.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:41: ByteConversionSink startChunkedConversion(
On 2013/08/22 15:04:05, floitsch wrote:
> Comment explaining that the function is more efficient with a
> ByteConversionSink.
> 
> ditto for other.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:51: class ZLibDecoder extends
Converter<List<int>, List<int>> {
On 2013/08/22 15:10:37, Søren Gjesse wrote:
> Add comment for this constructor as well. Currently dartdoc will put in the
doc
> from the super constructor is there is no doc, which is pretty confusing.

Done.

https://codereview.chromium.org/22867033/diff/1/sdk/lib/io/data_transformer.d...
sdk/lib/io/data_transformer.dart:113: if (_empty) _filter.process(const [], 0,
0);
On 2013/08/22 15:04:05, floitsch wrote:
> can't you replace this and the following lines with: addSlice([], 0, 0, false)
> 
> Add comment that you need to call _filter.process at least once (at least
that's
> what I understand).

Added comment. And no, since we don't want to write [] unless we really mean it
(otherwise we might get an extra frame).

[Søren Gjesse, 2013-08-22 16:07:25]

lgtm

https://codereview.chromium.org/22867033/diff/2001/sdk/lib/io/data_transforme...
File sdk/lib/io/data_transformer.dart (right):

https://codereview.chromium.org/22867033/diff/2001/sdk/lib/io/data_transforme...
sdk/lib/io/data_transformer.dart:89: class ZLibEncoder extends
Converter<List<int>, List<int>> {
We could consider making this and ZLibDecoder private to avoid exposing more
classes than needed. Ifaics this class does not provide features which are not
available through the codec classes.

[Anders Johnsen, 2013-08-23 07:11:53]

https://codereview.chromium.org/22867033/diff/2001/sdk/lib/io/data_transforme...
File sdk/lib/io/data_transformer.dart (right):

https://codereview.chromium.org/22867033/diff/2001/sdk/lib/io/data_transforme...
sdk/lib/io/data_transformer.dart:89: class ZLibEncoder extends
Converter<List<int>, List<int>> {
On 2013/08/22 16:07:26, Søren Gjesse wrote:
> We could consider making this and ZLibDecoder private to avoid exposing more
> classes than needed. Ifaics this class does not provide features which are not
> available through the codec classes.

Exposing both codecs and converters is in sync with how we do in dart:convert.
Allocating a codec just to get an encoder is a bit cumbersome.

[Anders Johnsen, 2013-08-23 07:12:24]

Committed patchset #2 manually as r26577 (presubmit successful).