http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1305852691.1305852965.4801.gz&fulltext=1#err0 seen on push http://hg.mozilla.org/mozilla-central/rev/9197b91fd9f4 there are several other rare oranges of the reftest-wait to be removed variety.. its not clear to me that they are dups or not. REFTEST TEST-START | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/554230-1.xhtml REFTEST TEST-PASS | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/554230-1.xhtml | (LOAD ONLY) REFTEST INFO | Loading a blank page REFTEST TEST-START | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/552651.html REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/552651.html | load failed: timed out waiting for reftest-wait to be removed REFTEST INFO | Saved log: START file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/552651.html REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering WaitForTestEnd REFTEST INFO | Saved log: [CONTENT] WaitForTestEnd: Adding listeners REFTEST INFO | Saved log: Initializing canvas snapshot
So why is that 1000ms timeout there? That said, reftest uses a 5 min timeout, so I'm a little surprised this test timed out...
Iirc, setTimeout() is used because I was unaware of executeSoon(). The 1000ms is random.
Let's change that for a start, then.
Change setTimeout() or 1000ms? Bigger or lower value? :) The idea of the test is to make sure we don't leak if we abort an xhr in state 3. See bug #552651, comment #12 for why it ended up as a crashtest. My understanding is that clearing the classname is necessary to make the test finish..? (A quick search reveals that a number of crashtests in layout/base seems to use this technique.) The only way I can interpret comment #0 is that clearing the classname does not happen (hence the test times out) indicating that setTimeout failed..?
> Change setTimeout() or 1000ms? Bigger or lower value? :) Use executeSoon. Chances are, the machine was under heavy load and the sync network load plus setTimeout call just took longer than the reftest timeout....
How would executeSoon() help in this situation? (Btw, the xhr loads async if this is what you mean by "sync network load".)
> How would executeSoon() help in this situation? By not going to the event loop for as long? Wait. So the setTimeout is just needed... why? To make sure we stop the test after the async XHR gets aborted?
(In reply to comment #7) > To make sure we stop the > test after the async XHR gets aborted? Yes. I'd be happy to learn about other, more robust ways.
Can you not just listen for onreadystatechange going into state 4?
Fair enough - this test works locally.
Assignee: nobody → bjarne
Status: NEW → ASSIGNED
Attachment #534702 - Flags: review?(bzbarsky)
Comment on attachment 534702 [details] [diff] [review] Dropping timeout, relying on getting to readystate==4 r=me
Attachment #534702 - Flags: review?(bzbarsky) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Can anyone please provide some guidelines that I can use to verify this fix? Do I need to create any specific environment to test it? Thanks!
You need to log in before you can comment on or make changes to this bug.