Bug 33342 - http/tests/xmlhttprequest/event-listener-gc.html is flaky
Summary: http/tests/xmlhttprequest/event-listener-gc.html is flaky
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 33295 33297
  Show dependency treegraph
 
Reported: 2010-01-07 13:23 PST by Eric Seidel (no email)
Modified: 2015-02-13 22:37 PST (History)
7 users (show)

See Also:


Attachments
proposed fix (1.68 KB, patch)
2015-02-03 13:20 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
second attempt (1.15 KB, patch)
2015-02-04 22:59 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
third attempt (2.01 KB, patch)
2015-02-07 14:05 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
fourth attempt (2.15 KB, patch)
2015-02-12 22:14 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 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.
Comment 3 Csaba Osztrogonác 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
Comment 4 Eric Seidel (no email) 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?
Comment 5 Alexey Proskuryakov 2015-02-03 12:47:52 PST
The test still fails in the same way, and on Mac too.
Comment 6 Alexey Proskuryakov 2015-02-03 13:20:34 PST
Created attachment 245960 [details]
proposed fix
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2015-02-03 15:02:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Alexey Proskuryakov 2015-02-04 12:53:18 PST
That didn't fix it.
Comment 10 Alexey Proskuryakov 2015-02-04 22:59:55 PST
Created attachment 246091 [details]
second attempt
Comment 11 WebKit Commit Bot 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>
Comment 12 WebKit Commit Bot 2015-02-05 10:36:35 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Alexey Proskuryakov 2015-02-05 20:35:07 PST
Aaand.. That also didn't fix it!
Comment 14 Alexey Proskuryakov 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.
Comment 15 Alexey Proskuryakov 2015-02-07 14:05:49 PST
Created attachment 246216 [details]
third attempt
Comment 16 Darin Adler 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.
Comment 17 WebKit Commit Bot 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>
Comment 18 WebKit Commit Bot 2015-02-07 16:54:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 19 Alexey Proskuryakov 2015-02-08 20:35:43 PST
It still fails :)
Comment 20 Alexey Proskuryakov 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.
Comment 21 Alexey Proskuryakov 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.
Comment 22 Darin Adler 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.
Comment 23 WebKit Commit Bot 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>
Comment 24 WebKit Commit Bot 2015-02-13 22:37:47 PST
All reviewed patches have been landed.  Closing bug.