Closed Bug 570160 Opened 10 years ago Closed 10 years ago

[regression] crash when printing or in print preview and A4 as page format [@ nsIFrame::SetNextSibling]


(Core :: Layout: Floats, defect, critical)

Not set



Tracking Status
status1.9.2 --- unaffected
status1.9.1 --- unaffected


(Reporter: bugzilla.i.sekler, Assigned: mats)




(Keywords: crash, regression, testcase)

Crash Data


(3 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100604 Firefox/3.7a5pre
Build Identifier: 

Firefox crashes when printing or in print preview for the $URL if page format is not Letter (tested with A4).

Good: 2009-10-02, SourceStamp=a80414164888

Bad: 2009-10-03, SourceStamp=0a7dd88dbe67

Regression window:

Reproducible: Always

Steps to Reproduce:
1. Set page format to A4
2. Load <>
3. Print or print preview the page.
Actual Results:  

The regression window is identical with <>, so this bug might be a dupe of Bug 553972, which should be reopened in this case.

The crash report whith an old official nightly while searching for the first bad build: <>

Wasn't able to make a minimal testcase yet.
crashes on windows xp too with 1.9.3 20100604040314

Signature	nsIFrame::SetNextSibling(nsIFrame*)
UUID	9d5d052b-8df2-4dd9-9344-c75882100604
Time 	2010-06-04 10:54:10.198115
Uptime	39
Last Crash	47257 seconds before submission
Product	Firefox
Version	3.7a5pre
Build ID	20100604040314
Branch	1.9.3
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	GenuineIntel family 15 model 2 stepping 9
Crash Address	0x20
User Comments	Bug 570160

Crashing Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	nsIFrame::SetNextSibling(nsIFrame*) 	layout/generic/nsIFrame.h:952
1 	xul.dll 	nsFrameList::RemoveFrame(nsIFrame*) 	layout/generic/nsFrameList.cpp:133
2 	xul.dll 	nsBlockFrame::CollectFloats(nsIFrame*,nsFrameList&,int,int) 	layout/generic/nsBlockFrame.cpp:6645
3 	xul.dll 	nsBlockFrame::CollectFloats(nsIFrame*,nsFrameList&,int,int) 	layout/generic/nsBlockFrame.cpp:6651
4 	xul.dll 	nsBlockFrame::PushLines(nsBlockReflowState&,nsLineList_iterator) 	layout/generic/nsBlockFrame.cpp:4273
5 	xul.dll 	nsBlockFrame::PlaceLine(nsBlockReflowState&,nsLineLayout&,nsLineList_iterator,nsFloatManager::SavedState*,nsRect&,int&,int*) 	
6 	xul.dll 	nsAStreamCopier::Process() 	xpcom/io/nsStreamUtils.cpp:380
7 	nspr4.dll 	_PR_MD_UNLOCK 	nsprpub/pr/src/md/windows/w95cv.c:344
8 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
9 	xul.dll 	nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&,nsLineLayout&,nsLineList_iterator,nsIFrame*,LineReflowStatus*) 	layout/generic/nsBlockFrame.cpp:3722
10 	xul.dll 	nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&,nsLineLayout&,nsLineList_iterator,nsFlowAreaRect&,int&,nsFloatManager::SavedState*,int*,LineReflowStatus*,int) 	layout/generic/nsBlockFrame.cpp:3653
11 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
12 		@0x12b967
Component: General → Layout: Tables
Depends on: 563009
Keywords: crash, regression
OS: Linux → All
QA Contact: general → layout.tables
Version: unspecified → Trunk
Attached file framedump
###!!! ASSERTION: why CreateContinuingFrame for a non-splittable frame?: 'aFrame->GetSplittableType() != NS_FRAME_NOT_SPLITTABLE', file c:/moz/hg2/layout/base/nsCSSFrameConstructor.cpp, line 8512
###!!! ASSERTION: OOF must be first continuation: '!aFrame || !aFrame->GetPrevContinuation()', file c:\moz\hg2\layout\generic\nsPlaceholderFrame.h, line 115
###!!! ASSERTION: float manager state should match saved state: 'aState->mX == mX && aState->mY == mY && aState->mFloatInfoCount == mFloats.Length()', file c:\moz\hg2\layout\generic\nsFloatManager.h, line 263
###!!! ASSERTION: available space should never grow: 'aOldAvailableSpace.x <= aNewAvailableSpace.x && aOldAvailableSpace.XMost() >= aNewAvailableSpace.XMost()', file c:/moz/hg2/layout/generic/nsBlockFrame.cpp, line 828
###!!! ASSERTION: Broken frame linkage: 'prevSibling && prevSibling->GetNextSibling() == aFrame', file c:/moz/hg2/layout/generic/nsFrameList.cpp, line 132
###!!! ASSERTION: Creating a circular frame list, this is very bad.: 'this != aNextSibling', file c:\moz\hg2\layout\generic\nsIFrame.h, line 951

The first assertion is caused by trying to create a continuation for
a placeholder frame...
Assignee: nobody → matspal
Ever confirmed: true
Attached file Testcase
Patch coming up...
Blocks: 492627
Component: Layout: Tables → Layout: Floats
Keywords: testcase
QA Contact: layout.tables → layout.floats
Hardware: x86 → All
Summary: [regression] crash when printing or in print preview and A4 as page format → [regression] crash when printing or in print preview and A4 as page format [@ nsIFrame::SetNextSibling]
Attached patch Patch rev. 1 (obsolete) — Splinter Review
Call SplitFloat() for incomplete placeholders also in the break-after case.

It would be fairly easy to re-arrange the code to share the 
SplitFloat/CreateNextInFlow bits for the break-after and
!NS_FRAME_IS_FULLY_COMPLETE blocks.  Let me know if you'd prefer that
and I'll make a new patch.
Attachment #449530 - Flags: review?(fantasai.bugs)
Attached patch Patch rev. 1 (diff -w) (obsolete) — Splinter Review
Yes, I think that would be better.
Attachment #449530 - Flags: review?(fantasai.bugs) → review-
Attached patch Patch rev. 2 (obsolete) — Splinter Review
Rearranging the code, should be functionally the same as the last patch.
Attachment #449530 - Attachment is obsolete: true
Attachment #449531 - Attachment is obsolete: true
Attachment #454447 - Flags: review?(fantasai.bugs)
Comment on attachment 454447 [details] [diff] [review]
Patch rev. 2

Overall, looks good. I think, however, that this bit

+  PRBool isBreakAfter = NS_INLINE_IS_BREAK_AFTER(aStatus);
+  if (isBreakAfter ? NS_FRAME_IS_NOT_COMPLETE(aStatus)
+                   : !NS_FRAME_IS_FULLY_COMPLETE(aStatus)) {
+  [...]
+  if (isBreakAfter) {

should be
+  if (!NS_FRAME_IS_FULLY_COMPLETE(aStatus)) {
+  [...]
+  if (NS_INLINE_IS_BREAK_AFTER(aStatus)) {

r+ with that change, unless you disagree
Attachment #454447 - Flags: review?(fantasai.bugs) → review+
Ok, I thought about that too, but I kept the old logic to minimize risk.
(the old logic doesn't make a continuation for
Are you saying that combination cannot occur or is the old logic
simply broken and it should have made a continuation in that case too?
(I'll add an assertion if it shouldn't occur)
I believe it should have made a continuation in that case, too. I suspect I didn't update it to handle overflow containers in that instance because there was no code to handle placeholders, so it seemed there were nothing to handle.
Ok, thanks for clarifying.
Attached patch Patch rev. 3Splinter Review
Updated per review comments.  I'll land this when the tree reopens...
Attachment #454447 - Attachment is obsolete: true
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
Blocks: 574958
Crash Signature: [@ nsIFrame::SetNextSibling]
You need to log in before you can comment on or make changes to this bug.