1328138 - Remove XMLDocument's async IDL attribute

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Simon Pieters [:zcorpan]]

alert('async' in document.implementation.createDocument(null,null)) // true

https://dxr.mozilla.org/mozilla-central/source/dom/webidl/XMLDocument.webidl#22

This is not in any spec nor any other browser as far as I know.

document.async was removed from HTML spec in https://github.com/whatwg/html/commit/e236f46820b93d6fe2e2caae0363331075c6c4fb

web-platform-tests test added in: https://github.com/w3c/web-platform-tests/pull/4331

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

It seems that the effect this has is that if you create an XMLDocument and then set .async = false before loading it, it will load synchronously.  (Which will freeze the page on a network load, I guess?)  I wouldn't think we'd want this functionality anyway.

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Bug 1328138 - Get rid of document.async

No other browser supports it, it's not in the specs, and the
functionality it enables (synchronous loading of documents) is probably
not desirable.

Review commit: https://reviewboard.mozilla.org/r/165352/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165352/

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8894259 [details]
Bug 1328138 - Get rid of document.async

https://reviewboard.mozilla.org/r/165352/#review170800

We need some telemetry data about usage. Hopefully it is very close to 0.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

If I'm reading this correctly, 1.02% of pages invoke the getter, and 4.72% invoke the setter:

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-08-20&keys=__none__!__none__!__none__&max_channel_version=nightly%252F57&measure=USE_COUNTER2_XMLDOCUMENT_ASYNC_getter_PAGE&min_channel_version=null&processType=*&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2017-08-09&table=0&trim=1&use_submission_date=0
https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-08-21&keys=__none__!__none__!__none__&max_channel_version=nightly%252F57&measure=USE_COUNTER2_XMLDOCUMENT_ASYNC_setter_PAGE&min_channel_version=null&processType=*&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2017-08-09&table=0&trim=1&use_submission_date=0

Is that right?  If so, that's extremely high.  Is this used internally or in libraries or something?  It doesn't seem practical to remove unless we can figure out what's using it and fix it.

Comment 5

[Andrew McCreight [:mccr8]]

I think this is what the isNukedFromDocument for async subtest is testing. This fails in Firefox but passes in other browsers.

Comment 6

[(no longer active)]

Current use counter data from telemetry:

The data comes from Firefox 64 beta. It was collected from 508,758,104 top level page loads and 3,530,295,659 individual document loads.

ASYNC_getter:	0.001%	
ASYNC_setter:	0.007%	

Olli, what do you think for removal?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I think this should go with .load() https://searchfox.org/mozilla-central/source/dom/webidl/XMLDocument.webidl#17
And the now expired telemetry for that hints the usage is very very low.
So, I guess we could try.

Comment 8

[(no longer active)]

Attachment: Bug 1328138 - Remove XMLDocument.async

Comment 9

[Boris Zbarsky [:bzbarsky]]

Telemetry shows the setter used on .02% of pageloads.  I don't know where the numbers in comment 6 are coming from.  I'm looking at the beta62 data, which has well over an order of magnitude more data points than comment 6 cites (10.5e9 pageloads).  See https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-10-01&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=beta%252F62&measure=USE_COUNTER2_XMLDOCUMENT_ASYNC_setter_PAGE&min_channel_version=null&processType=*&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2018-05-08&table=0&trim=1&use_submission_date=0

Comment 10

[(no longer active)]

Hmm, I was using https://georgf.github.io/usecounters/index.html#kind=page&group=XMLDOCUMENT&channel=beta&version=64 but that only cites ~1.3e9 page loads.  On 62 beta the count is much higher as you noted.  On 63 it is lower, but on 60 and 61 the usage of the setter is close to 0.02%.

I guess this is too high for removal?  Do we have a specific guideline that we use for these cases?  IIRC Blink uses a threshold of 0.03%.

We don't seem to have separate telemetry for document.load() right?

Comment 11

[Boris Zbarsky [:bzbarsky]]

> I guess this is too high for removal?

I don't know.  0.02% is certainly much better than the numbers from comment 4...

> Do we have a specific guideline that we use for these cases?

Not yet.

I think the right thing to do is to put this behind a pref, flip the pref, and see what fallout, if any, is reported.

> We don't seem to have separate telemetry for document.load() right?

We do.  It's tracked by the UseOfDOM3LoadMethod and ChromeUseOfDOM3LoadMethod counters.  It's not an IDL usecounter because we wanted to track those separately, hence the different naming.

load() usage is _much_ lower than .async setter usage.  More like .003% of pageloads (so almost an order of magnitude less).  I'm not sure why, actually.

Comment 12

[(no longer active)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #11)
> > Do we have a specific guideline that we use for these cases?
> 
> Not yet.
> 
> I think the right thing to do is to put this behind a pref, flip the pref,
> and see what fallout, if any, is reported.

That sounds good to me.

> > We don't seem to have separate telemetry for document.load() right?
> 
> We do.  It's tracked by the UseOfDOM3LoadMethod and
> ChromeUseOfDOM3LoadMethod counters.  It's not an IDL usecounter because we
> wanted to track those separately, hence the different naming.
> 
> load() usage is _much_ lower than .async setter usage.  More like .003% of
> pageloads (so almost an order of magnitude less).  I'm not sure why,
> actually.

I see.  That boggles my mind...  It seems to suggest that there are *tons* pages that set document.async to false/true but then *don't* call document.load().  <https://georgf.github.io/usecounters/index.html#kind=page&group=DEPRECATED&channel=beta&version=62> says document.load() is used on 0.003% of top-level pages.

If my understanding above correct, hiding both should actually be quite safe, since setting async would only change the behavior of load() so for most of the existing consumers, it is a no-op already.

Comment 13

[(no longer active)]

Attachment: Bug 1328138 - Disable the XMLDocument.async API on trunk;

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e369b6ee8ff
Disable the XMLDocument.async API on trunk; r=bzbarsky

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0e369b6ee8ff

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This isn't fully fixed yet, though. Could you file a followup for the actual removal?

Comment 17

[(no longer active)]

(In reply to :Ms2ger (he/him; ⌚ UTC+1/+2) from comment #16)

This isn't fully fixed yet, though. Could you file a followup for the actual removal?

I intentionally didn't file a follow-up yet because it's still unclear what the follow-up work would be (we don't yet know if unshipping will be successful.) I have a calendar entry to revisit this if everything goes uneventful but if you really want a bug here is one for you: bug 1546112.

Comment 18

[ExE Boss]

Attachment: MDN: Browser compatibility table update

Comment 19

[Kohei Yoshino [:kohei]]

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2019/xmldocument-load-and-xmldocument-async-have-been-removed/