| Summary: | http/tests/xmlhttprequest/event-listener-gc.html is flaky | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||
| Component: | Tools / Tests | Assignee: | 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
Eric Seidel (no email)
2010-01-07 13:23:10 PST
Another failure, just now. Same bot: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r53052%20(2272)/http/tests/xmlhttprequest/event-listener-gc-diffs.txt Two in a row, actually: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r53053%20(2274)/http/tests/xmlhttprequest/event-listener-gc-diffs.txt Fail again, but on Qt bot: http://build.webkit.org/results/Qt%20Linux%20Release/r53268%20%285990%29/results.html 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? The test still fails in the same way, and on Mac too. Created attachment 245960 [details]
proposed fix
Comment on attachment 245960 [details] proposed fix Clearing flags on attachment: 245960 Committed r179572: <http://trac.webkit.org/changeset/179572> All reviewed patches have been landed. Closing bug. That didn't fix it. Created attachment 246091 [details]
second attempt
Comment on attachment 246091 [details] second attempt Clearing flags on attachment: 246091 Committed r179696: <http://trac.webkit.org/changeset/179696> All reviewed patches have been landed. Closing bug. Aaand.. That also didn't fix it! Tweaked test output a little bit in <http://trac.webkit.org/r179749>, hoping that this will shed some light on the issue. Created attachment 246216 [details]
third attempt
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 on attachment 246216 [details] third attempt Clearing flags on attachment: 246216 Committed r179789: <http://trac.webkit.org/changeset/179789> All reviewed patches have been landed. Closing bug. It still fails :) 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. 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 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 on attachment 246507 [details] fourth attempt Clearing flags on attachment: 246507 Committed r180105: <http://trac.webkit.org/changeset/180105> All reviewed patches have been landed. Closing bug. |