Bug 161291

Summary: REGRESSION (r193286): Promise chain no longer prevent UI refresh
Product: WebKit Reporter: Kvet <mpcteam.kvet>
Component: WebCore Misc.Assignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Critical CC: buildbot, cdumez, commit-queue, dbates, esprehn+autocc, ggaren, jaffathecake, kangil.han, koivisto, rniwa, ryanhaddad, sam, simon.fraser, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar, Regression
Version: Safari Technology Preview   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews103 for mac-elcapitan
none
Patch none

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
<rdar://problem/28062149>
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]
Patch
Comment 5 Geoffrey Garen 2017-02-08 15:02:17 PST
Comment on attachment 300971 [details]
Patch

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

r=me

> 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]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/webappapis/scripting/event-loops/microtask_after_script.html
imported/w3c/web-platform-tests/custom-elements/upgrading.html
js/dom/modules/module-incorrect-relative-specifier.html
imported/w3c/web-platform-tests/custom-elements/adopted-callback.html
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]
Patch
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: 

https://build.webkit.org/results/Apple%20iOS%2010%20Simulator%20Release%20WK2%20(Tests)/r211961%20(3594)/http/tests/cache/disk-cache/disk-cache-remove-several-pending-writes-pretty-diff.html
Comment 15 Antti Koivisto 2017-02-09 11:43:53 PST
I would suspect r211963 for that.