New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 605517 link

Starred by 13 users

Issue metadata

Status: WontFix
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

disallow blocking methods from microtasks

Project Member Reported by jochen@chromium.org, Apr 21 2016

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.
 

Comment 1 by adamk@chromium.org, Apr 21 2016

I have this nagging memory that we looked into this back when we were specifying and implementing MutationObservers. Google tells me that  issue 374115  is one place where this came up before, though that sounds specific to the devtools.
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?
Components: Blink>Architecture
Project Member

Comment 4 by bugdroid1@chromium.org, 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

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.
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?
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.

Comment 8 by jochen@chromium.org, May 10 2016

Cc: rbyers@chromium.org
Owner: jochen@chromium.org
Status: Assigned (was: Untriaged)
+rbyers because compat
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.
done, thanks for the hint!
Project Member

Comment 11 by bugdroid1@chromium.org, 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

Project Member

Comment 12 by bugdroid1@chromium.org, 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

Comment 13 by tkent@chromium.org, Jun 23 2016

Components: -Blink>Architecture Blink>Internals
Renaming Blink>Architecture to Blink>Internals

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.
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?
jlutz777: ping?
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!
Cc: jochen@chromium.org
 Issue 634308  has been merged into this issue.
Status: WontFix (was: Assigned)
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.
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. 
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. 
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!
Perhaps there's scope for versions of alert / confirm and so on that return Promises?

Comment 27 by a...@chromium.org, 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.
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?
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