Bug 167652 - imported/w3c/web-platform-tests/shadow-dom/slotchange.html is a flaky failure
Summary: imported/w3c/web-platform-tests/shadow-dom/slotchange.html is a flaky failure
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on: 177597
Blocks: 148695
  Show dependency treegraph
 
Reported: 2017-01-31 10:57 PST by Ryan Haddad
Modified: 2018-10-01 20:46 PDT (History)
15 users (show)

See Also:


Attachments
Reduction (1.17 KB, text/html)
2018-09-21 19:55 PDT, Ryosuke Niwa
no flags Details
Fixes the bug (6.13 KB, patch)
2018-09-21 20:58 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2017-01-31 10:57:24 PST
LayoutTest imported/w3c/web-platform-tests/shadow-dom/slotchange.html is a flaky failure

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r211432%20(1844)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fshadow-dom%2Fslotchange.html

--- /Volumes/Data/slave/sierra-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/shadow-dom/slotchange-expected.txt
+++ /Volumes/Data/slave/sierra-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/shadow-dom/slotchange-actual.txt
@@ -1,9 +1,9 @@
 
 Harness Error (TIMEOUT), message = null
 
-PASS slotchange event: Append a child to a host. 
-PASS slotchange event: Remove a child from a host. 
-PASS slotchange event: Remove a child before adding an event listener. 
+TIMEOUT slotchange event: Append a child to a host. Test timed out
+TIMEOUT slotchange event: Remove a child from a host. Test timed out
+TIMEOUT slotchange event: Remove a child before adding an event listener. Test timed out
 PASS slotchange event: Change slot= attribute to make it un-assigned. 
 TIMEOUT slotchange event: Change slot's name= attribute so that none is assigned. Test timed out
 PASS slotchange event: Change slot= attribute to make it assigned.
Comment 1 Ryan Haddad 2017-02-02 13:38:27 PST
Marked test as flaky in http://trac.webkit.org/projects/webkit/changeset/211593
Comment 3 Radar WebKit Bug Importer 2018-08-01 21:18:40 PDT
<rdar://problem/42841417>
Comment 4 Ryosuke Niwa 2018-09-21 19:55:38 PDT
Created attachment 350479 [details]
Reduction

The issue here is that the JS wrapper of a HTMLSlotElement can be prematurely collected. We need to use the GC retention mechanism we introduced in https://bugs.webkit.org/show_bug.cgi?id=184307 to keep these slot elements alive.
Comment 5 Ryosuke Niwa 2018-09-21 20:58:42 PDT
Created attachment 350486 [details]
Fixes the bug
Comment 6 Ryosuke Niwa 2018-09-21 20:59:31 PDT
Comment on attachment 350486 [details]
Fixes the bug

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

> LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers.html:27
> +            const x = new Array(100);

This path is used for in-browser testing. This test also catches the same bug in Chrome sometimes LOL
Comment 7 Build Bot 2018-09-21 21:02:35 PDT
Attachment 350486 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MutationObserver.cpp:193:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Saam Barati 2018-09-24 15:18:47 PDT
Comment on attachment 350486 [details]
Fixes the bug

r=me seems reasonable
Comment 9 Ryosuke Niwa 2018-09-24 16:11:38 PDT
Comment on attachment 350486 [details]
Fixes the bug

Clearing flags on attachment: 350486

Committed r236440: <https://trac.webkit.org/changeset/236440>
Comment 10 Ryosuke Niwa 2018-09-24 16:11:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Ryosuke Niwa 2018-10-01 20:46:04 PDT
Removed the failing test expectation in https://trac.webkit.org/r236710 now that I observe the test isn't failing on the flakiness dashboard.