See upcoming testcase, which crashes Mozilla on load. It also crashes 1.8 branch builds, so it doesn't seem to be a regression. Talkback ID: TB22233772X ReparentFrame nsBlockFrame::HandleOverflowPlaceholdersOnPulledLine nsBlockFrame::Reflow
Created attachment 236243 [details] [diff] [review] Patch rev. 1 When the nsBlockReflowContext destroys float continuations due to the reflow being COMPLETE: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockReflowContext.cpp&rev=1.143&root=/cvsroot&mark=725,735#703 in some cases [the placeholders for] these continuations does not live in the frame tree (including the overflowOutOfFlowsProperty list) but in nsBlockReflowState::mOverflowPlaceholders which will then contain stale placeholders and the crash occurs if the rest of the reflow looks at this data, e.g. nsBlockFrame::HandleOverflowPlaceholdersOnPulledLine. There is this comment: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&root=/cvsroot&mark=5601-5604#5568 which I guess is correct in a way, provided that no-one is using that part of the block reflow state after that point. After reading through the code and trying various ways to fix this, I get the impression that there are several ways this can happen though. This patch fixes the crash for the attached testcases, but I'm unsure if it's the correct fix. I think there is at least one path through nsLineLayout::ReflowFrame where a similar crash might happen if we're unlucky. http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineLayout.cpp&rev=3.238&root=/cvsroot&mark=1155,1163#1126
I just asked Robert about a similiar thing in tables, isn't it wrong to mark a frame complete if nsBlockReflowState::mOverflowPlaceholders is not empty?
Blocks are more complicated, unfortunately. We mark the block complete when the regular in-flow content has all been reflowed. But there could still be float content that needs to be reflowed, and hence overflowing placeholders.
I think the basic idea of removing these destroyed floats' placeholders from blockReflowState's mOverflowPlaceholders, when a float completes and destroys its next-in-flows, is the right idea. I don't see how this patch does that though
Mats, are you still on this or do you want me to take it?
You're most welcome to :-)
Assignee: mats.palmgren → roc
FWIW, I don't see this crahs on the trunk (linux) any more, both testcases load w/o crashing here.
In a 2006-12-07 build, it still crashes, not anymore in a 2006-12-08 build (it hangs there, in a 2006-12-30 build, it doesn't even hang anymore). Marking worksforme?
Roc, you still have work to do here, even if this doesn't appear to crash any more?
No, I think I fixed this in bug 359371. Perhaps the reflow branch stopped the crash and bug 359371 stopped the hang.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
This still happens on branch (since there's no reflow branch). Is it worth fixing there?
Did you check? bug 359371 is fixed on branch.
The branch crash appears to be a null dereference. Is there an exploitable flaw here?
Whiteboard: [sg:nse null deref?]
Whiteboard: [sg:nse null deref?] → [sg:nse null deref?] fixed by bug 359371 on trunk, but not branch?
You need to log in before you can comment on or make changes to this bug.