Last Comment Bug 675490 - Crash [@ nsFrameList::RemoveFrame] on printing/print preview
: Crash [@ nsFrameList::RemoveFrame] on printing/print preview
Status: RESOLVED FIXED
: crash, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Windows 7
: -- critical (vote)
: mozilla8
Assigned To: Bernd
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-31 02:38 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2011-08-14 05:13 PDT (History)
4 users (show)
khuey: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (429 bytes, application/xhtml+xml)
2011-07-31 02:38 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
reduced testcase (433 bytes, application/xhtml+xml)
2011-07-31 11:50 PDT, Bernd
no flags Details
patch (2.14 KB, patch)
2011-08-04 12:01 PDT, Bernd
no flags Details | Diff | Splinter Review
patch (5.00 KB, patch)
2011-08-06 13:07 PDT, Bernd
bzbarsky: review+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2011-07-31 02:38:10 PDT
Created attachment 549638 [details]
testcase

See testcase, which crashes current trunk build on printing/print preview.

Previous bug, bug 642088.

https://crash-stats.mozilla.com/report/index/bp-8f6eb718-2f41-4018-8ef5-50bf72110730
0 	xul.dll 	nsFrameList::RemoveFrame 	layout/generic/nsFrameList.cpp:129
1 	xul.dll 	nsTableFrame::PushChildren 	layout/tables/nsTableFrame.cpp:1958
2 	xul.dll 	nsTableFrame::ReflowChildren 	
3 	xul.dll 	nsTableFrame::ReflowTable 	layout/tables/nsTableFrame.cpp:1900
4 	xul.dll 	nsTableFrame::Reflow 	layout/tables/nsTableFrame.cpp:1804
5 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:959
6 	xul.dll 	nsContainerFrame::StealOverflowFrames 	layout/generic/nsContainerFrame.h:643
7 	xul.dll 	PL_DHashTableOperate 	obj-firefox/xpcom/build/pldhash.c:625
Comment 1 Bernd 2011-07-31 03:22:14 PDT
I will look into it once my debug builds compiles...
Comment 2 Bernd 2011-07-31 10:53:28 PDT
I see it in my debug build.
Comment 3 Bernd 2011-07-31 11:50:13 PDT
Created attachment 549676 [details]
reduced testcase

the structure now  avoids table pseudo frame creation.
Its important that thead is not the first child of the table
Comment 4 Bernd 2011-08-03 22:07:15 PDT
what happens here is that on the first page

tbody splits at the available space and creates a next inflow
the table puts the nif on its overflow list.

the table height computation reports a slightly larger desired size (2 px) than available so the table gets marked as truncated

and gets pushed entirely on to second page

then the same table frame is reflowed and gets back the frame from the overflow list

now the mFrames are

tfoot > tbody > thead > tbody_nif 

where > stands for next sibling relation ship
Comment 5 Bernd 2011-08-04 12:01:48 PDT
Created attachment 550779 [details] [diff] [review]
patch

this fixes the crash but does not fix the problem with the tfoots that are not placed due to size constraints.
Comment 6 Bernd 2011-08-06 13:07:27 PDT
Created attachment 551274 [details] [diff] [review]
patch

The problem comes here from the fact that a repeatable tfoot can't be placed. If it is not pushed it will cause in CalcDesiredHeight a increase by one cellspacing and misplaced tfoot on the page where it does not fit.
Comment 7 Alice0775 White 2011-08-06 20:59:12 PDT
This also crash Aurora7.0a2
http://hg.mozilla.org/releases/mozilla-aurora/rev/bd419b4cdaea
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a2) Gecko/20110806 Firefox/7.0a2 ID:20110806042002

bp-7e67d10e-da21-4eae-b882-641902110806
Comment 8 Bernd 2011-08-07 05:11:27 PDT
Comment on attachment 551274 [details] [diff] [review]
patch

Boris, we compute the repeatability of tfoot as a quarter of the page size, however we are not guaranteed that the tfoot will fit. The patch here says if it does not fit then we remove the repeatability and push the tfoot to the next inflow. Otherwise tfoot will end as a not placed frame at 0,0 relative to the table and spoil nsTableFrame::CalcDesiredHeight at http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableFrame.cpp#2977 by addinga 0 height and one cellspacing in excess. 

I will push this over to try, once my commit is reenabled  (bug  677040)
Comment 9 Boris Zbarsky [:bz] 2011-08-08 12:05:02 PDT
Comment on attachment 551274 [details] [diff] [review]
patch

r=me
Comment 10 Bernd 2011-08-09 12:33:58 PDT
http://tbpl.mozilla.org/?tree=Try&rev=1de37cd663c7 shows a consistent failure pattern in crashtests

REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/layout/tables/crashtests/576890-1.html | assertion count 2 is more than expected 0 assertions
REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/layout/tables/crashtests/576890-2.html | assertion count 12 is more than expected 0 assertions
REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/layout/tables/crashtests/576890-3.html | assertion count 2 is more than expected 0 assertions

==> more work required
Comment 11 Bernd 2011-08-09 12:41:23 PDT
more work is means bug 610245
Comment 12 Bernd 2011-08-13 10:31:02 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/6f4b90a6c292 pushed together with the patch for bug 678447, when pushing this to aurora or beta both patches are required otherwise crash tests will go orange.
Comment 13 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-08-14 05:13:32 PDT
http://hg.mozilla.org/mozilla-central/rev/6f4b90a6c292

Note You need to log in before you can comment on or make changes to this bug.