482310 - Add an API to asynchronously copy data to an output stream

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Shawn Wilsher :sdwilsh]

Originally from a dev.platform thread.  I'd like to see an API that allows us to asynchronously write data to a file, and bz suggested this:
OK.  So what you want here, basically, is a function that takes your data in some form and takes an output stream (notably NOT an nsIFile or filename) and does the async copy, right?  This should be pretty simple to write, and I'm happy to do so if someone points me to the right place to dump it.... ;) 

In terms of error handling, I'd by happy with the ability to register a function that receives error information, and handles it accordingly (much like how mozIStorageStatementCallback::handleError works).

This probably isn't the right component, but I'm not sure where the right one is.  This felt like the best place to put it.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

How would this differ from NS_AsyncCopy or the nsIAsyncStreamCopier?

Comment 2

[Boris Zbarsky [:bzbarsky]]

The idea here is to have a simple-to-use JS-only API.  It'd basically be a thin shim around nsIAsyncStreamCopier.

The right component for this is whatever toolkit component is correct for "toolkit utility functions", no?

Comment 3

[Benjamin Smedberg]

I think network utility functions should live under netwerk/. Toolkit is a container for too much random stuff already.

Comment 4

[Boris Zbarsky [:bzbarsky]]

OK, the question remains of how to expose the JS function to consumers usefully...

Comment 5

[Benjamin Smedberg]

I recommend Cu.import("resource://gre/network.js")

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

hm, sure, though nsIAsyncStreamCopier isn't really hard to use

Comment 7

[Boris Zbarsky [:bzbarsky]]

It takes 10-20 lines of boilerplate for what can (and should, and is in C++) be a single function call.

Comment 8

[Shawn Wilsher :sdwilsh]

mimicking nsNetUtil.h for JS would probably be incredibly useful.  Of course, it can't have the same signature, but similar is good too.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

This has tests for the C++ class I added, but not yet for the .jsm.  With any luck, Shawn will write some of the latter.  ;)

I didn't expose the full API that nsNetUtil or the interface exposes (complete with being able to provide an event target, being able to control closing behavior, etc).  We could add some optional arguments for all that, with an exception thrown if we have to create a buffered wrapper for a stream the caller wants kept open, because otherwise there might be data stuck in the buffers that the caller doesn't know about...

Comment 10

[Benjamin Smedberg]

Comment on attachment 373358 [details] [diff] [review]
Proposed patch

>diff --git a/netwerk/base/src/NetUtil.jsm b/netwerk/base/src/NetUtil.jsm

>+/***
>+ * Necko utilities

nit, extra * in header

>+var NetUtil = {

any reason not to make this 'const' as well?

>+        if (!sourceBuffered && !sinkBuffered) {
>+            // wrap the sink in a buffered stream.
>+            var bostream = Cc["@mozilla.org/network/buffered-output-stream;1"]
>+                             .createInstance(Ci.nsIBufferedOutputStream);

My personal style preference for new code is to put operators at the end of the line and not to line up dots:

            var bostream = Cc["@mozilla.org/network/buffered-output-stream;1"].
                createInstance(Ci.nsIBufferedOutputStream);

But biesi or you should make that final decision.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> any reason not to make this 'const' as well?

Hmm.  Probably not.  I just copy/pasted from XPCOMUtils.jsm here.  ;)

As far as operators, I was following what I understood to be our general JS style.  I personally prefer the style you describe as well, and would be happy to switch to it!

Comment 12

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/55339f8d68fa, but forgot to address the '*' and const nits.  I'll do that once the tree reopens.

Filed bug 491936 on writing the tests.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/3b1fb9fce222 with the comment and const change.

Comment 14

[Shawn Wilsher :sdwilsh]

Tests added in bug 491936.

Comment 15

[Eric Shepherd [:sheppy]]

I haven't written this up yet, but I've reorganized the code modules content to create a good place to put it. It will go here when I write it, which should be very shortly:

https://developer.mozilla.org/en/JavaScript_code_modules/NetUtil.jsm

Comment 16

[Eric Shepherd [:sheppy]]

OK, this is now documented here:

https://developer.mozilla.org/en/JavaScript_code_modules/NetUtil.jsm