As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 632227 - NetUtil should have a helper method to read an input stream asynchronously
: NetUtil should have a helper method to read an input stream asynchronously
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla5
Assigned To: Shawn Wilsher :sdwilsh
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks: 604699 632238
  Show dependency treegraph
 
Reported: 2011-02-07 15:39 PST by Shawn Wilsher :sdwilsh
Modified: 2011-04-12 10:42 PDT (History)
9 users (show)
sdwilsh: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1.0 (7.14 KB, patch)
2011-02-07 16:18 PST, Shawn Wilsher :sdwilsh
no flags Details | Diff | Splinter Review
v1.1 (6.63 KB, patch)
2011-02-07 16:21 PST, Shawn Wilsher :sdwilsh
bzbarsky: review+
cbiesinger: superreview+
Details | Diff | Splinter Review
v1.1 for checkin (3.99 KB, patch)
2011-03-28 12:56 PDT, Shawn Wilsher :sdwilsh
no flags Details | Diff | Splinter Review

Description User image Shawn Wilsher :sdwilsh 2011-02-07 15:39:04 PST
It turns out to be really hard to read an input stream asynchronously (at least from JS).  I'd like to make that easier with a new method on NetUtil.  I considered modifying asyncFetch (and could still), but I feel like asyncRead makes more sense.

Input welcome.
Comment 1 User image Shawn Wilsher :sdwilsh 2011-02-07 15:44:18 PST
On second thought, the code is nearly identical for this new case.  asyncFetch is probably the right place for this.
Comment 2 User image Shawn Wilsher :sdwilsh 2011-02-07 16:18:21 PST
Created attachment 510439 [details] [diff] [review]
v1.0

This was surprisingly painless.
Comment 3 User image Shawn Wilsher :sdwilsh 2011-02-07 16:21:16 PST
Created attachment 510440 [details] [diff] [review]
v1.1

Managed to miss two spots in the java-doc comment that talks about the source.  Fixed now.
Comment 4 User image Shawn Wilsher :sdwilsh 2011-02-07 16:22:56 PST
I guess biesi might be a good person for sr?
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-28 10:16:02 PDT
Comment on attachment 510440 [details] [diff] [review]
v1.1

r=me
Comment 6 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2011-03-28 10:29:10 PDT
Is it guaranteed that pump stays alive?
Comment 7 User image Shawn Wilsher :sdwilsh 2011-03-28 10:34:41 PDT
(In reply to comment #6)
> Is it guaranteed that pump stays alive?
Hmm, AFAICT, nothing.  I think we have the same problem with the existing implementation and |channel|.  bz - am I missing something here?
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-28 11:13:00 PDT
The pump is kept alive through the read process, yes.  Basically, either the pump is waiting on the stream (and then the stream is keeping it alive) or it's  inside OnInputStreamReady and there's a ref to it on the stack.

Similar with channels; between AsyncOpen returning and OnStopRequest being called the channel is kept alive by the necko machinery.
Comment 9 User image Shawn Wilsher :sdwilsh 2011-03-28 12:56:30 PDT
Created attachment 522434 [details] [diff] [review]
v1.1 for checkin
Comment 10 User image Shawn Wilsher :sdwilsh 2011-03-28 12:57:14 PDT
(In reply to comment #8)
> The pump is kept alive through the read process, yes.  Basically, either the
> pump is waiting on the stream (and then the stream is keeping it alive) or it's
>  inside OnInputStreamReady and there's a ref to it on the stack.
> 
> Similar with channels; between AsyncOpen returning and OnStopRequest being
> called the channel is kept alive by the necko machinery.
Thanks for the clarification!
Comment 11 User image Shawn Wilsher :sdwilsh 2011-03-31 11:22:16 PDT
http://hg.mozilla.org/mozilla-central/rev/76fbb32b78af
Comment 12 User image Eric Shepherd [:sheppy] 2011-04-12 10:42:39 PDT
Updated documentation:

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

Also mentioned on Firefox 5 for developers.

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