1349570 - nsPipe3 should not be nsISeekableStream

Status: RESOLVED INVALID

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

Description

[Andrea Marchesini [:baku]]

Currently we have just ::Tell() implemented. ::Seek() and ::SetEOF just return an error code.
It seems that nsPipe3 is not used as seekable in our codebase, and the removal is green on try.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: pipe_noSeekable.patch

Comment 2

[Andrea Marchesini [:baku]]

Attachment: pipe_noSeekable.patch

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8850064 [details] [diff] [review]
pipe_noSeekable.patch

How did you check Tell isn't being used?
I couldn't figure out any easy way to check that pipe's inputstream isn't accessed as nsISeekableStream.

Did you check also that Thunderbird doesn't use the inputstream as seekable?

Please explain, and ask review again.

Comment 4

[Andrea Marchesini [:baku]]

> How did you check Tell isn't being used?

I relay in tests: I pushed the current patch to try and it's green.

> Did you check also that Thunderbird doesn't use the inputstream as seekable?

Also this is not easy to do. I prefer to push this patch, and in case we break something we can introduce a "nsITellableStream".

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Andrea Marchesini [:baku] from comment #4)
> > How did you check Tell isn't being used?
> 
> I relay in tests: I pushed the current patch to try and it's green.
ok, I don't think that is good enough.

Is there some particular reason why this change must be done? And if so, better to audit at least
Gecko code about the possible usage, and just running tests isn't really auditing.

Comment 6

[Andrea Marchesini [:baku]]

I think I have found a better solution.