Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

A path for disabling sync XHR #62

Closed
RByers opened this issue Nov 8, 2017 · 12 comments
Closed

A path for disabling sync XHR #62

RByers opened this issue Nov 8, 2017 · 12 comments
Assignees

Comments

@RByers
Copy link
Member

RByers commented Nov 8, 2017

Synchronous XHR is bad for performance, and doing blocking network access on your UI thread is a long-known anti-pattern. Some thoughts / notes from TPAC discussion here

@RByers RByers changed the title Work to disable sync XHR A path for disabling sync XHR Nov 8, 2017
@RByers RByers self-assigned this Nov 10, 2017
@RByers
Copy link
Member Author

RByers commented Nov 10, 2017

To summarize from the TPAC discussion, there was agreement between Chromium, Firefox and Edge that we should start with a Feature Policy that let's sites disable support for sync-XHR (causing it to fail similarly to a network failure). There are already major customers lined up (eg. AMP) looking to set this on some iframes they host to reduce their main thread jank.

@clelland is working on an implementation in chromium and will propose a concrete design here for discussion in a forum that can include Microsoft. Once there's rough consensus he'll propose a PR to the XHR spec.

/cc @annevk @cramforce

@RByers
Copy link
Member Author

RByers commented Jan 10, 2018

See discussion in whatwg/xhr#178

@RByers
Copy link
Member Author

RByers commented Mar 2, 2018

Note the sync-xhr policy is shipping in Chrome 65+ to enable opt-out (but doesn't change any default behavior of course).

@RByers
Copy link
Member Author

RByers commented Mar 2, 2018

/cc @AngeloKai @spanicker

@annevk
Copy link

annevk commented Mar 2, 2018

Why does this issue exist? Can't we discuss this between the XHR and Feature Policy repositories? It's not really an intervention, is it?

@RByers
Copy link
Member Author

RByers commented May 14, 2018

Tracking in the XHR repo makes sense to me (FP is really about the generic infrastructure, ideally not specific policies). We might someday want to do something more intervention-like (like flipping the policy default to disabled by default in some cases as part of a deprecation path), but we can always re-open this issue if it seems useful...

@RByers RByers closed this as completed May 14, 2018
@spanicker
Copy link

spanicker commented Jun 2, 2018

Hey all,
I am now working on disallowing sync xhr during beforunload and unload (page dismissal).
Could you clarify if this is an "intervention" or a change that is now blessed by the spec?

The spec has a warning but it says that "UAs may experiment with throwing exception" rather than it's okay to throw exception in certain cases. Could the spec be re-worded to disallow sync xhr in certain cases like page dismissal? Thanks.

Full text of warning:
"Synchronous XMLHttpRequest outside of workers is in the process of being removed from the web platform as it has detrimental effects to the end user’s experience. (This is a long process that takes many years.) Developers must not pass false for the async argument when current global object is a Window object. User agents are strongly encouraged to warn about such usage in developer tools and may experiment with throwing an "InvalidAccessError" DOMException when it occurs."

@RByers @annevk

@annevk
Copy link

annevk commented Jun 2, 2018

@spanicker I think if such a restriction is meant to be adopted by other browsers it'd ideally be standardized via changes to HTML and XMLHttpRequest.

@RByers
Copy link
Member Author

RByers commented Jun 4, 2018

Exactly. "intervention" issues like this are intended to track experimentation and brainstorming. This is likely one of those cases other engines won't know how supportive to be of until we've proven it's sufficiently web compatible, so we might not be able to land the necessary change in HTML/XHR specs in advance of shipping in Chrome. But I think, like all interventions, we'd want to start with at least having some spec PRs ready before shipping, then circle back with data post-stable and try to get spec changes landed.

All that said, it wouldn't surprise me if @AngeloKai or @patrickkettner were to say that Edge had active interest in this and that they'd review and help us get agreement on spec text now.

@spanicker
Copy link

@AngeloKai and @patrickkettner - could you comment on interest from Edge on disallowing Sync XHR from beforeunload (& unload).
There was agreement on this at TPAC per this doc:
https://docs.google.com/document/d/1G4-oIXQBcqysk4PNHBzCE4XAaGURipPTbajfx5iDIRc/edit
3rd point under "Conclusion from TPAC discussion: Agreed we should all try to: ...", l
listed as "Announce deprecation for unload/beforeunload case"
Though I wasn't in the room so wanted to check if there is any further context here, or is it okay to go ahead with this now.

@spanicker
Copy link

@RByers @ojanvafai - were any other browser reps present for that? Could we cite the notes in that TPAC discussion section as vendor agreement on deprecation of sync xhr from beforeunload / unload?

@RByers
Copy link
Member Author

RByers commented Jun 18, 2018

There were folks from Firefox and Apple in the room, but we had network issues preventing us from taking detailed minutes so I don't want to risk speaking for anyone in particular. I definitely think it's safe to say that Microsoft supported that plan as I believe it was @AngeloKai who came to us with that doc suggesting deprecation the unload case first. That should be sufficient for our intent process, but feel free to point out that the notes say that there was agreement at the meeting conclusion to "Announce deprecation for unload/beforeunload case".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants