Closed Bug 406137 Opened 12 years ago Closed 12 years ago
Crash loading www
.redhat .com [@ ns IFrame::Get Style Display] [@ ns IFrame::Get Position]
With trunk CVS, I crash loading www.redhat.com. The attached testcase crashes during load unless the image is in the cache (in which case shift-reload will still crash). www.redhat.com crashes in nsIFrame::GetStyleDisplay and the testcase crashes in nsIFrame::GetPosition. Both are crashing due to floatFrame being NULL in nsBlockReflowState::RecoverFloats. A build from this morning (9am PST) doesn't crash and the crashing CVS build is from 10pm PST, so this is from bug 320378.
This is the stacktrace from the testcase.
The problem is that we can have dangling frame pointers in a line's float cache. If we skip reflowing the line due to willReflowAgain, then in the skip path we do RecoverStateFrom on the line, which walks the float cache, which crashes on the dangling pointer. This patch fixes the issue by forcing reflow of any dirty lines with floats even if willReflowAgain is true. That reflow might give bogus positions, but that's OK because we always dirty such a line after reflowing it. We also have to be sure not to RecoverStateFrom on a skipped line if it's a block; it might have descendant lines with float caches containing dangling pointers. This is OK because such a RecoverStateFrom call never moves frames, it just adds floats to the space manager, and it doesn't matter if the space manager is wrong during this doomed reflow. It's really nasty that the float cache can contain dangling pointers like this. The solution is to remove the float cache but I don't want to have to do that for 1.9.
David, this is a very nasty regression that will cause a lot of crashes on trunk. If you aren't able to review this today perhaps we should back out the original patch?
Comment on attachment 290857 [details] [diff] [review] fix r+sr=dbaron
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 406297
Crash Signature: [@ nsIFrame::GetStyleDisplay] [@ nsIFrame::GetPosition]
Crash Signature: [@ nsIFrame::GetStyleDisplay] [@ nsIFrame::GetPosition] → [@ nsIFrame::GetStyleDisplay] [@ nsIFrame::GetPosition]
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.