Closed Bug 389744 Opened 17 years ago Closed 17 years ago

Crash [@ nsPropertyTable::PropertyList::Equals] when zooming in a dd with overflow:scroll; position:fixed

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9beta1

People

(Reporter: martijn.martijn, Assigned: sharparrow1)

References

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(2 files)

http://crash-stats.mozilla.com/report/index/18eec46b-3bc2-11dc-9141-001a4bd43ed6
0  	nsPropertyTable::PropertyList::Equals(unsigned short,nsIAtom *)
1 	nsPropertyTable::GetPropertyListFor(unsigned short,nsIAtom *)
2 	nsPropertyTable::GetPropertyInternal(nsPropertyOwner,unsigned short,nsIAtom *,int,unsigned int *)
3 	nsPropertyTable::GetProperty(nsPropertyOwner,nsIAtom *,unsigned int *)
4 	nsContainerFrame::GetOverflowFrames(nsPresContext *,int)
5 	nsContainerFrame::GetFirstChild(nsIAtom *)
6 	nsFrameManager::CaptureFrameState(nsIFrame *,nsILayoutHistoryState *)
7 	nsFrameManager::CaptureFrameState(nsIFrame *,nsILayoutHistoryState *)
8 	nsFrameManager::CaptureFrameState(nsIFrame *,nsILayoutHistoryState *)
9 	nsFrameManager::CaptureFrameState(nsIFrame *,nsILayoutHistoryState *)
Fantasai, this is probably yours
Not mine. I tried backing out my changes and it still crashes.
I can tell you that it crashes on a ButtonBoxFrame(scrollbarbutton), though.

Capturing frame state for Canvas(html)(-1) frame 0x8e330bc
Checking child list ...done
Checking child list Overflow-list...done
Checking child list Overflow-list...done
Checking child list Fixed-list...done
Capturing frame state for TextBox(label)(-1)[value=] frame 0x8ee3dd8
Checking child list ...done
Capturing frame state for TextBox(label)(1)[value=…] frame 0xb3b8a574
Checking child list ...done
Capturing frame state for HTMLScroll(dd)(1) frame 0x92fa5b8
Checking child list ...done
Capturing frame state for ScrollbarFrame(scrollbar)(-1) frame 0x92fa7b4
Checking child list ...done
Capturing frame state for ButtonBoxFrame(scrollbarbutton)(-1) frame 0x92fa824
Checking child list ...done
Checking child list Overflow-list...
Program dist/bin/firefox-bin (pid = 22268) received signal 11.
Flags: blocking1.9?
Attached patch PatchSplinter Review
The issue here is that changing the text size is causing a reframe, which is an unusual case for this codepath.  I haven't quite figured out how changing the text size causes a reframe. However, this change is still correct by itself, and it fixes the crash.
Assignee: nobody → sharparrow1
Status: NEW → ASSIGNED
Attachment #277461 - Flags: review?(dbaron)
I tracked down the cause of the reframe: nsLayoutUtils::GetBeforeFrame is broken for scrollframes because it assumes that the content insertion frame (in this case, the scrolled frame) is the first child of the frame that gets passed in, which is incorrect for scrollframes.  It doesn't find the existing generated content frame, and assumes that one needs to be constructed because the scrollframe has a generated content style.  Therefore, a scrollframe with a ::before style unconditionally requests a frame reconstruct every time its style gets reresolved.

CCing bz because he wrote nsLayoutUtils::GetBeforeFrame.
So... this reframe will happen in general for anything that doesn't support ::before but is styled with it.  We really need a better way to handle pseudo-elements.  I do agree that we should fix GetBeforeFrame, though...

It looks like simply calling GetFirstChildFrame on aFrame->GetContentInsertionFrame() instead of aFrame should do the trick, right?  That change would make a lot of sense to me.
Flags: blocking1.9? → blocking1.9+
Blocks: 392335
Blocks: 394588
Blocks: 389770
Comment on attachment 277461 [details] [diff] [review]
Patch

r+sr+a1.9=dbaron, but please add a comment explaining that we can't destroy the old rule tree until after ProcessRestyledFrames because if there are any frame reconstructs in the change list, the frames to be reconstructed will still have pointers to the old rule tree.
Attachment #277461 - Flags: superreview+
Attachment #277461 - Flags: review?(dbaron)
Attachment #277461 - Flags: review+
Attachment #277461 - Flags: approval1.9+
I checked this in to the trunk for you since I wasn't sure whether you'd be able to before the freeze.  Sorry for taking so long to get to reviewing it...
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007100104 Minefield/3.0a9pre
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Crash Signature: [@ nsPropertyTable::PropertyList::Equals]
You need to log in before you can comment on or make changes to this bug.