About 50% of the time, loading the testcase triggers these six assertions: 1. ###!!! ASSERTION: Why are we being called?: '!mDidInitialReflow', file layout/base/nsPresShell.cpp, line 2373 2. ###!!! ASSERTION: initial containing block already created: 'nsnull == mInitialContainingBlock', file layout/base/nsCSSFrameConstructor.cpp, line 8760 3. ###!!! ASSERTION: Already have an undisplayed context entry for aContent: '!GetUndisplayedContent(aContent)', file layout/base/nsFrameManager.cpp, line 574 4. ###!!! ASSERTION: unexpected mInitialContainingBlock: 'processChildren ? !mInitialContainingBlock : mInitialContainingBlock == contentFrame', file layout/base/nsCSSFrameConstructor.cpp, line 4308 5. ###!!! ASSERTION: unexpected second call to SetInitialChildList: 'Not Reached', file layout/generic/nsContainerFrame.cpp, line 111 6. ###!!! ASSERTION: Why is the root in mDirtyRoots already?: 'mDirtyRoots.IndexOf(rootFrame) == -1', file layout/base/nsPresShell.cpp, line 2462 and then closing it triggers: 7. ###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file layout/base/nsPresShell.cpp, line 676

Good stuff. We get into nsMediaDocument::StartLayout when OnStartRequest fires. We grab the presshell, start layout on it. Then we do a synchronous paint, which flushes pending notifications. That triggers a frame reconstruct in the parent document (from the pending restyle), which blows away the presshell we just started layout, creates a new one, and starts layout on it. Then we return from the paint call, move on to the next presshell for the document (the newly-created one!) and proceed to try starting layout on it. Then we lose.

The key here is that the image needs to come in after the attr set happened but before the restyle is processed. Fun. :( Not sure how to guarantee that in a test.

Pushed changeset f05dd60fa3ed.

Comment on attachment 343977 [details] [diff] [review] Do it like the sinks I do think that we should take this on branch, but I'd request apprval11.9.0.5 (not .4) if I could...

Again, I'd probably try for the next branch release not this very next one.

Comment on attachment 343977 [details] [diff] [review] Do it like the sinks approved for 1.9.0.5 and 1.8.1.19, a=dveditz for release-drivers (please watch tinderbox for the tree to open before landing)

Fixed on both branches.

verified fixed for 1.8.1.19 - using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.20pre) Gecko/2008120304 Firefox/2.0.0.20pre (1.8.1 Debug Build) and the Testcase from Jesse. I do not see the Assertions from Comment #0, only ###!!! ASSERTION: Adding child where we already have a child? This will likely misbehave: 'Error', file /work/mozilla/builds/1.8.1/mozilla/xpfe/components/shistory/src/nsSHEntry.cpp, line 580, which is a different Bug. --> Verified fixed 1.8.1.19

also verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2008120309 Firefox/3.0.6pre (Debug Build) and the testcase from Jesse. I do not see the Assertions from Comment #0 when using the Testcase --> Verified 1.9.0.5

Comment on attachment 344163 [details] [diff] [review] 1.8 branch non-ugly patch a=asac for 1.8.0