1006485 - FileHandle: Rename FileHandle to MutableFile and LockedFile to FileHandle

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Jan Varga [:janv]]

FileSystem API is based on FileHandle API exposed via IndexedDB, but LockedFile object has been renamed to FileHandle, there's no object that represents a mutable file and DOM events have been replaced with promises.
FileSystem API implementation will likely reuse our current FileHandle API implementation exposed via IndexedDB. We don't know what will happen with FileHandle API in IndexedDB, but for now we should fix the object names at least.
We also need to prefix existing interfaces with "IDB".

FileHandle -> IDBMutableFile
LockedFile -> IDBFileHandle

Everything should be backwards compatible, we don't want to break existing consumers of the API. I guess no consumer needs to check object names.

This is also needed for FileHandle on PBackground, since I want to use consistent terminology in IPDL protocols, DOM objects, etc.

Comment 1

[Jan Varga [:janv]]

Also IDBDatatabase.mozCreateFileHandle() -> IDBDatabase.createMutableFile().

Comment 2

[Jan Varga [:janv]]

Attachment: patch

Ok, here's the patch, the only incompatible change is renaming of the error LockedFileInactiveError to FileHandleInactiveError, but I think existing consumers shouldn't rely on this.

Comment 3

[Jan Varga [:janv]]

Attachment: a diff for easier review

diff -u orig/LockedFile.webidl new/FileHandle.webidl
diff -u orig/LockedFile.h new/FileHandle.h
diff -u orig/LockedFile.cpp new/FileHandle.cpp

Comment 4

[Jan Varga [:janv]]

Comment on attachment 8422005 [details] [diff] [review]
patch

Hm, it seems I forgot to prefix the interfaces ...

Comment 5

[Jan Varga [:janv]]

Attachment: Patch a - Rename FileHandle to MutableFile and LockedFile to FileHandle

Comment 6

[Jan Varga [:janv]]

Attachment: Patch b - hg rename some files separately to make the main patch easier to review

Comment 7

[Jan Varga [:janv]]

Comment on attachment 8437942 [details] [diff] [review]
Patch a - Rename FileHandle to MutableFile and LockedFile to FileHandle

Ok, this patch just renames FileHandle to MutableFile and LockedFile to FileHandle, there are also some s/NS_ASSERTION/MOZ_ASSERT

Non-mechanical changes will come in a separate patch.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8437942 [details] [diff] [review]
Patch a - Rename FileHandle to MutableFile and LockedFile to FileHandle

Review of attachment 8437942 [details] [diff] [review]:
-----------------------------------------------------------------

r=me assuming this builds and passes tests :)

::: dom/indexedDB/IDBDatabase.h
@@ +233,2 @@
>    MozCreateFileHandle(const nsAString& aName, const Optional<nsAString>& aType,
> +                      ErrorResult& aRv)

Can we get a followup to issue a deprecated warning here (and in the other deprecated methods)?

Comment 9

[Jan Varga [:janv]]

https://tbpl.mozilla.org/?tree=Try&rev=84d98ab1b2a2

Comment 10

[Jan Varga [:janv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/00d924bed2d1
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ee90f5b0aac

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/00d924bed2d1
https://hg.mozilla.org/mozilla-central/rev/7ee90f5b0aac

Comment 12

[Jan Varga [:janv]]

Sorry, forgot to set leave-open

Comment 13

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Hm, what's left to do here? I like one(-ish) patch per bug usually. Can we just have another for any additional work?

Comment 14

[Jan Varga [:janv]]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #13)
> Hm, what's left to do here? I like one(-ish) patch per bug usually. Can we
> just have another for any additional work?

Here's the last patch I was planning to attach here:
https://tbpl.mozilla.org/?tree=Try&rev=16e9a3e79abe

The "IDB" prefix is what needs to be done.

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yeah, let's just move to a new bug for that.

Comment 16

[Jan Varga [:janv]]

Ok

Comment 17

[Jan Varga [:janv]]

Filed bug 1026735.

Comment 18

[Jan Varga [:janv]]

Filed bug 1029209 for the last part - the "IDB" prefix.

Comment 19

[Jean-Yves Perrier [:teoli]]

Jan, in order to update the documentation, am I correct about what this bug (together with bug 1029209) does (for Fx 33):
- FileHandle becomes IDBMutableFile
- LockedFile becomes IDBFileHandle
- The properties, methods and inheritance of these two interfaces are unchanged
- Old versions of the API (i.e. FileHandle & LockedHandle) are no more available

Also I have a question about IDBDatabase.mozCreateFileHandle. It looks like bug 1024312 (at this time not fixed) will deprecate it (but not yet remove it) and add the standard IDBDatabase.createMutableFile; but it looks like this bug has patch already adding the new method in the webidl ( http://hg.mozilla.org/mozilla-central/diff/00d924bed2d1/dom/webidl/IDBDatabase.webidl ). 

I am a bit lost here. Has IDBDatabase.createMutableFile already been added or not (Gecko 33) ?

Comment 20

[Jan Varga [:janv]]

(In reply to Jean-Yves Perrier [:teoli] from comment #19)
> Jan, in order to update the documentation, am I correct about what this bug
> (together with bug 1029209) does (for Fx 33):
> - FileHandle becomes IDBMutableFile

right

> - LockedFile becomes IDBFileHandle

right

and FileRequest becomes IDBFileRequest

> - The properties, methods and inheritance of these two interfaces are
> unchanged

right

there are also 2 new attributes added for backwards compatibility:
IDBFileHandle.filehandle // now mutableFile
http://mxr.mozilla.org/mozilla-central/source/dom/webidl/IDBFileHandle.webidl

IDBFileRequest.lockedFile // now fileHandle
http://mxr.mozilla.org/mozilla-central/source/dom/webidl/IDBFileRequest.webidl

> - Old versions of the API (i.e. FileHandle & LockedHandle) are no more
> available

well, just the WebIDL interfaces are not available anymore, so for example following JS doesn't work:
if (window.FileHandle) {
  ...
}

> 
> Also I have a question about IDBDatabase.mozCreateFileHandle. It looks like
> bug 1024312 (at this time not fixed) will deprecate it (but not yet remove
> it) and add the standard IDBDatabase.createMutableFile; but it looks like
> this bug has patch already adding the new method in the webidl (
> http://hg.mozilla.org/mozilla-central/diff/00d924bed2d1/dom/webidl/
> IDBDatabase.webidl ). 
> 

No, mozCreateFileHandle() has been already deprecated internally, there are now two methods doing the same:
mozCreateFileHandle() and createMutableFile()

The bug is about adding a warning for web developers, etc.

> I am a bit lost here. Has IDBDatabase.createMutableFile already been added
> or not (Gecko 33) ?

yes, it's been already added