Closed Bug 1006485 Opened 6 years ago Closed 6 years ago

FileHandle: Rename FileHandle to MutableFile and LockedFile to FileHandle

Categories

(Core :: Storage: IndexedDB, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: janv, Assigned: janv)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files, 2 obsolete files)

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.
Blocks: 942542
Also IDBDatatabase.mozCreateFileHandle() -> IDBDatabase.createMutableFile().
Keywords: dev-doc-needed
Attached 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)
Attached 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
Comment on attachment 8422005 [details] [diff] [review]
patch

Hm, it seems I forgot to prefix the interfaces ...
Attachment #8422005 - Flags: review?(bent.mozilla)
Attachment #8422005 - Attachment is obsolete: true
Attachment #8422012 - Attachment is obsolete: true
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+
Depends on: 1024312
https://hg.mozilla.org/mozilla-central/rev/00d924bed2d1
https://hg.mozilla.org/mozilla-central/rev/7ee90f5b0aac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
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?
(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.
Ok
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Summary: FileHandle: Rename FileHandle to IDBMutableFile and LockedFile to IDBFileHandle → FileHandle: Rename FileHandle to MutableFile and LockedFile to FileHandle
Filed bug 1026735.
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)
(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.