Closed Bug 1402888 Opened 7 years ago Closed 7 years ago

nsIStreamTransportService::createInputTransport startOffset and readLimit is not used

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(3 files)

We can simplify nsIStreamTransportService removing startOffset and readLimit params. If needed, SlicedInputStream can be used.
Attachment #8911836 - Flags: review?(bugs)
Attachment #8911836 - Attachment description: slicedInputStream.patch → part 1 - CreateInputTransport
Attachment #8911839 - Flags: review?(bugs)
If you prefer I can file a separate bug. CreateOutputTransport is not used.
Attachment #8911853 - Flags: review?(bugs)
Attachment #8911853 - Flags: review?(bugs) → review+
Comment on attachment 8911836 [details] [diff] [review] part 1 - CreateInputTransport > // > // OK, we need to use the stream transport service if > // > // (1) the stream is blocking > // (2) the stream does not support nsIAsyncInputStream > // >+ if (mStreamOffset != UINT64_MAX || mStreamLength != UINT64_MAX) { >+ mStream = new SlicedInputStream(mStream, mStreamOffset, mStreamLength); >+ mStreamOffset = 0; Why mStreamOffset = 0; here? It is set to 0 later in the method. I don't understand if (mStreamOffset != UINT64_MAX || mStreamLength != UINT64_MAX) { Why limit SlicedInputStream creation that way? (I need to read the other patch too)
Comment on attachment 8911836 [details] [diff] [review] part 1 - CreateInputTransport but ok, that stuff goes away
Attachment #8911836 - Flags: review?(bugs) → review+
Comment on attachment 8911836 [details] [diff] [review] part 1 - CreateInputTransport No, I still don't understand. Why we add the sliced stream here and then remove it.
Attachment #8911836 - Flags: review+
Comment on attachment 8911839 [details] [diff] [review] part 2 - nsIInputStreamPump Yeah, you need to explain these patches still again, sorry.
Attachment #8911839 - Flags: review?(bugs)
Attachment #8911839 - Flags: review?(bugs)
Attachment #8911836 - Flags: review?(bugs)
Attachment #8911836 - Flags: review?(bugs) → review+
Attachment #8911839 - Flags: review?(bugs) → review+
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d434da3de9ea Remove nsIStreamTransportService::createInputStream startOffset and readLimit params, r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/560296786c0c Remove nsIInputStreamPump::Init() offset and limit params, r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/7e992ffa004c Remove nsIStreamTransportService::createOutputStream, r=smaug
You busted TB with that. We DO use those parameters. https://dxr.mozilla.org/comm-central/search?q=CreateInputTransport&redirect=false I don't see how to work not having them.
Flags: needinfo?(bugs)
Flags: needinfo?(amarchesini)
Or do I need to switch to SlicedInputStream?
Hmm, SlicedInputStream() doesn't have the aCloseWhenDone parameter, which we also use.
I forked nsIStreamTransportService into C-C, see bug 1403393.
(In reply to Jorg K (GMT+2) from comment #12) > Hmm, SlicedInputStream() doesn't have the aCloseWhenDone parameter, which we > also use. Probably it's better to add aCloseWhenDone parameter in SlicedInputStream instead.
Flags: needinfo?(amarchesini)
Up to you. Right now, C-C is busted and I can't allow it to remain busted until you implement that. I can back out my fork later.
Turned out that we can use this: - rv = serv->CreateInputTransport(m_multipleMsgMoveCopyStream, int64_t(offset), - int64_t(size), false, + RefPtr<SlicedInputStream> slicedStream = + new SlicedInputStream(slicedStream, offset, uint64_t(size)); + rv = serv->CreateInputTransport(m_multipleMsgMoveCopyStream, false, getter_AddRefs(m_transport)); Sorry about the noise.
Flags: needinfo?(bugs)
We have too many ways to deal with inputStream and our documentation is not good as it should.
Depends on: 1403771
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: