Summary: | Prevent synchronous XHR in beforeunload / unload event handlers | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, commit-queue, darin, dbates, ews-watchlist, ggaren, japhet, rniwa, sam, webkit-bug-importer, youennf | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
URL: | https://www.chromestatus.com/feature/4664843055398912 | ||||||||||||
Bug Depends on: | 206315 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Chris Dumez
2019-12-05 14:02:01 PST
Created attachment 384955 [details]
Patch
Created attachment 384970 [details]
Patch
Comment on attachment 384970 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384970&action=review > Source/WebCore/loader/DocumentThreadableLoader.cpp:134 > + if (!m_async && (!document.page() || !document.page()->areSynchronousLoadsAllowed())) { Seems like we want to put this behind some kind of a setting / feature flag? Do other browsers forbid this as well? It seems like if we want to make this kind of change, we should first understand the compatibility story and if it's ok, make a change to HTML5 to standardize this. Otherwise, it's not clear why this is an ok change to make. (In reply to Ryosuke Niwa from comment #5) > Do other browsers forbid this as well? No, other browsers do not do this at this time. (In reply to Ryosuke Niwa from comment #5) > Do other browsers forbid this as well? Please see https://www.chromestatus.com/feature/4664843055398912 (In reply to Chris Dumez from comment #7) > (In reply to Ryosuke Niwa from comment #5) > > Do other browsers forbid this as well? > > No, other browsers do not do this at this time. Sorry, I meant the opposite. Not sure what happened there: Chrome has been testing this for a while: https://www.chromestatus.com/feature/4664843055398912 (In reply to Ryosuke Niwa from comment #4) > Comment on attachment 384970 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384970&action=review > > > Source/WebCore/loader/DocumentThreadableLoader.cpp:134 > > + if (!m_async && (!document.page() || !document.page()->areSynchronousLoadsAllowed())) { > > Seems like we want to put this behind some kind of a setting / feature flag? Good idea, will do. Created attachment 385015 [details]
Patch
Created attachment 385025 [details]
Patch
Comment on attachment 385025 [details] Patch Clearing flags on attachment: 385025 Committed r253213: <https://trac.webkit.org/changeset/253213> All reviewed patches have been landed. Closing bug. |