NetUtil should have a helper method to read an input stream asynchronously

RESOLVED FIXED in mozilla5

Status

()

Core
Networking
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Tracking

({dev-doc-complete})

Trunk
mozilla5
x86
Windows 7
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
On second thought, the code is nearly identical for this new case.  asyncFetch is probably the right place for this.
(Assignee)

Comment 2

7 years ago
Created attachment 510439 [details] [diff] [review]
v1.0

This was surprisingly painless.
Attachment #510439 - Flags: review?(bzbarsky)
(Assignee)

Comment 3

7 years ago
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.
Attachment #510439 - Attachment is obsolete: true
Attachment #510440 - Flags: review?(bzbarsky)
Attachment #510439 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

7 years ago
I guess biesi might be a good person for sr?
(Assignee)

Updated

7 years ago
Whiteboard: [needs review bz]

Updated

7 years ago
Blocks: 632238

Comment 5

6 years ago
Comment on attachment 510440 [details] [diff] [review]
v1.1

r=me
Attachment #510440 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

6 years ago
Attachment #510440 - Flags: superreview?(cbiesinger)
(Assignee)

Updated

6 years ago
Whiteboard: [needs review bz] → [needs sr biesi]
Is it guaranteed that pump stays alive?
(Assignee)

Comment 7

6 years ago
(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

6 years ago
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.
Attachment #510440 - Flags: superreview?(cbiesinger) → superreview+
(Assignee)

Comment 9

6 years ago
Created attachment 522434 [details] [diff] [review]
v1.1 for checkin
Attachment #510440 - Attachment is obsolete: true
(Assignee)

Comment 10

6 years ago
(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!
Flags: in-testsuite?
Keywords: checkin-needed
Whiteboard: [needs sr biesi] → [can land]
(Assignee)

Comment 11

6 years ago
http://hg.mozilla.org/mozilla-central/rev/76fbb32b78af
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed → dev-doc-needed
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → mozilla2.2
Updated documentation:

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

Also mentioned on Firefox 5 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.