1500343 - Remove MutableFile/FileHandle support

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, task, P3)

Description

[Jan Varga [:janv]]

According to bug 1499547 comment 2, there are no uses of this API and we should just remove it.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

It turns out there's a proposal https://github.com/WICG/writable-files up for discussion at the WebPlat TPAC meeting tomorrow.  I'm going to look into this more today, but Jan if you want to glance at the spec and get any thoughts back to me, that'd be great.  In particular I want to better understand their mutability semantics, especially as they relate to IDB.

The writeable-files spec exists in the context of:
- A new SW "file" event proposal https://github.com/ewilligers/file-handling that's reminiscent of Firefox OS's MIME handling in many ways.  It would only be for installed apps.
- Which is similar to a "launch" event proposal https://github.com/WICG/sw-launch/ which is also largely about installed apps, although registerProtocolHandler() has a similar use-case.
- These are both historically related to https://github.com/chromium/ballista/blob/master/docs/explainer.md

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

It turns out the spec in comment 1 is more about file manipulation outside of IndexedDB.  The only relevance to IDB for the spec is persisting an access right to manipulate a file.  It would not affect our removal of this API.

Comment 3

[Simon Giesecke [:sg] [he/him]]

When opening this bug, it was assumed that there are no uses of IDBMutableFile/IDBFileHandle, but it turned out that this was only true for the beta channel. We we need to officially deprecate it, check the telemetry and also figure out what to do with existing databases which contain data for it (cf. https://bugzilla.mozilla.org/show_bug.cgi?id=1338603#c3).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Remove the unused createMutableFile and mozCreateFileHandle APIs.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Remove now-unused dom.fileHandle.enabled preference.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Remove the IDBFileHandle, IDBFileRequest, IDBMutableFile webidl interfaces. -- may not compile/run

Comment 7

[Boris Zbarsky [:bzbarsky]]

Andrew, I tried to do this at some point, but got too tied up in trying to remove as much as I could and hence deep in the weeds of MutableFile/File/Blob stuff. It might be worth someone removing just enough of this to remove DOMRequest, which is the real reason I was poking at it....

Comment 9

[Anne (:annevk)]

Hey Simon, given the apparent need to file bug 1639550 and the extremely low usage numbers for these APIs (2 pings for 24M samples on beta 83), I suggest we go ahead and remove these. For safety we could disable them on Nightly/Beta for one or two cycles first and we should email an intent to unship to dev-platform, but other than that I don't think there's anything needed here. What do you think?

Comment 10

[Jan Varga [:janv]]

We discussed about this recently. It seems we might want to implement NativeIO (https://github.com/fivedots/nativeio-explainer) which is very similar to our SimpleDB internal API.
NativeIO would be a good replacement for MutableFile/FileHandle (However, we don't have any concrete plans nor a timeline for that yet).
So yes, we can start the process of disabling it.

Comment 11

[Anne (:annevk)]

That looks interesting, though I don't think we necessarily need replacements for features that didn't get adopted and in general we should be removing features that are proprietary to Firefox as they are a maintenance burden for us and a potential web compatibility problem.

Comment 12

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Deprecate IDBDatabase.createMutableFile and IDBMutableFile.open r=janv

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/035a9b3d4547
Deprecate IDBDatabase.createMutableFile and IDBMutableFile.open r=janv,dom-storage-reviewers,emilio

Comment 14

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/035a9b3d4547

Comment 15

[Kagami [:saschanaz] (they/them)]

DOMRequest is only used by IDBMutableFile/FileHandle, maybe it should be removed together?

Comment 16

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 1: Remove IDBDatabase::CreateMutableFile r=#dom-storage-reviewers

Comment 17

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 2: Remove dom.fileHandle.enabled usage in ActorsParent.cpp r=#dom-storage-reviewers

Depends on D159730

Comment 18

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 3: Remove dom.fileHandle.enabled usage in IDBRequest.cpp r=#dom-storage-reviewers

Depends on D159731

Comment 19

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 4: Remove IDL for IDBFileHandle/FileRequest/MutableFile r=#dom-storage-reviewers

Depends on D159732

Comment 20

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 5: Remove PBackgroundIDBDatabaseRequest r=#dom-storage-reviewers

The only use was in IDBDatabase::CreateMutableFile which is removed in part 1.

Depends on D159733

Comment 21

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 6: Remove PBackgroundFileRequest r=#dom-storage-reviewers

Of course this class should be removed, this is just to make it progressive.

Depends on D159734

Comment 22

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 7: Remove PBackgroundFileHandle r=#dom-storage-reviewers

Depends on D159856

Comment 23

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 8: Remove PBackgroundMutableFile r=#dom-storage-reviewers

Depends on D159891

Comment 24

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 9: Remove DatabaseOrMutableFile r=#dom-storage-reviewers

Depends on D160383

Comment 25

[Kagami [:saschanaz] (they/them)]

Attachment: Bug 1500343 - Part 10: Remove IDBFileHandle/IDBFileRequest/IDBMutableFile r=#dom-storage-reviewers

Depends on D160384

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65fd33247185
Part 1: Remove IDBDatabase::CreateMutableFile r=dom-storage-reviewers,emilio,janv
https://hg.mozilla.org/integration/autoland/rev/07211e8132e4
Part 2: Remove dom.fileHandle.enabled usage in ActorsParent.cpp r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/6f6882b65188
Part 3: Remove dom.fileHandle.enabled usage in IDBRequest.cpp r=dom-storage-reviewers,jari

Comment 27

[Kagami [:saschanaz] (they/them)]

Landing some initial part of this to prevent bitrotting. Technically it causes intermediate state where IDL interfaces are still exposed while the functions to construct them are removed. But it should be okay as this pref has been disabled for a while and zero people are using it per the telemetry.

Comment 28

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/65fd33247185
https://hg.mozilla.org/mozilla-central/rev/07211e8132e4
https://hg.mozilla.org/mozilla-central/rev/6f6882b65188

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f9e20fcae19
Part 4: Remove IDL for IDBFileHandle/FileRequest/MutableFile r=dom-storage-reviewers,emilio,asuth
https://hg.mozilla.org/integration/autoland/rev/f6a065149ce1
Part 5: Remove PBackgroundIDBDatabaseRequest r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/220abddba749
Part 6: Remove PBackgroundFileRequest r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/de71281af75c
Part 7: Remove PBackgroundFileHandle r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/c5c7e5d59035
Part 8: Remove PBackgroundMutableFile r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/abe4c30b975e
Part 9: Remove DatabaseOrMutableFile r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/5d0b35ae2241
Part 10: Remove IDBFileHandle/IDBFileRequest/IDBMutableFile r=dom-storage-reviewers,asuth

Comment 30

[Iulian Moraru]

Backed out for causing build bustages on ActorsParent.cpp.

• Push with failures

• Failure log for Bb builds Also affectes Hazard builds.

• Backout link

[task 2023-02-22T22:55:40.893Z] 22:55:40     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/indexedDB'
[task 2023-02-22T22:55:40.893Z] 22:55:40     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/gcc/bin/g++ --sysroot /builds/worker/fetches/sysroot-x86_64-linux-gnu -std=gnu++17 -isystem /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/c++/7.5.0 -isystem /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/x86_64-linux-gnu/c++/7.5.0 -isystem /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/x86_64-linux-gnu -isystem /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include -o ActorsParent.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/dom/indexedDB -I/builds/worker/workspace/obj-build/dom/indexedDB -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/storage -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/third_party/sqlite3/src -I/builds/worker/checkouts/gecko/xpcom/build -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fno-exceptions -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -gdwarf-4 -freorder-blocks -Os -fno-omit-frame-pointer -funwind-tables -Werror -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wno-invalid-offsetof -Wno-error=deprecated -Wduplicated-cond -Wimplicit-fallthrough -Wlogical-op -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -Wformat-overflow=2 -Wno-psabi -fno-strict-aliasing -ffp-contract=off  -MD -MP -MF .deps/ActorsParent.o.pp   /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp
[task 2023-02-22T22:55:40.894Z] 22:55:40    ERROR -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:9410:6: error: 'void mozilla::dom::indexedDB::{anonymous}::Database::NoteInactiveMutableFile()' defined but not used [-Werror=unused-function]
[task 2023-02-22T22:55:40.895Z] 22:55:40     INFO -   void Database::NoteInactiveMutableFile() {
[task 2023-02-22T22:55:40.895Z] 22:55:40     INFO -        ^~~~~~~~
[task 2023-02-22T22:55:40.895Z] 22:55:40    ERROR -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:9402:6: error: 'void mozilla::dom::indexedDB::{anonymous}::Database::NoteActiveMutableFile()' defined but not used [-Werror=unused-function]
[task 2023-02-22T22:55:40.895Z] 22:55:40     INFO -   void Database::NoteActiveMutableFile() {
[task 2023-02-22T22:55:40.896Z] 22:55:40     INFO -        ^~~~~~~~
[task 2023-02-22T22:55:40.896Z] 22:55:40    ERROR -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:3840:1: error: 'static mozilla::dom::indexedDB::{anonymous}::ObjectStoreAddOrPutRequestOp::StoredFileInfo mozilla::dom::indexedDB::{anonymous}::ObjectStoreAddOrPutRequestOp::StoredFileInfo::CreateForMutableFile(mozilla::SafeRefPtr<mozilla::dom::indexedDB::FileInfo<mozilla::dom::indexedDB::DatabaseFileManager> >)' defined but not used [-Werror=unused-function]
[task 2023-02-22T22:55:40.896Z] 22:55:40     INFO -   ObjectStoreAddOrPutRequestOp::StoredFileInfo::CreateForMutableFile(
[task 2023-02-22T22:55:40.896Z] 22:55:40     INFO -   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2023-02-22T22:55:40.897Z] 22:55:40     INFO -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp: In member function 'virtual nsresult mozilla::dom::indexedDB::{anonymous}::DatabaseMaintenance::Run()':
[task 2023-02-22T22:55:40.898Z] 22:55:40  WARNING -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:13577:3: warning: 'maintenanceAction' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2023-02-22T22:55:40.898Z] 22:55:40     INFO -     switch (maintenanceAction) {
[task 2023-02-22T22:55:40.898Z] 22:55:40     INFO -     ^~~~~~
[task 2023-02-22T22:55:40.899Z] 22:55:40     INFO -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:13570:21: note: 'maintenanceAction' was declared here
[task 2023-02-22T22:55:40.899Z] 22:55:40     INFO -     MaintenanceAction maintenanceAction;
[task 2023-02-22T22:55:40.899Z] 22:55:40     INFO -                       ^~~~~~~~~~~~~~~~~
[task 2023-02-22T22:55:40.903Z] 22:55:40  WARNING -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:13563:18: warning: 'databaseIsOk' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2023-02-22T22:55:40.903Z] 22:55:40     INFO -     if (NS_WARN_IF(!databaseIsOk)) {
[task 2023-02-22T22:55:40.903Z] 22:55:40     INFO -                    ^
[task 2023-02-22T22:55:40.904Z] 22:55:40     INFO -  /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp:13557:8: note: 'databaseIsOk' was declared here
[task 2023-02-22T22:55:40.904Z] 22:55:40     INFO -     bool databaseIsOk;
[task 2023-02-22T22:55:40.904Z] 22:55:40     INFO -          ^~~~~~~~~~~~
[task 2023-02-22T22:55:40.905Z] 22:55:40     INFO -  cc1plus: all warnings being treated as errors
[task 2023-02-22T22:55:40.906Z] 22:55:40    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:669: ActorsParent.o] Error 1
[task 2023-02-22T22:55:40.906Z] 22:55:40     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/indexedDB'

Comment 31

[Kagami [:saschanaz] (they/them)]

Huh, it seems GCC is better at detecting dead code 🙂

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af5a18ddc6e3
Part 4: Remove IDL for IDBFileHandle/FileRequest/MutableFile r=dom-storage-reviewers,emilio,asuth
https://hg.mozilla.org/integration/autoland/rev/42a6fb62ca2e
Part 5: Remove PBackgroundIDBDatabaseRequest r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/08540aeb1905
Part 6: Remove PBackgroundFileRequest r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/410e053fee55
Part 7: Remove PBackgroundFileHandle r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/06df2ac13e2a
Part 8: Remove PBackgroundMutableFile r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/1f1abbf1f07d
Part 9: Remove DatabaseOrMutableFile r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/ec09f5de38b4
Part 10: Remove IDBFileHandle/IDBFileRequest/IDBMutableFile r=dom-storage-reviewers,asuth

Comment 33

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/af5a18ddc6e3
https://hg.mozilla.org/mozilla-central/rev/42a6fb62ca2e
https://hg.mozilla.org/mozilla-central/rev/08540aeb1905
https://hg.mozilla.org/mozilla-central/rev/410e053fee55
https://hg.mozilla.org/mozilla-central/rev/06df2ac13e2a
https://hg.mozilla.org/mozilla-central/rev/1f1abbf1f07d
https://hg.mozilla.org/mozilla-central/rev/ec09f5de38b4

Comment 34

[Hamish Willee]

FF112 docs work for this can be tracked in https://github.com/mdn/content/issues/25360 (it is largely complete).