Bug 171445

Summary: LayoutTests/js/dom/Promise-static-all/race.html are flakey - Unhandled Promise Rejection messages
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebCore JavaScriptAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jlewis3, joepeck, ryanhaddad, saam, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix saam: review+

Description Joseph Pecoraro 2017-04-28 12:55:08 PDT
Summary:
LayoutTests/js/dom/Promise-static-all/race.html are flakey - Unhandled Promise Rejection messages

The layout tests creates two rejected promises p7, and p8 but does nothing with them. They are legitimate unhandled promise rejections.

These promises can get garbage collected during the test run. So the logs depend on whether or not they get GC'd before the RejectionTracker's task happens.

I'm going to silence the unhandledrejection errors in these tests.
Comment 1 Joseph Pecoraro 2017-04-28 13:02:42 PDT
> These promises can get garbage collected during the test run.

Hmm, actually I don't think thats true. They are in window.p7 and window.p8.

Either way, I think disabling the messages is the right thing to do but I'll look into this a bit more.
Comment 2 Joseph Pecoraro 2017-04-28 13:06:06 PDT
Running the test in the browser I always see the unhandled rejection errors, which is the behavior I would expect. The messages are not important for this test so I'm going to silence them.

There may be some subtle issue here regarding the order of operations between microtasks and tasks, but again not important for this test and not particularly important in general for these optional events.
Comment 3 Joseph Pecoraro 2017-04-28 13:07:14 PDT
Created attachment 308585 [details]
[PATCH] Proposed Fix
Comment 4 Joseph Pecoraro 2017-04-28 13:07:40 PDT
For reference:
https://build.webkit.org/results/Apple%20iOS%2010%20Simulator%20Release%20WK2%20(Tests)/r215929%20(1023)/results.html

The diffs look like:
--- /Volumes/Data/slave/ios-simulator-10-release-tests-wk2/build/layout-test-results/js/dom/Promise-static-all-expected.txt
+++ /Volumes/Data/slave/ios-simulator-10-release-tests-wk2/build/layout-test-results/js/dom/Promise-static-all-actual.txt
@@ -1,3 +1,5 @@
+CONSOLE MESSAGE: line 1: Unhandled Promise Rejection: p7
+CONSOLE MESSAGE: line 1: Unhandled Promise Rejection: p8
 Test Promise.all
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Comment 5 Joseph Pecoraro 2017-04-28 13:14:32 PDT
<https://trac.webkit.org/r215938>
Comment 7 Ryan Haddad 2017-04-28 17:47:33 PDT
Reopening this per Matt's comment.
Comment 8 Joseph Pecoraro 2017-04-28 18:14:31 PDT
Comment on attachment 308585 [details]
[PATCH] Proposed Fix

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

> LayoutTests/js/dom/Promise-static-all.html:13
> +// window.onunhandledrejection = () => false;

Oops, this line was commented out by accident.
Comment 9 Joseph Pecoraro 2017-04-28 18:19:55 PDT
Fixed in: https://trac.webkit.org/r215966

Thanks for pointing it out!