disallow blocking methods from microtasks |
|||||
Issue description
currently, this will show an alert:
new Promise(function(res,rej) { resolver = res }).then(function() { alert(42); });
res(23);
creating a nested message loop during microtask execution.
I'd argue that we should disallow any blocking API during microtasks.
,
Apr 21 2016
I think this would probably break a bunch of content which uses any new Promise based API and calls alert(). I think that content is bad... but we'll have to see how much shouting occurs. Can you post a patch that disables this and logs to the console an error instead and lets see what happens?
,
Apr 21 2016
,
May 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f5b86386accbd17d06a26475df674a2e1df8bf2 commit 0f5b86386accbd17d06a26475df674a2e1df8bf2 Author: jochen <jochen@chromium.org> Date: Wed May 04 21:03:59 2016 Disallow certain blocking DOM calls during microtask execution. BUG= 605517 R=adamk@chromium.org,esprehn@chromium.org,haraken@chromium.org Review-Url: https://codereview.chromium.org/1940253002 Cr-Commit-Position: refs/heads/master@{#391640} [add] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/fast/dom/modal-dialogs-during-microtasks-expected.txt [add] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/fast/dom/modal-dialogs-during-microtasks.html [add] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/fast/workers/resources/worker-xhr-microtasks.js [add] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/fast/workers/worker-xhr-microtasks-expected.txt [add] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/fast/workers/worker-xhr-microtasks.html [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-fetch-failure-output-expected.txt [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/http/tests/security/suborigins/crossorigin/suborigin-cors-fetch-failure-output.php [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/http/tests/serviceworker/sync-xhr-doesnt-deadlock.html [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/duplicate-revalidation-reload.html [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/bindings/core/v8/ExceptionState.h [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/DOMWindow.h [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/Deprecation.cpp [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/LocalDOMWindow.h [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/RemoteDOMWindow.cpp [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/RemoteDOMWindow.h [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/UseCounter.h [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/frame/Window.idl [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in [modify] https://crrev.com/0f5b86386accbd17d06a26475df674a2e1df8bf2/tools/metrics/histograms/histograms.xml
,
May 9 2016
We're using Bluebird to manage async calls to our back end. In the "then" block, we often call window.alert to report success to the user after a long-running operation. If an error is returned from the back end, we always call window.alert to report the error. Is it common knowledge that this is not OK to do? I've never even heard of this issue until we started getting this new deprecation warning from Chrome 52.0.2729.0. If this is common knowledge and we've missed it, then our bad -- we'll suck it up and work around it. But I can't help thinking that a lot of applications are following the same pattern and will have big problems with this.
,
May 9 2016
I'm trying to make it common knowledge by adding those warnings, so thanks for reaching out!
Would replacing the alert('error occurred') with setTimeout(function() { alert('error occurred'); }, 0) work for your use case?
,
May 9 2016
Yeah, off the top of my head, the setTimeout solution is probably what we would use in the error handler. (I feel better about calling window.alert in an error situation -- where the page could be in a weird state -- than I do about putting the error in a "modal" DIV on the page.) But this is pretty new and I have to do more testing before I'll be confident.
,
May 10 2016
+rbyers because compat
,
May 10 2016
Just checking: One of the forbidden APIs is window.confirm, right? That's not listed here: https://www.chromestatus.com/features/5647113010544640 but it seems like it should be included.
,
May 10 2016
done, thanks for the hint!
,
Jun 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80a379647fd83d34f5fbfed7ec1151f9fe28d916 commit 80a379647fd83d34f5fbfed7ec1151f9fe28d916 Author: jochen <jochen@chromium.org> Date: Thu Jun 09 09:18:42 2016 Make blocking of nested message loop causing APIs during microtasks experimental BUG= 605517 R=esprehn@chromium.org Review-Url: https://codereview.chromium.org/2050623002 Cr-Commit-Position: refs/heads/master@{#398822} [modify] https://crrev.com/80a379647fd83d34f5fbfed7ec1151f9fe28d916/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80a379647fd83d34f5fbfed7ec1151f9fe28d916 commit 80a379647fd83d34f5fbfed7ec1151f9fe28d916 Author: jochen <jochen@chromium.org> Date: Thu Jun 09 09:18:42 2016 Make blocking of nested message loop causing APIs during microtasks experimental BUG= 605517 R=esprehn@chromium.org Review-Url: https://codereview.chromium.org/2050623002 Cr-Commit-Position: refs/heads/master@{#398822} [modify] https://crrev.com/80a379647fd83d34f5fbfed7ec1151f9fe28d916/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
,
Jun 23 2016
Renaming Blink>Architecture to Blink>Internals
,
Jun 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6052c6b8df5a08fd9ea88243a7ad3c46182c7dfc commit 6052c6b8df5a08fd9ea88243a7ad3c46182c7dfc Author: jochen <jochen@chromium.org> Date: Fri Jun 24 09:27:08 2016 Un-deprecate modal dialogs during microtasks We still plan to disallow SyncXHR, but bump the removal to M54 BUG= 605517 R=rbyers@chromium.org Review-Url: https://codereview.chromium.org/2089353003 Cr-Commit-Position: refs/heads/master@{#401841} [delete] https://crrev.com/66ab9f9074c3d1082a7ab1062914fece3810c104/third_party/WebKit/LayoutTests/fast/dom/modal-dialogs-during-microtasks-expected.txt [delete] https://crrev.com/66ab9f9074c3d1082a7ab1062914fece3810c104/third_party/WebKit/LayoutTests/fast/dom/modal-dialogs-during-microtasks.html [modify] https://crrev.com/6052c6b8df5a08fd9ea88243a7ad3c46182c7dfc/third_party/WebKit/LayoutTests/fast/workers/worker-xhr-microtasks-expected.txt [modify] https://crrev.com/6052c6b8df5a08fd9ea88243a7ad3c46182c7dfc/third_party/WebKit/Source/core/frame/Deprecation.cpp [modify] https://crrev.com/6052c6b8df5a08fd9ea88243a7ad3c46182c7dfc/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
,
Aug 1 2016
This removal is going to cause severe issues in the software we develop. I work on business software that is installed on-premise, so removals like this without significant lead time is extremely troublesome. We use synchronous XHRs throughout our web application because we often need to ensure clear completion of a task before moving on to the next operation. Using a synchronous request ensures the user has not moved on to another task when he or she receives an error message for a failed operation (and you get UI blocking for free). I realize this isn't ideal in a consumer sense, but it makes sense for business software. Changing from synchronous to asynchronous requests is a significant code refactoring that cannot be done overnight, and then we need to allow for sufficient testing and time for the customer to upgrade. If this change is made according to the current timeline, we will have no choice but tell customers to stop using Chrome for now. I am fine with the deprecation, but I would like to request that the removal be delayed. I look at this like the showModalDialog removal. Although that also required a significant code change for us, we learned about it in the bug tracker and had a year or two to make changes to prepare ourselves. There was also a flag provided for those of us that couldn’t make the required code changes in time.
,
Aug 1 2016
Hey, thanks for reaching out!
Can you help me understand two points?
Assuming you have code like this:
p.then(function(v) {
// ...
var req = new XMLHttpRequest();
req.open("GET", "http://www.example.org/example.txt", false);
req.send();
//...
});
You should be able to replace it with code like this
p.then(function(v) {
var rs;
var p = new Promise(function(res, rej) { rs = res; });
setTimeout(function() {
// ...
var req = new XMLHttpRequest();
req.open("GET", "http://www.example.org/example.txt", false);
req.send();
//...
rs(v);
}, 0);
return p;
});
does that work for you? Is that mechanical enough so that the time window (3 more months before this would hit stable) is enough?
With regard to deprecations in general, are you aware of the plans to remove sync XHR all together: https://xhr.spec.whatwg.org/#sync-warning? It would be interesting to now how this influences your planning?
,
Aug 3 2016
jlutz777: ping?
,
Aug 4 2016
Hello, Sorry for the delayed response ... apparently I'm not getting emailed with updates. We could implement some workarounds similar to what you are suggesting, but the timeframe is still difficult. Our application is quite large with lots of configuration options, so testing can take weeks to fully test it out in all of the browsers. Obviously that is beyond the development time to make the changes, which aren't easy to pick out because we need to determine which areas are in microtasks and which aren't. Finally, customers install this on-premise, so they then need to do the actual upgrade. 3 months isn't sufficient time to get a fix, test it out, and then get the word out to all our customers that our current version of software will not be compatible with Chrome starting with version 54. As for the sync XHR deprecation, we are aware and have it on our radar, but we haven't acted on it yet. Worst case, we figured we would get a year or two lead time like with showModalDialog. I'm realizing that may have been a bad assumption. I appreciate the quick response and suggested fix for the issue!
,
Aug 5 2016
,
Aug 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9df4ea728b809a6a3a25f833e0293f656e6802da commit 9df4ea728b809a6a3a25f833e0293f656e6802da Author: jochen <jochen@chromium.org> Date: Fri Aug 05 11:00:46 2016 Remove code to block sync XHRs during microtask executions People really love sync XHRs it seems :-/ BUG= 605517 R=rbyers@chromium.org Review-Url: https://codereview.chromium.org/2211353003 Cr-Commit-Position: refs/heads/master@{#410028} [delete] https://crrev.com/30ae54999d1c86d368c8a94acf383114160fd19c/third_party/WebKit/LayoutTests/fast/workers/worker-xhr-microtasks-expected.txt [delete] https://crrev.com/30ae54999d1c86d368c8a94acf383114160fd19c/third_party/WebKit/LayoutTests/fast/workers/worker-xhr-microtasks.html [modify] https://crrev.com/9df4ea728b809a6a3a25f833e0293f656e6802da/third_party/WebKit/Source/core/frame/Deprecation.cpp [modify] https://crrev.com/9df4ea728b809a6a3a25f833e0293f656e6802da/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp [modify] https://crrev.com/9df4ea728b809a6a3a25f833e0293f656e6802da/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
,
Aug 5 2016
,
Aug 6 2016
It's not so much that people love sync XHRs, it's that we need them. As jlutz pointed out above, there are many situations where blocking the page is highly desirable. Because of the sync XHR deprecation we don't write new code that uses them, but our application requirements force us to instead write code that blocks the user from interacting with the page until an async XHR returns. Same result for the end user, more complex and fragile code for us. :( In the year 2016, it doesn't even make sense that sync XHRs are deprecated. The W3C made such a strong statement against sync XHRs because early browsers were single-threaded, so a long-running sync XHR would block the entire browser UI. This often led to the user needing to kill the whole browser process. Browsers have come a long way since then -- a sync XHR now only blocks its web page -- so the question of whether this behavior is desirable or detrimental should really be up to us as web developers.
,
Aug 9 2016
If I am understanding this correctly, this could have far-ranging negative consequences for many apps. Because of the asynchronous nature of so many js toolkits (Firebase, Parse, etc ad infinitum), the pattern of "call an async function (ajax etc), wait for the callback, take action" is ubiquitous. Isn't it likely there's tons of code out there doing "alert(error-of-some-sort)" on those callbacks? Won't this break all of that code? (including mine?). I fear this is breaking a very wide set of code on short notice to fix a not-that-likely bug? Just my $0.02.
,
Aug 16 2016
I encountered this when using Sync XHR in a fairly complex web worker. (Sync XHR is required because we are using Emscripten with a synchronous C++ codebase which will occasionally have to download content in a synchronous manner) I understand the concerns over Sync XHR in the main thread, but is it necessary to impact web workers with this change? It may be possible to add a large number of timeouts to avoid the microtask restriction, but that isn't a small change for us and since the sync XHRs occur only occasionally (but potentially in various different spots) it could slow down execution even when they are not being used.
,
Aug 22 2016
the point is that a microtask is already highly asynchronous. if you really need to block the page, you can as well block it later, as you don't have control over when the microtask runs anyways. anyways, we've rolled back the deprecation. thanks for all the feedback!
,
Sep 1 2016
Perhaps there's scope for versions of alert / confirm and so on that return Promises?
,
Sep 1 2016
The problem with alert and friends is not only that they're synchronous, but that they are spec-ed as popping up user-unfriendly UI. There are better alternatives. This is not the mid '90s. Use <dialog>. Use notifications. Please stop using alerts.
,
Sep 14 2016
I just saw this warning, we have code in the field that won't get updated for Chrome 54. What will be the behavior in Chrome 54? Will Chrome54 throw an exception when this is attempted, will it just skip the request without a warning, or will it skip the request and provide a warning message but no error is thrown?
,
Oct 17 2016
I'm running Chrome 54 and I'm (happily) surprised to see my old code still working. (I have sync XHRs inside promises, from a vendor-provided library for communicating with label printers.) I was panicking to update, but it seems like things are still working. Can I expect these sync XHRs to keep working in Chrome 54? (Of course, I'll keep looking for a workaround!) |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by adamk@chromium.org
, Apr 21 2016