Summary: | WebKit relies on unpredictable timing for onload events | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Peebles <pumpkingod> | ||||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, mitz | ||||||||
Priority: | P1 | Keywords: | Regression | ||||||||
Version: | 420+ | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 5122, 5487, 6560 | ||||||||||
Attachments: |
|
Description
Daniel Peebles
2006-03-09 21:52:02 PST
In fact, it even happens when just running it in the debugger with no breakpoints or pauses. It seems pretty random Sounds like the same as this random failure I got from BuildBot yesterday: <http://build.webkit.org/results/post-commit-powerpc-mac-os-x/990/results.html>. Created attachment 6975 [details]
A test case for the bug
To reproduce the bug, try putting this in your WebKit folder, and type
bash Bug.bash > results
This is simultaneously running the same test 20 times to make sure it gets nice and slow, and outputting it to the results file. If you check the results when it's done, you'll find that several of the results give the full render tree of the unmodified (by the javascript onload event) page, and others show a text only representation of the modified page.
It's apparently not specific to DumpRenderTree, but that's just where I ran into it. I'll look deeper into it tomorrow The root cause for the undeterministic behavior is that <object> (and perhaps other) subframes are created and loaded by the render tree, triggered either by the layout timer or by event dispatch or by other mechanisms. Frame::checkCompleted(), which emits the load DOM event, waits for the subframes to load, but obviously only if they have already been created. This warrants a bug by itself, since it means that the document load event may arbitrarily be emitted before subframes have loaded. What's causing the problem here, though, is that in the case that the subframe gets created and its main resource fails to load, the order of calls in -[WebDataSource _receivedMainResourceError:complete:] is such that the WebFrame reports completion (which invokes DRT's waiting delegate and causes it to dump) before the WebCore Frame checks for completion (which fires the onload handler, calling dumpAsText, waitUntilDone etc. when it's too late). Created attachment 6986 [details] Reduction This file reliably reproduces the bug. When you invoke DRT on it, the lines appear in the reverse order, since the dump is triggered before the load event. If you delete the <script></script> on the fifth line, the lines appear in the correct order, since the subframe loading isn't triggered. The order of the calls in -[WebDataSource _receivedMainResourceError:complete:] was changed in r10901 to fix <rdar://problem/3853672> (Malformed HTML using crashes Safari in NSFireTimer), so part of this bug is probably a regression. Created attachment 7096 [details] Emit load events before notifying the WebFrame load delegate Retaining the bridge and checking the cancelled flag in -[WebLoader didFailWithError:] are sufficient to prevent the <rdar://problem/3853672> crash (or an assertion failure). The updated acid2 results should finally fix bug 5487. This patch obviously doesn't address the other issue I mentioned in comment #5. I am pretty sure now (also based on the date bug 5487 was reported) that this is a regression from the fix for <rdar://problem/3853672>. (In reply to comment #5) > The root cause for the undeterministic behavior is that <object> (and perhaps > other) subframes are created and loaded by the render tree, triggered either by > the layout timer or by event dispatch or by other mechanisms. > > Frame::checkCompleted(), which emits the load DOM event, waits for the > subframes to load, but obviously only if they have already been created. This > warrants a bug by itself, since it means that the document load event may > arbitrarily be emitted before subframes have loaded. Either Maciej or Hyatt (can't remember which) mentioned to me the other day that we probably should be loading subframes based on the DOM tree, not the render tree. Comment on attachment 7096 [details]
Emit load events before notifying the WebFrame load delegate
OK. Seems like we can do this -- taking a risk sounds good. Shouldn't have braces around that return in WebLoader.m.
r=me
I comitted this patch |