1122788 - Incorrect behavior of DOM File access to deleted files on debug builds

Status: RESOLVED FIXED

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

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

If a DOM File is constructed for a file that is later deleted, trying to read it will invoke the reader's onerror handler, except on debug builds.  On debug builds, the routine SeekableStreamAtBeginning in xpcom/io/nsMultiplexInputStream.cpp will call the Tell() method on the input stream, which performs its deferred open, but SeekableStreamAtBeginning discards the error from that method.

I ran into this in bug 1068838 while trying to adjust dom/base/test/test_fileapi.html to use the File constructor (in a chrome script) instead of writing to a file input element's |value| property and opening files directly; this may be related to how those two paths use different BlobImpls.

It's possible that this bug belongs in XPCOM instead of DOM; adjust as needed.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: fix.patch

I didn't find any reasonable way to fix this blocker for bug 1198095.
Another approach would be to change the nsTArray of nsIInputStream in some data struct with a boolean and then the first time we use this particular nsIInputStream, we check if the Tell() returns 0, but that will increase the level of complexity, almost for nothing.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8695902 [details] [diff] [review]
fix.patch

We could try skipping the tell() for file streams in particular, but we may have a buffered file stream which would have the same problem...

r=me, I guess.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d271b9e473fd

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for being a likely cause of https://treeherder.mozilla.org/logviewer.html#?job_id=18307492&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/1de4d57e6fa3

Comment 5

[Andrea Marchesini [:baku]]

Attachment: e10s

For e10s we must propagate the error code from Available. The reason why this is needed, is that, here:

http://mxr.mozilla.org/mozilla-central/source/dom/ipc/Blob.cpp#1626

we use Available() to force the stream open, but if this fails, in:

http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsMultiplexInputStream.cpp#725

we don't propagate such error value to the child process.
Probably would be a good idea to update mStatus for each public nsMultiplexInputStream method.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8696155 [details] [diff] [review]
e10s

r=me

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59c2efe077f6
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f21d895d333

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59c2efe077f6