Doug Turner (:dougt) Reporter
	Description • 13 years ago

Follow up from bug 717103 - this implements the file handle methods.

	Jan Varga [:janv]
	Comment 2 • 13 years ago

If I understood it correctly, .get() and .getEditable() return two different objects (File and FileHandle) backed by the same OS file. The whole point of FileHandle is to provide automatic locking. Now, when you have a (big sized) File object returned by .get(), you pass it to FileReader, then you immediatelly call .getEditable() and do some writing (FileReader is still reading) ...

Jonas, should DeviceStorage.get() delegate to FileHandle.getFile() ?
So File objects would not be created directly.

	Doug Turner (:dougt) Reporter
	Comment 3 • 13 years ago

>  The whole point of FileHandle is to provide automatic locking.

I thought it was so that you'd be able to write to the file.

	Doug Turner (:dougt) Reporter
	Comment 4 • 13 years ago

oh, i like what you're saying Jan.  Just expose .get() on device storage -- which is the common case.  If you want to write, you use the FileReader and getEditable?

	Jan Varga [:janv]
	Comment 5 • 13 years ago

FileHandle != FileWriter
FileHandle.getFile() creates only a read lock

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 6 • 13 years ago

The point of FileHandle is to provide the ability to write, as well the locking mechanisms needed to allow writing safely.

And yes, DeviceStorage.get should simply return the same Blob as you'd get from DeviceStorage.getEditable().getFile() (modulo the asynchronous syntax needed).

The main advantages with DeviceStorage.get is that it provides a simpler syntax for the common case of simply wanting to read a file, and it also allows for safer security models since it allows read-only access.

	Jan Varga [:janv]
	Comment 7 • 13 years ago

Sorry, I wanted to say "The main point" instead of "whole point"
And it provides the ability to write indeed.

	Jan Varga [:janv]
	Comment 8 • 13 years ago

Doug, I still think that you should pass an object that implements nsIFileStorage to DOMFileHandle::Create()
For now, only StorageId() has to be implemented.

Anyway, we might want to add additional methods to nsIFileStorage to lock the file at
OS level to interact with other processes.

SQLite has a cross platform implementation of shared and exclusive locks. That might
serve as a good example once we need it.

	Doug Turner (:dougt) Reporter
	Comment 9 • 13 years ago

jan, as on IRC, I still do not understand why you need StorageID.  I do not think you need nsIFileStorage to provide OS locking -- you can do that based on the nsIFile used to create the FileHandle.

	Jan Varga [:janv]
	Comment 10 • 13 years ago

Some things need to be (or might be needed) done only for DeviceStorage some only for IndexedDB and some for both, so I created the abstract nsIFileStorage interface.
Feel free to ignore it if you are convinced it's completely useless for DeviceStorage.

We don't need the OS file locking in IndexedDB, but FileHandle has to work with both "file storages". If you want to put OS file locking insided DOMFileHandle ok, up to you.

	Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]
	Comment 11 • 12 years ago

Lucas - What specifically do you want reviewed here, the implementation or the whole concept?

	Lucas Adamski [:ladamski]
	Comment 12 • 12 years ago

This API permits writing to a file, so it seems like it should get a secreview and possibly an implementation review as well.  A flaw here would be pretty bad.

	Jan Jongboom [:janjongboom] (inactive)
	Comment 13 • 11 years ago

Any news?

	Jan Varga [:janv]
	Comment 14 • 11 years ago

We're going to implement FileHandle in child processes first.

	Paul Theriault [:pauljt] (no longer reading bugmail)
	Comment 15 • 11 years ago

Removing secreview tag here for now. Please re-flag if/when this progresses.

	Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)
	Comment 16 • 7 years ago

FxOS/Gonk has been removed from the codebase. Mass-invalidating FxOS related Device Interface bugs to clean up the component. 

If I incorrectly invalidated something, please let me know.

	Firefox Bug Husbandry Bot
	Comment 17 • 7 years ago

Bulk correction of resolution of B2G bugs to INCOMPLETE.