528 bytes, application/xhtml+xml
1.14 KB, patch
|Details | Diff | Splinter Review|
1.19 KB, patch
Alexander Sack: approval1.8.0.next+
|Details | Diff | Splinter Review|
Created attachment 343940 [details] testcase (has external dependencies) 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.
Created attachment 343977 [details] [diff] [review] Do it like the sinks
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 apprval126.96.36.199 (not .4) if I could...
Created attachment 344162 [details] [diff] [review] 1.8 branch patch Again, I'd probably try for the next branch release not this very next one.
Created attachment 344163 [details] [diff] [review] 1.8 branch non-ugly patch
Comment on attachment 343977 [details] [diff] [review] Do it like the sinks approved for 188.8.131.52 and 184.108.40.206, a=dveditz for release-drivers (please watch tinderbox for the tree to open before landing)
Fixed on both branches.
verified fixed for 220.127.116.11 - using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:18.104.22.168pre) Gecko/2008120304 Firefox/22.214.171.124pre (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 126.96.36.199
also verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:188.8.131.52pre) 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 184.108.40.206
Comment on attachment 344163 [details] [diff] [review] 1.8 branch non-ugly patch a=asac for 1.8.0