Bug 161291 - REGRESSION (r193286): Promise chain no longer prevent UI refresh
Summary: REGRESSION (r193286): Promise chain no longer prevent UI refresh
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari Technology Preview
Hardware: All All
: P2 Critical
Assignee: Sam Weinig
Keywords: InRadar, Regression
Depends on:
Reported: 2016-08-27 09:27 PDT by Kvet
Modified: 2017-02-09 11:55 PST (History)
16 users (show)

See Also:

Patch (5.59 KB, patch)
2017-02-08 14:57 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (968.71 KB, application/zip)
2017-02-08 15:36 PST, Build Bot
no flags Details
Patch (9.89 KB, patch)
2017-02-08 15:52 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kvet 2016-08-27 09:27:25 PDT
In Safari 10, Safari Technology Preview and Webkit Nightly Build behavour of native Promise object changed from that one in Safari 9.

Now during microtasks execution there are unnecessary UI updates. This change can break existing codebase because all (latest Chrome, FireFox, IE, Edge) browsers that support Promise object behaves correctly. Even promise polyfill in checked browsers (IE9, Android 4 Browser) works correctly. I assume that this is very critical bug in Safari 10 that breaks down compatibility with another browsers.

Issue can be reproduced in https://jsfiddle.net/Lo7utx4s/ . Clicking on 'TEST PROMISE BEHAVIOR' should not cause blue rectangle blinks with white or black.
Comment 1 Alexey Proskuryakov 2016-08-29 13:53:55 PDT
Regressed in https://trac.webkit.org/changeset/193286
Comment 2 Radar WebKit Bug Importer 2016-08-29 13:54:39 PDT
Comment 3 Jake Archibald 2017-02-08 08:24:18 PST
This causes setTimeout to fire before promises in some cases. Demo: http://output.jsbin.com/xenumo/quiet
Comment 4 Sam Weinig 2017-02-08 14:57:14 PST
Created attachment 300971 [details]
Comment 5 Geoffrey Garen 2017-02-08 15:02:17 PST
Comment on attachment 300971 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=300971&action=review


> Source/WebCore/dom/Microtasks.cpp:81
> +    Vector<std::unique_ptr<Microtask>> toKeep;
> +    do {

I think this would be better as a while loop, since the queue can be initially empty at a checkpoint, right?
Comment 6 Geoffrey Garen 2017-02-08 15:13:39 PST
Mac bot:

Regressions: Unexpected text-only failures (4)
  imported/w3c/web-platform-tests/custom-elements/adopted-callback.html [ Failure ]
  imported/w3c/web-platform-tests/custom-elements/upgrading.html [ Failure ]
  imported/w3c/web-platform-tests/html/webappapis/scripting/event-loops/microtask_after_script.html [ Failure ]
  js/dom/modules/module-incorrect-relative-specifier.html [ Failure ]
Comment 7 Build Bot 2017-02-08 15:35:59 PST
Comment on attachment 300971 [details]

Attachment 300971 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3027786

New failing tests:
Comment 8 Build Bot 2017-02-08 15:36:04 PST
Created attachment 300976 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 Sam Weinig 2017-02-08 15:52:17 PST
Created attachment 300978 [details]
Comment 10 Sam Weinig 2017-02-08 17:44:27 PST
Committed r211913: <http://trac.webkit.org/changeset/211913>
Comment 12 Yusuke Suzuki 2017-02-09 10:12:48 PST
(In reply to comment #11)
> This change may have caused:
> https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/
> r211913%20(3409)/http/tests/misc/module-absolute-url-pretty-diff.html

I'll land the fix for that.
Comment 13 Yusuke Suzuki 2017-02-09 10:17:32 PST
Committed r211968: <http://trac.webkit.org/changeset/211968>
Comment 14 Ryan Haddad 2017-02-09 10:23:28 PST
It also looks like it caused http/tests/cache/disk-cache/disk-cache-remove-several-pending-writes.html to fail: 

Comment 15 Antti Koivisto 2017-02-09 11:43:53 PST
I would suspect r211963 for that.