FileHandle: Rename FileHandle to MutableFile and LockedFile to FileHandle

RESOLVED FIXED in mozilla33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: janv, Assigned: janv)

Tracking

(Depends on 1 bug, Blocks 1 bug, {dev-doc-needed})

Trunk
mozilla33
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 942542
(Assignee)

Comment 1

5 years ago
Also IDBDatatabase.mozCreateFileHandle() -> IDBDatabase.createMutableFile().
Keywords: dev-doc-needed
(Assignee)

Comment 2

5 years ago
Posted patch patch (obsolete) — Splinter Review
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.
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Attachment #8422005 - Flags: review?(bent.mozilla)
(Assignee)

Comment 3

5 years ago
Posted patch a diff for easier review (obsolete) — Splinter 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
(Assignee)

Comment 4

5 years ago
Comment on attachment 8422005 [details] [diff] [review]
patch

Hm, it seems I forgot to prefix the interfaces ...
Attachment #8422005 - Flags: review?(bent.mozilla)
(Assignee)

Comment 5

5 years ago
Attachment #8422005 - Attachment is obsolete: true
Attachment #8422012 - Attachment is obsolete: true
(Assignee)

Comment 7

5 years ago
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.
Attachment #8437942 - Flags: review?(bent.mozilla)
Attachment #8437944 - Flags: review?(bent.mozilla) → review+
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)?
Attachment #8437942 - Flags: review?(bent.mozilla) → review+
(Assignee)

Updated

5 years ago
Depends on: 1024312
https://hg.mozilla.org/mozilla-central/rev/00d924bed2d1
https://hg.mozilla.org/mozilla-central/rev/7ee90f5b0aac
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Assignee)

Comment 12

5 years ago
Sorry, forgot to set leave-open
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hm, what's left to do here? I like one(-ish) patch per bug usually. Can we just have another for any additional work?
(Assignee)

Comment 14

5 years ago
(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.
Yeah, let's just move to a new bug for that.
(Assignee)

Comment 16

5 years ago
Ok
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Summary: FileHandle: Rename FileHandle to IDBMutableFile and LockedFile to IDBFileHandle → FileHandle: Rename FileHandle to MutableFile and LockedFile to FileHandle
(Assignee)

Comment 17

5 years ago
Filed bug 1026735.
(Assignee)

Comment 18

5 years ago
Filed bug 1029209 for the last part - the "IDB" prefix.
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) ?
Flags: needinfo?(Jan.Varga)
(Assignee)

Comment 20

5 years ago
(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
Flags: needinfo?(Jan.Varga)
You need to log in before you can comment on or make changes to this bug.