Closed Bug 563009 Opened 14 years ago Closed 14 years ago

Print preview crashes Firefox [@ nsIFrame::SetNextSibling(nsIFrame*) ]

Categories

(Core :: Layout: Tables, defect, P2)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla2.0b4
Tracking Status
blocking2.0 --- final+
status1.9.2 --- wanted

People

(Reporter: giovanni, Assigned: dbaron)

References

Details

(Keywords: assertion, crash, testcase, Whiteboard: [sg:critical?][fixed by patches in 563584][critsmash:patch])

Crash Data

Attachments

(4 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 FirePHP/0.4
Build Identifier: firefox 3.6.3

Go to the reported page, go to file, print preview.

It systematically hangs the browser. I bet it has something to do with the floated inline-blocks.


Reproducible: Always

Steps to Reproduce:
1. Go to the reported page, go to file, print preview.
2. Watch the browser hang forever.
crashes on trunk Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100430 ID:20100430072714 with

Signature	nsIFrame::SetNextSibling(nsIFrame*)
UUID	3c61644c-4d4e-4fc2-8053-e5d192100430
Time 	2010-04-30 14:43:17.243040
Uptime	11980
Last Crash	395456 seconds before submission
Product	Firefox
Version	3.7a5pre
Build ID	20100430072714
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 Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x20
User Comments	
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 	nsTableFrame::PushChildren(nsAutoTPtrArray<nsTableRowGroupFrame,8> const&,int) 	layout/tables/nsTableFrame.cpp:1936
3 	xul.dll 	nsTableFrame::ReflowChildren(nsTableReflowState&,unsigned int&,nsIFrame*&,nsRect&) 	
4 	xul.dll 	nsTableFrame::ReflowTable(nsHTMLReflowMetrics&,nsHTMLReflowState const&,int,nsIFrame*&,unsigned int&) 	layout/tables/nsTableFrame.cpp:1878
5 	xul.dll 	nsTableFrame::Reflow(nsPresContext*,nsHTMLReflowMetrics&,nsHTMLReflowState const&,unsigned int&) 	layout/tables/nsTableFrame.cpp:1782
6 	xul.dll 	nsContainerFrame::ReflowChild(nsIFrame*,nsPresContext*,nsHTMLReflowMetrics&,nsHTMLReflowState const&,int,int,unsigned int,unsigned int&,nsOverflowContinuationTracker*) 	layout/generic/nsContainerFrame.cpp:738
7 	xul.dll 	nsTableOuterFrame::Reflow(nsPresContext*,nsHTMLReflowMetrics&,nsHTMLReflowState const&,unsigned int&) 	layout/tables/nsTableOuterFrame.cpp:1087
8 		@0x12d87e7f

checking crashstats this is low volume on 1.9.2/.3.
Status: UNCONFIRMED → NEW
Component: General → Layout: Tables
Ever confirmed: true
Keywords: crash, testcase
QA Contact: general → layout.tables
Summary: Print preview crashes Firefox → Print preview crashes Firefox [@ nsIFrame::SetNextSibling(nsIFrame*) ]
Version: unspecified → 1.9.2 Branch
Affects my mozilla-central debug build on x86 Linux, too. OS --> All, version --> Trunk.

When opening testcase in print preview, I get 157 lines of these assertions:
{
###!!! ASSERTION: Broken frame linkage: 'prevSibling && prevSibling->GetNextSibling() == aFrame', file ../../../mozilla/layout/generic/nsFrameList.cpp, line 132
###!!! ASSERTION: How did that happen?: '!mContent->GetPrimaryFrame()', file ../../../mozilla/layout/base/nsCSSFrameConstructor.cpp, line 11804
###!!! ASSERTION: Losing track of existing primary frame: '!aFrame || !mPrimaryFrame || aFrame == mPrimaryFrame', file ../../dist/include/nsIContent.h, line 893
###!!! ASSERTION: Why did we get called?: '!aChildContent->GetPrimaryFrame() || aChildContent->GetPrimaryFrame()->GetContent() != aChildContent', file ../../../mozilla/layout/base/nsCSSFrameConstructor.cpp,
}

...followed by a single instance of this assertion:
###!!! ASSERTION: Creating a circular frame list, this is very bad.: 'this != aNextSibling', file ../../../mozilla/layout/base/../generic/nsIFrame.h, line 951

and then a crash on the line after that one:
>     if (mNextSibling && mNextSibling->GetPrevSibling() == this) {

We crash in that context because mNextSibling is inaccessible, because it's member data on |this|, and |this| is null.

Flagging as potentially security-sensitive.
Group: core-security
Keywords: assertion
OS: Windows Vista → All
Whiteboard: [sg:critical?]
Version: 1.9.2 Branch → Trunk
Attached file reporter's testcase (obsolete) (deleted) —
Here's a copy of the testcase linked to in the URL field. (I've verified that I crash when loading this saved copy.)
(er, s/loading/print-previewing)
Existing testcase is a bit large -- a reduced testcase would be very helpful here. [adding qawanted keyword]
Keywords: qawanted
Isn't this the same as bug 554376?
Looks like it might be, based on crash location & the involvement of tables & print-preview.

Note though that bug 554376's testcase only triggers the "Broken frame linkage" and "Creating a circular frame list" assertions, in my build.  It doesn't trigger the other 3 assertions mentioned in comment 2.  And actually, this bug's testcase triggers instances of all three other assertions **before** it ever hits the "Broken frame linkage" and "Creating a circular frame list" ones.

For now, marking as dependent, but not necessarily a dupe.
Depends on: 554376
FWIW, despite comment 0 having a 3.6.3 user-agent string, I can't reproduce this in my 3.6.x nightly build:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.5pre) Gecko/20100430 Namoroka/3.6.5pre
That is: no hang or crash, and print-preview looks fine, in the my 3.6.5pre nightly build.

vs. an immediate crash on print-preview, in a m-c nightly:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100429 Minefield/3.7a5pre
http://crash-stats.mozilla.com/report/index/bp-d6f8f886-fb52-4d71-8f9d-fb0b02100430
blocking2.0: --- → ?
>Isn't this the same as bug 554376?
No,  bug 554376 is fixed in my tree but this still crashes my debug build.
Attached file reduced testcase
Attached file reflow log
the nextinflow logic is seriously broken  on this , on the second page the table has already a nextinflow on its initial reflow

tblO 06E8E950 Reflow a=15600,59614 c=15600,UC dirty v-resize nif=06E915E8 cnt=10566
Hi guys, i-m
Hi guys, now that you have a reduced testcase, could you please delete the original reported page from the attachments? It contains some personal information.
I mean, it's not that confidential, but can you just delete it? Thanks.
Comment on attachment 442846 [details]
reporter's testcase

Bugzilla doesn't let us easily delete attachments, but I've marked that attachment as private, which is the next best thing. (That means that only the people in the Mozilla Security Group can see it.)
Attachment #442846 - Attachment is private: true
Keywords: qawanted
Hmm, an incarnation of not seeing  the wood for the trees, the table on the second page is not the nextinflow of the table from the first page bug the original table, which explains the cycling.
Attached file more reduced testcase
Attached file float cache assert
Removing the top most div triggers a strange assert
###!!! ASSERTION: float manager state should match saved state: 'aState->mX == m
X && aState->mY == mY && aState->mFloatInfoCount == mFloats.Length()', file d:\m
oz_src\src\layout\generic\nsFloatManager.h, line 263
The assert is from bug 25888, This might be a float cache issue.
Depends on: 563584
The assert can be triggered purely by <div>s, I filed bug 563584 for this. For me this looks like a block layout issue rather than a table problem. The table probably turns the problem here into disaster as it creates the repeated footer twice for the same table that the block layout feeds into the second page.
Whiteboard: [sg:critical?] → [sg:critical?][depends on 563584]
Whiteboard: [sg:critical?][depends on 563584] → [sg:critical?][depends on 563584][critsmash:investigating]
Note: The middle three asserts in comment 2 (all except "Broken frame linkage" and "Creating a circular frame list") are also mentioned in bug 563837, which has a patch that looks to be landing soon.  That landing may end up fixing those assertions here, too.
Assigning to dbaron since he's working on bug 563584 which will hopefully fix this.
(In reply to comment #21)
> Note: The middle three asserts in comment 2 (all except "Broken frame linkage"
> and "Creating a circular frame list") are also mentioned in bug 563837, which
> has a patch that looks to be landing soon.  That landing may end up fixing
> those assertions here, too.

Just to follow up on this comment -- bug 563837 has now landed, and it seems to have fixed all but those two assertions on this bug's original testcase, as I suspected it would.
The content of attachment 442846 [details] has been deleted by
    Reed Loden [:reed] <reed@reedloden.com>
who provided the following reason:

Removal requested by attachment submitter for reason specified in comment #14.

The token used to delete this attachment was generated at 2010-05-20 13:32:30 PDT.
(In reply to comment #14)
> Hi guys, now that you have a reduced testcase, could you please delete the
> original reported page from the attachments? It contains some personal
> information.
> I mean, it's not that confidential, but can you just delete it? Thanks.

Done. Thanks for the assistance!
Attachment #442846 - Attachment is private: false
Possible sg:crit, blocking until we have some sort of resolution.
blocking2.0: ? → final+
Looks like a full fix to bug 563584 is going to be Hard, perhaps we should go for some kind of mitigation that stops us from crashing when bug 563584 occurs?
Dbaron, have an estimate of when you can start looking into this?
David says he's made good progress on bug 563584 and we should probably have a fix there shortly.

David, how does your patch affect this bug?
Depends on: 570169
In bug 563584, David says he has patches that fix this bug.
Whiteboard: [sg:critical?][depends on 563584][critsmash:investigating] → [sg:critical?][depends on 563584][critsmash:patch]
If we want to fix this on 3.6.x as well, will we need a different strategy there?
No longer depends on: 570169
Whiteboard: [sg:critical?][depends on 563584][critsmash:patch] → [sg:critical?][fixed by patches in 563584][critsmash:patch]
Progress towards landing the patches in bug 563584 is stopped while dbaron works on some Firefox 4 beta features.
(In reply to comment #33)
> Progress towards landing the patches in bug 563584 is stopped while dbaron
> works on some Firefox 4 beta features.

Will he be done after beta 1?
I noticed bug 563584 is blocking final+, but since it's a large set of patches I'd be more comfotable with blocking last-beta+.
(In reply to comment #34)
> (In reply to comment #33)
> > Progress towards landing the patches in bug 563584 is stopped while dbaron
> > works on some Firefox 4 beta features.
> 
> Will he be done after beta 1?

I guess he's not done yet.
Fixed by landing of bug 563584.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Crash Signature: [@ nsIFrame::SetNextSibling(nsIFrame*) ]
Flags: in-testsuite?
Landed the crashtests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/495aac9f895d
Group: core-security
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.