Remove MutableFile/FileHandle support
Categories
(Core :: Storage: IndexedDB, task, P3)
Tracking
()
People
(Reporter: janv, Assigned: saschanaz)
References
(Blocks 3 open bugs)
Details
(Keywords: dev-doc-complete, site-compat, Whiteboard: idb-mutablefile)
Attachments
(14 files)
63.72 KB,
patch
|
Details | Diff | Splinter Review | |
15.41 KB,
patch
|
Details | Diff | Splinter Review | |
16.37 KB,
patch
|
Details | Diff | Splinter Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1500343 - Part 3: Remove dom.fileHandle.enabled usage in IDBRequest.cpp r=#dom-storage-reviewers
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Comment hidden (obsolete) |
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 3•5 years ago
|
||
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•5 years ago
|
||
Comment 5•5 years ago
|
||
Comment 6•5 years ago
|
||
Comment 7•5 years ago
|
||
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....
Updated•5 years ago
|
Comment 9•4 years ago
•
|
||
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?
Reporter | ||
Comment 10•4 years ago
|
||
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•4 years ago
|
||
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.
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 12•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Comment 13•3 years ago
|
||
Comment 14•3 years ago
|
||
bugherder |
Assignee | ||
Comment 15•2 years ago
|
||
DOMRequest is only used by IDBMutableFile/FileHandle, maybe it should be removed together?
Assignee | ||
Comment 16•2 years ago
|
||
Assignee | ||
Comment 17•2 years ago
|
||
Depends on D159730
Assignee | ||
Comment 18•2 years ago
|
||
Depends on D159731
Assignee | ||
Comment 19•2 years ago
|
||
Depends on D159732
Assignee | ||
Comment 20•2 years ago
|
||
The only use was in IDBDatabase::CreateMutableFile which is removed in part 1.
Depends on D159733
Assignee | ||
Comment 21•2 years ago
|
||
Of course this class should be removed, this is just to make it progressive.
Depends on D159734
Updated•2 years ago
|
Assignee | ||
Comment 22•2 years ago
|
||
Depends on D159856
Assignee | ||
Comment 23•2 years ago
|
||
Depends on D159891
Assignee | ||
Comment 24•2 years ago
|
||
Depends on D160383
Assignee | ||
Comment 25•2 years ago
|
||
Depends on D160384
Updated•2 years ago
|
Comment 26•2 years ago
|
||
Assignee | ||
Comment 27•2 years ago
|
||
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•2 years ago
|
||
bugherder |
Assignee | ||
Updated•2 years ago
|
Comment 29•2 years ago
|
||
Comment 30•2 years ago
|
||
Backed out for causing build bustages on ActorsParent.cpp.
-
Failure log for Bb builds Also affectes Hazard builds.
[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'
Assignee | ||
Comment 31•2 years ago
|
||
Huh, it seems GCC is better at detecting dead code 🙂
Comment 32•2 years ago
|
||
Comment 33•2 years ago
|
||
bugherder |
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
Updated•2 years ago
|
Comment 34•2 years ago
|
||
FF112 docs work for this can be tracked in https://github.com/mdn/content/issues/25360 (it is largely complete).