I see 2 approaches here:
1. Use nsINativeFileWatcherService to follow changes in the underline file of the Blob. Problem is that, currently, nsINativeFileWatcherService works on windows only. We can add the support for other platforms based on Kqueue or Fsevent or INotify but it's a big task.
2. Do I/O all the time BlobImplFile::GetSize() is called. Easy, but it means we do I/O on the main-thread.
And for IPC, we do a sync IPC request if needed from content process to parent process.
I think it's better to go for the second approach.

The site compatibility doc claims that the issue arises when a previous FileReader instance is reused when reloading the File, but I see when a new FileReader instance is used. Maybe you're referring to the File instance (which is reused)?

(In reply to Andrea Marchesini (:baku) from comment #2)
> I see 2 approaches here:
1.
Can we check how Google Chrome has implemented this.
2.
Another observation, In Firefox when we do FORM submit (POST) the web server receive the latest version of the file.
3.
(sorry if i am naive)
At end of the low level OS file I/O read operation, we will be knowing actual size of the file.
So at time cant we fix
* files[0].size;
* event.loaded;
* event.total;
* event.target.result.byteLength;
If we can change event.target.result.byteLength after created, we should think adding extra property some thing like.
* event.target.result.contentByteLength;
To say actual size of file.
and
* event.target.result.remainingContent;
A buffer with the remaining content of the file.
Then propose it to the HTML5 spec.

> Can we check how Google Chrome has implemented this.
Done immediately: chrome does sync I/O.
> Another observation, In Firefox when we do FORM submit (POST) the web server
> receive the latest version of the file.
Sure
> Then propose it to the HTML5 spec.
This is interesting.
So, what I'm doing right now is this:
1. File.size does sync I/O if the File object points to a real file on the local filesystem.
This can happen only if the file has been chosen from a FilePicker.
This is going to be the first step and with this we will fix this issue.
2. Follow up: for windows we have a working nsIFileWatcherService.
I'm planning to use it (I have a 80% written patch for this). This means that for Windows we don't do sync IO all the time but we cache the size value after the first read. Then we will receive notifications from the FileWatcher service when something changes.
3. Follow up (super low priority - I guess): I want to implement a nsIFileWatcherServicer for linux too.
GTK has a GFileWatcher thing and we already have GTK as dependence for Linux and *BSD. This should work at the same way of point 2.
4. If we can cover mac too, somehow with FSEvent or KQueue, we can remove the sync I/O read completely, I hope.

A dummy performance test for .size handling is http://mozilla.pettay.fi/moztests/file.size.html
Blink is really bad with that, and I assume we'll regress that quite a bit with (1), which is acceptable for now and (2-4) will bring back the good performance.
It feels a bit bad if we have just (2) but not (3-4), and it is a tad error prone to have so different code paths for different OSes. But sure, (2) makes performance better for most of the users.

Khuey, with this patch, I propose a new 'RequestSyncIO()' method for the BlobImpl class.
The basic implementations of BlobImpl return false; BlobImplFile returns true and MultipleBlobImpl returns true if one of the sub blobImpl returns true.
This is useful for e10s and RemoteBlob because they do sync GetSize and GetLastModified when needed.

(In reply to tgvaughan from comment #5)
> The site compatibility doc claims that the issue arises when a previous
> FileReader instance is reused when reloading the File, but I see when a new
> FileReader instance is used. Maybe you're referring to the File instance
> (which is reused)?
Thanks. Fixed the confusing description.

This patch is built on top of nsIFileWatcherService. So far it works fine on windows and on any Gtk-platform builds (with bug 1263398).
I'm also planning to write a nsIFileWatcher that does polling and file bugs for MacOSX and Android.

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #14)
> This would be landing very late in 46 beta. I would rather not do that
> unless we absolutely have to.
> Should this block the release? If we don't fix it in 46 beta, would this
> drive a dot release?
Not at all. Actually Jonas things that the current behavior is correct. This is definitely not a regression.
The spec says that the File obj is immutable. This means that if the underlying file changes, the DOM File object should not be usable in any API.
I still think that the spec should be changed and this patch is important to have. But this is not related with FF 46 and so on.

OK. Thanks for the feedback! I think we should keep this change with 48 then.
Comment 1 gives the regression window as 45. Not sure whether to mark this as unaffected or simply not track it. Comment 4 also says, "45 ESR and 46+ require the fix" which is why I was asking.

Note that the important IO here is not the filesize. The important IO is the file contents.
It's really important that a call to FileReader.readAsText(HTML_FORM_INPUT.files[0]) doesn't produce data that contains partially the old data and partially the new data.
Likewise, the following should not result in partially old and partially new data being read, and it should not result in any gaps in the data:
var myFile = HTML_FORM_INPUT.files[0];
var chunkSize = 1024*1024;
loadChunk(0);
function loadChunk(n) {
var nChunks = Math.ceil(myFile.size / chunkSize);
if (n >= nChunks) {
done();
return;
}
displayProgressBar(n/nChunks);
var fr = new FileReader();
fr.readAsText(myFile.slice(chunkSize * n, chunkSize * (n+1)));
fr.onload = function() {
process(fr.result);
loadChunk(n+1);
}
}
Neither should
var myFile = HTML_FORM_INPUT.files[0];
var chunkSize = 1024*1024;
var nChunks = Math.ceil(myFile.size / chunkSize);
var slices = [];
for (i = 0; i < nChunks; i++) {
slices.push(myFile.slice(chunkSize * i, chunkSize * (i+1)));
}
loadChunk(0);
function loadChunk(n) {
if (n >= slices.length) {
done();
return;
}
displayProgressBar(n/slices.length);
var fr = new FileReader();
fr.readAsText(slices[n]);
fr.onload = function() {
process(fr.result);
loadChunk(n+1);
}
}
It's much preferable that these examples result in a read error, than that they result in corrupted data.
I don't see how that's achievable if we update File.size.
Ideally I think we should use nsIFileWatcherService to detect if the OS-file changed since the File object was created. And any time the file changes, fail any attempts to read from the File or any slices that has been created from it. That's more dependable than relying on the filesize changing.

> Ideally I think we should use nsIFileWatcherService to detect if the OS-file
> changed since the File object was created. And any time the file changes,
> fail any attempts to read from the File or any slices that has been created
> from it. That's more dependable than relying on the filesize changing.
Indeed. Before asking you a NI I actually filed bug 1264371.
But the point of this bug is to support the second FileReader and let it read the content of the file after the file is changed.
So I guess we should mark this bug as invalid and file a new one to throw more exceptions than what we currently do :)
Definitely we should finish the work I've started, implementing nsIFileWatcherService for all the supported platform (currently we have just windows). I wrote a patch to support it using GIO if we compile FF with GTK (bug 1263398) and I have an half-written patch that does polling for other platforms. For Mac we need FSEvents and KQueue.

Yeah, i think bug 1264371 covers the remaining changes that we want here.
IIRC File.size already grabs a snapshot of the filesize when the File object is created, and then never changes no matter what. Which is exactly the behavior that we want.
So marking this as INVALID.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #23)
> So .... can I cancel this review?
Yes. I have to change the test. I'll propose a different version of the same patch (to do exactly the opposite) in a separate bug.

(In reply to Andrea Marchesini (:baku) from comment #26)
> This is actually wrong. Onces the underlying file is changed, any API should
> not be able to use it.
It is common we attach a file then user realize s/he have not saved the final changes to the document.
Do we have a work around to solve such case?
Also I dont see in the file reader error handler there is no way to identify what caused the error.
Is there a way to identify the cause?

I think we could return a different File object from HTMLInputElement.files[0] if the file changed on disk.
But we currently don't, so there's no workaround that I can think of.
Likewise, I'd be fine with us firing a special error from FileReader if the file changed on disk. But I don't think we currently do.