1499547 - Consider removing DOMRequest

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task, P5)

Description

[Andrew McCreight [:mccr8]]

Maybe this is completely hopeless, but DOMRequest is the cause of one of the failures in WPT tests that we fail and all other browsers pass (dom/historical.html) so I figured we should at least have a bug on file for the removal, even if just to WONTFIX it with some kind of justification for it.

(I looked through the summary of every bug with DOMRequest in the name and didn't see that any had ever been filed about removing it.)

Comment 1

[Boris Zbarsky [:bzbarsky]]

The only places DOMRequest is used are:

1) BrowserElement.webidl.  We could switch this to Promise, I bet.
2) As an interface IDBFileRequest, which is used in IDBFileHandle, inherits from.
   Looks like that's some sort of Gecko-only stuff?
3) IDBMutableFile directly.

I wonder whether the IDBMutableFile/FileRequest/FileHandle stuff is actually used...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Looks like we have telemetry for createMutableFile and mozCreateFileHandle on IDBDatabase, which are the relevant entry points.  Use counter for createMutableFile at https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-08-30&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=beta%252F62&measure=USE_COUNTER2_IDBDATABASE_CREATEMUTABLEFILE_PAGE&min_channel_version=null&processType=content&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2018-06-25&table=0&trim=1&use_submission_date=0> says there are no uses at all: 0 sample sum out  of 8.78e9 samples.

The use counter for mozCreateFileHandle seems to not exist at all, suggesting that we were never hitting that codepath.

This is sounding like stuff we might be able to just remove...

Comment 3

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

The mutable file handle stuff is all b2g-era stuff exposed by IndexedDB and the dead DeviceStorage API (https://developer.mozilla.org/en-US/docs/Archive/B2G_OS/API/Device_Storage_API) that we probably should indeed  remove.  :janv, do you concur?

In terms of consumers, the Mozilla dweb project at https://github.com/mozilla/libdweb seems to be trying to bring back many of the b2g APIs that were removed/disabled so that they can be exposed to WebExtensions.  In particular, https://github.com/Gozala/random-access-idb-mutable-file/ builds on top of IDBMutableFile.  Note that I don't think that's a reason to keep the API around in IndexedDB.  IndexedDB's clever refcounted blob storage seems like a sufficient abstraction layer for higher level storage stuff to build on.

Comment 4

[Jan Varga [:janv]]

Yeah, the counters were added in bug 1271457. There are no uses, so I think it's time to remove it.
I filed bug 1500343 for that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK, so the BrowserElement API... Is it used at all?  Could we just remove it?

Comment 6

[(no longer active)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #5)
> OK, so the BrowserElement API... Is it used at all?  Could we just remove it?

IIRC it's used by the devtools team for the responsive design view and that's why we've had to keep it around.  (I'd love if someone could convince them to switch to <xul:browser> but perhaps in the last year or two other consumers have crept in.)

Perhaps it would be nice to find a way to prevent new consumers from using the API at least.  :-)

Comment 7

[Boris Zbarsky [:bzbarsky]]

> IIRC it's used by the devtools team for the responsive design view

Well, <iframe mozbrowser> is used.  But is the actual script API used?  Initial searchfoxing on some of the methods was only turning up test uses.

I guess I should try disabling the direct API tests, adding some MOZ_CRASH and doing a trypush...

Comment 8

[(no longer active)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #7)
> > IIRC it's used by the devtools team for the responsive design view
> 
> Well, <iframe mozbrowser> is used.  But is the actual script API used? 
> Initial searchfoxing on some of the methods was only turning up test uses.

Removing the ones that aren't used above would be super nice!

Comment 9

[Gregory Pappas [:gregp]]

Attachment: Bug 1499547 - Remove DOMRequest

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/31cf6b1269c8
Remove DOMRequest r=webidl,smaug,emilio

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/31cf6b1269c8

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Is this something we should call out in the Fx125 relnotes?

Comment 13

[Gregory Pappas [:gregp]]

Probably not. This interface was very obscure and can't be constructed by JS.

Comment 14

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

I think this should have wait a bit until we could see it's completely good on 124, because the whole point of having a pref is to make it easy to reenable it without backing out any big chunk of patch.

Comment 15

[Gregory Pappas [:gregp]]

Agreed. Should we back out now or wait and hope that there's no issue?

Comment 16

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

Backing out would be the safest option, yup.

Comment 17

[Gregory Pappas [:gregp]]

Sorry for forgetting about this conversation 😬

So far, it seems we've gotten lucky and a backout hasn't been necessary.