Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 482310 - Add an API to asynchronously copy data to an output stream
: Add an API to asynchronously copy data to an output stream
: dev-doc-complete
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.2a1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Patrick McManus [:mcmanus]
Depends on:
Blocks: 485976 491936
  Show dependency treegraph
Reported: 2009-03-09 13:23 PDT by Shawn Wilsher :sdwilsh
Modified: 2009-09-03 13:25 PDT (History)
7 users (show)
sdwilsh: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proposed patch (22.78 KB, patch)
2009-04-17 12:00 PDT, Boris Zbarsky [:bz] (still a bit busy)
benjamin: review+
benjamin: superreview+
Details | Diff | Splinter Review

Description Shawn Wilsher :sdwilsh 2009-03-09 13:23:10 PDT
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) 2009-03-10 00:44:29 PDT
How would this differ from NS_AsyncCopy or the nsIAsyncStreamCopier?
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2009-03-10 07:05:57 PDT
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 [:bsmedberg] 2009-03-10 07:25:39 PDT
I think network utility functions should live under netwerk/. Toolkit is a container for too much random stuff already.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2009-03-10 07:29:53 PDT
OK, the question remains of how to expose the JS function to consumers usefully...
Comment 5 Benjamin Smedberg [:bsmedberg] 2009-03-10 07:40:41 PDT
I recommend Cu.import("resource://gre/network.js")
Comment 6 Christian :Biesinger (don't email me, ping me on IRC) 2009-03-10 08:16:20 PDT
hm, sure, though nsIAsyncStreamCopier isn't really hard to use
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2009-03-10 08:24:29 PDT
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 2009-03-10 09:11:52 PDT
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 [:bz] (still a bit busy) 2009-04-17 12:00:52 PDT
Created attachment 373358 [details] [diff] [review]
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 [:bsmedberg] 2009-05-06 08:58:37 PDT
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[";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[";1"].

But biesi or you should make that final decision.
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2009-05-06 20:33:05 PDT
> 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 [:bz] (still a bit busy) 2009-05-07 14:01:56 PDT
Pushed, 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 [:bz] (still a bit busy) 2009-05-07 14:09:29 PDT
Pushed with the comment and const change.
Comment 14 Shawn Wilsher :sdwilsh 2009-08-05 13:23:48 PDT
Tests added in bug 491936.
Comment 15 Eric Shepherd [:sheppy] 2009-09-03 13:00:35 PDT
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:
Comment 16 Eric Shepherd [:sheppy] 2009-09-03 13:25:28 PDT
OK, this is now documented here:

Note You need to log in before you can comment on or make changes to this bug.