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
:
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 | Review
v1.1 (6.63 KB, patch)
2011-02-07 16:21 PST, Shawn Wilsher :sdwilsh
bzbarsky: review+
cbiesinger: superreview+
Details | Diff | Review
v1.1 for checkin (3.99 KB, patch)
2011-03-28 12:56 PDT, Shawn Wilsher :sdwilsh
no flags Details | Diff | Review

Description 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 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 Shawn Wilsher :sdwilsh 2011-02-07 16:18:21 PST
Created attachment 510439 [details] [diff] [review]
v1.0

This was surprisingly painless.
Comment 3 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 Shawn Wilsher :sdwilsh 2011-02-07 16:22:56 PST
I guess biesi might be a good person for sr?
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-03-28 10:16:02 PDT
Comment on attachment 510440 [details] [diff] [review]
v1.1

r=me
Comment 6 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-03-28 10:29:10 PDT
Is it guaranteed that pump stays alive?
Comment 7 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 Boris Zbarsky [:bz] (Out June 25-July 6) 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 Shawn Wilsher :sdwilsh 2011-03-28 12:56:30 PDT
Created attachment 522434 [details] [diff] [review]
v1.1 for checkin
Comment 10 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 Shawn Wilsher :sdwilsh 2011-03-31 11:22:16 PDT
http://hg.mozilla.org/mozilla-central/rev/76fbb32b78af
Comment 12 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.