Bug 33342

Summary: http/tests/xmlhttprequest/event-listener-gc.html is flaky
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, ggaren, gustavo, hamaji, levin, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 33295, 33297    
Attachments:
Description Flags
proposed fix
none
second attempt
none
third attempt
none
fourth attempt none

Eric Seidel (no email)
Reported 2010-01-07 13:23:10 PST
http/tests/xmlhttprequest/event-listener-gc.html failed on GTK Linux 64-bit Debug bot http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r52942%20(2212)/results.html --- layout-test-results/http/tests/xmlhttprequest/event-listener-gc-expected.txt 2010-01-07 13:01:07.450743702 -0800 +++ layout-test-results/http/tests/xmlhttprequest/event-listener-gc-actual.txt 2010-01-07 13:01:07.450743702 -0800 @@ -4,7 +4,4 @@ If the test passes, you'll see a series of 'PASS' messages below. -PASS: event handler fired after garbage collection. -PASS: event handler fired after garbage collection. -PASS: event handler fired after garbage collection.
Attachments
proposed fix (1.68 KB, patch)
2015-02-03 13:20 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
second attempt (1.15 KB, patch)
2015-02-04 22:59 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
third attempt (2.01 KB, patch)
2015-02-07 14:05 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
fourth attempt (2.15 KB, patch)
2015-02-12 22:14 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 3 2010-01-14 08:58:46 PST
Fail again, but on Qt bot: http://build.webkit.org/results/Qt%20Linux%20Release/r53268%20%285990%29/results.html
Eric Seidel (no email)
Comment 4 2010-01-15 11:52:08 PST
Another failure: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r53335%20(2435)/http/tests/xmlhttprequest/event-listener-gc-diffs.txt This test was last touched by Sinchiro 4 months ago, and looks to be originally from Geoff. http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/xmlhttprequest/event-listener-gc.html I'm not sure I really understand the failure condition. It's possible that sometimes these listeners are getting incorrectly destroyed?
Alexey Proskuryakov
Comment 5 2015-02-03 12:47:52 PST
The test still fails in the same way, and on Mac too.
Alexey Proskuryakov
Comment 6 2015-02-03 13:20:34 PST
Created attachment 245960 [details] proposed fix
WebKit Commit Bot
Comment 7 2015-02-03 15:02:46 PST
Comment on attachment 245960 [details] proposed fix Clearing flags on attachment: 245960 Committed r179572: <http://trac.webkit.org/changeset/179572>
WebKit Commit Bot
Comment 8 2015-02-03 15:02:51 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 9 2015-02-04 12:53:18 PST
That didn't fix it.
Alexey Proskuryakov
Comment 10 2015-02-04 22:59:55 PST
Created attachment 246091 [details] second attempt
WebKit Commit Bot
Comment 11 2015-02-05 10:36:30 PST
Comment on attachment 246091 [details] second attempt Clearing flags on attachment: 246091 Committed r179696: <http://trac.webkit.org/changeset/179696>
WebKit Commit Bot
Comment 12 2015-02-05 10:36:35 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 13 2015-02-05 20:35:07 PST
Aaand.. That also didn't fix it!
Alexey Proskuryakov
Comment 14 2015-02-06 10:50:57 PST
Tweaked test output a little bit in <http://trac.webkit.org/r179749>, hoping that this will shed some light on the issue.
Alexey Proskuryakov
Comment 15 2015-02-07 14:05:49 PST
Created attachment 246216 [details] third attempt
Darin Adler
Comment 16 2015-02-07 15:51:20 PST
Comment on attachment 246216 [details] third attempt As more and more tests depend on this, we should probably add a way to force layout to internals, and then only use "document.body.offsetTop" when outside the test runners.
WebKit Commit Bot
Comment 17 2015-02-07 16:53:54 PST
Comment on attachment 246216 [details] third attempt Clearing flags on attachment: 246216 Committed r179789: <http://trac.webkit.org/changeset/179789>
WebKit Commit Bot
Comment 18 2015-02-07 16:54:01 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 19 2015-02-08 20:35:43 PST
It still fails :)
Alexey Proskuryakov
Comment 20 2015-02-11 10:11:29 PST
Looks like even with the forced layout, there is enough work on the main thread that has higher priority than the timer. I could finally reproduce locally, and have a patch that fixes that, but want to do a bit more investigation into why timers are so low priority.
Alexey Proskuryakov
Comment 21 2015-02-12 22:14:29 PST
Created attachment 246507 [details] fourth attempt I couldn't find any relevant mistakes in how timers function. The work that delays our zero delay timer is from run loop observers that are serviced earlier, and IPC messages are also serviced earlier. So, let's just stop using a timer here.
Darin Adler
Comment 22 2015-02-13 21:52:56 PST
Comment on attachment 246507 [details] fourth attempt View in context: https://bugs.webkit.org/attachment.cgi?id=246507&action=review > LayoutTests/http/tests/xmlhttprequest/event-listener-gc.html:41 > +window.addEventListener("load", test); > +window.addEventListener("load", collect); No need for "window." on these lines of code.
WebKit Commit Bot
Comment 23 2015-02-13 22:37:40 PST
Comment on attachment 246507 [details] fourth attempt Clearing flags on attachment: 246507 Committed r180105: <http://trac.webkit.org/changeset/180105>
WebKit Commit Bot
Comment 24 2015-02-13 22:37:47 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.