Add a flush operations to IOSink

Add optional flush argument to some file methods

BUG=http://dartbug.com/8794

R=whesse@google.com, ajohnsen@google.com

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

[Søren Gjesse, 2013-08-01 09:17:57]

Please scrutinize this to see if I got the semantics right for the sync/async
operations on IOSink.

[Anders Johnsen, 2013-08-01 13:54:27]

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

https://codereview.chromium.org/20745006/diff/1/sdk/lib/io/io_sink.dart#newco...
sdk/lib/io/io_sink.dart:254: Future flush() {
So, several comments and concerns about this approach:

1) It's not very portable. It only works for files, and will be
near-to-impossible to implement for other data-types. IOSink is exposed as a
class for other users, where they expect this stuff should work for their
supplied StreamConsumer.

2) Calling flush directly like so will ignore anything currently queued. That
is, it's only flushing the current operation, not what's currently buffered by
the IOSinks StreamController (I may be mistaken, but that's how I read the
code).

I would recommend another direction that may not flush stuff directly for files
like this does, however, it'll be portable. Let 'flush()' return
'Future.value()' in the case the stream is not paused. If paused, return a
future that will complete once the stream is resumed again. That should ensure
that it's possible to implement a way for flushing any data currently in
buffers, all the way down to the target(may it be OS). The question is then what
to do in the case of an add immediate after flush() - that can definitely be
debated.

[Søren Gjesse, 2013-08-01 14:29:18]

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

https://codereview.chromium.org/20745006/diff/1/sdk/lib/io/io_sink.dart#newco...
sdk/lib/io/io_sink.dart:254: Future flush() {
On 2013/08/01 13:54:28, Anders Johnsen wrote:
> So, several comments and concerns about this approach:
> 
> 1) It's not very portable. It only works for files, and will be
> near-to-impossible to implement for other data-types. IOSink is exposed as a
> class for other users, where they expect this stuff should work for their
> supplied StreamConsumer.
> 
> 2) Calling flush directly like so will ignore anything currently queued. That
> is, it's only flushing the current operation, not what's currently buffered by
> the IOSinks StreamController (I may be mistaken, but that's how I read the
> code).
> 
> I would recommend another direction that may not flush stuff directly for
files
> like this does, however, it'll be portable. Let 'flush()' return
> 'Future.value()' in the case the stream is not paused. If paused, return a
> future that will complete once the stream is resumed again. That should ensure
> that it's possible to implement a way for flushing any data currently in
> buffers, all the way down to the target(may it be OS). The question is then
what
> to do in the case of an add immediate after flush() - that can definitely be
> debated.

As the comment above states this only does anything for files and also only
works reliably when using add or when the future from addStream has completed.
As far as I can see our implementation will send data written directly to the OS
when add is used (sync StreamController in IOSink implementation), and therefore
sending flush will work.

I don't think this makes much sense for anything but files, and also I don't see
it have much meaning while addStream is used.

I am fine with dropping this and stating that if flushing file content to disk
is required then one should use a RandomAccessFile.

[Søren Gjesse, 2013-10-28 14:29:41]

Changed the approach...

PTAL

[Anders Johnsen, 2013-10-28 15:13:22]

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

https://codereview.chromium.org/20745006/diff/7001/sdk/lib/io/io_sink.dart#ne...
sdk/lib/io/io_sink.dart:58: Future flush();
I still disagree that this should be part of the IOSink. It's far to few
interfaces which supports this, and the user can create IOSinks for any
StreamConsumer.

[Søren Gjesse, 2013-11-05 08:47:00]

Committed patchset #4 manually as r29880 (presubmit successful).