Crash [@ nsFrameList::DestroyFrames] on reload with -moz-column-count, -moz-inline-block and blockquote::first-letter

VERIFIED FIXED

Status

()

defect
--
critical
VERIFIED FIXED
13 years ago
8 years ago

People

(Reporter: martijn.martijn, Assigned: mats)

Tracking

(5 keywords)

Dependency tree / graph
Bug Flags:
blocking1.7.14 -
blocking-aviary1.0.9 -
blocking1.8.1 +
blocking1.8.0.7 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?], crash signature)

Attachments

(5 attachments)

Reporter

Description

13 years ago
See upcoming testcase, which crashes Mozilla on reload.
It crashes with current trunk builds, with Firefox1.5.0.6, but not with Mozilla1.7.13.


Talkback ID: TB22162189Z
nsFrameList::DestroyFrames   nsLineBox::DeleteLineList   nsFrameList::DestroyFrames   nsLineBox::DeleteLineList   nsLineBox::DeleteLineList   nsFrameList::DestroyFrames   CanvasFrame::Destroy   nsFrameList::DestroyFrames   nsFrameList::DestroyFrames   nsFrameManager::Destroy   DocumentViewerImpl::Destroy   DocumentViewerImpl::Show   nsPresContext::EnsureVisible   PresShell::UnsuppressAndInvalidate   PresShell::UnsuppressPainting
Reporter

Comment 3

13 years ago
Posted file original testcase
Reporter

Comment 4

13 years ago
Doesn't crash with a 2004-05-26 build, crashes directly with a 2004-10-10 build.
Assignee

Comment 5

13 years ago
###!!! ASSERTION: Out of flow frame doesn't have the expected parent: 'outOfFlowFrame->GetParent() == this', file nsBlockFrame.cpp, line 7159
###!!! ASSERTION: Parent not consistent with exepectations: 'aOldParent == aFrame->GetParent()', file nsBlockFrame.cpp, line 577
###!!! ASSERTION: Float frame has wrong parent: 'floatFrame->GetParent() == mBlock', file nsBlockReflowState.cpp, line 840
WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file nsBlockFrame.cpp, line 7207
###!!! ASSERTION: Float frame has wrong parent: 'floatFrame->GetParent() == mBlock', file nsBlockReflowState.cpp, line 840
WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file nsBlockFrame.cpp, line 7207
###!!! ASSERTION: Float frame has wrong parent: 'floatFrame->GetParent() == mBlock', file nsBlockReflowState.cpp, line 840
WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file nsBlockFrame.cpp, line 7207
###!!! ASSERTION: Float frame has wrong parent: 'floatFrame->GetParent() == mBlock', file nsBlockReflowState.cpp, line 840
###!!! ASSERTION: Float frame has wrong parent: 'floatFrame->GetParent() == mBlock', file nsBlockReflowState.cpp, line 840
WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file nsBlockFrame.cpp, line 7207
###!!! ASSERTION: Float frame has wrong parent: 'floatFrame->GetParent() == mBlock', file nsBlockReflowState.cpp, line 840
WARNING: nsBlockFrame::CheckFloats: Explicit float list is out of sync with float cache: file nsBlockFrame.cpp, line 7207
OS: Windows XP → All
Assignee

Comment 6

13 years ago
Posted patch Patch ASplinter Review
<blockquote style="display: -moz-inline-block;">

GetStyleDisplay()->IsBlockLevel() is false.
IsFloatContainingBlock() is true (it's a nsBlockFrame).

This fixes it for me, although I'm a bit unsure what I have in my
tree at the moment ;-)  I'll do some more analysis tomorrow...
Fwiw, I think this looks like the right change...  IsBlockLevel() is more about display-role at the moment (in CSS3 Box speak), whereas we care about display-model.
Flags: blocking1.9?
Flags: blocking1.8.1?
Flags: blocking1.8.0.7?

Comment 8

13 years ago
Putting on the 1.8.1/FF2 radar...
Flags: blocking1.8.1? → blocking1.8.1+
Assignee

Updated

13 years ago
Assignee: nobody → mats.palmgren
If 1.8.1 gets this we want it in 1.8.0.7 also (unless it misses, in which case 1.8.0.8)
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Assignee

Updated

13 years ago
Attachment #234124 - Attachment description: wip → Patch A
Assignee

Comment 11

13 years ago
Patch A: checked in to trunk at 2006-08-21 20:04 PDT.
(will ask for branch approvals in a couple of days)

-> FIXED
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee

Comment 12

13 years ago
Posted patch Patch BSplinter Review
I scanned layout/generic for similar code and I think I've found one more.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.788&root=/cvsroot&mark=4877#4854
What do you think?
Whiteboard: needs approval requests
Assignee

Updated

13 years ago
Attachment #234124 - Flags: approval1.8.1?
Attachment #234124 - Flags: approval1.8.0.7?
Assignee

Comment 14

13 years ago
"Patch B" checked in to trunk at 2006-08-24 21:17 PDT.

Comment 15

13 years ago
Comment on attachment 234124 [details] [diff] [review]
Patch A

a=schrep for drivers
Attachment #234124 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 234124 [details] [diff] [review]
Patch A

approved for 1.8.0 branch, a=dveditz for drivers

What about Patch B for the 1.8/1.8.0 branch?
Attachment #234124 - Flags: approval1.8.0.7? → approval1.8.0.7+
Assignee

Comment 17

13 years ago
Patch A: checked in to MOZILLA_1_8_BRANCH 2006-08-25 14:47 PDT.
Patch A: checked in to MOZILLA_1_8_0_BRANCH at 2006-08-25 14:56 PDT.

On 1.8/1.8.0 branches I get (SeaMonkey debug Linux/gtk2):
 testcase:           no assertions, no crash
 inbetween testcase: several nasty assertions, but no crash
 original testcase:  several nasty assertions and crash (unrelated to this
                     code, I got the same crash before the change)

on trunk I get:
 testcase:           no assertions, no crash
 inbetween testcase: 1 assertion and 1 warning, but no crash
                     ASSERTION: bad float placement: 'NS_SUCCEEDED(rv)',
                      file nsBlockReflowState.cpp, line 1019
                     WARNING: aFrame is already associated with a region:
                      file nsSpaceManager.cpp, line 809
 original testcase:  no assertions, no crash

I tried "Patch B" on branches as well, with the same test results.
Assignee

Comment 18

13 years ago
(In reply to comment #16)
> What about Patch B for the 1.8/1.8.0 branch?

I haven't seen any cases where this problem is triggered; I'm guessing the risk
is about the same as Patch A; I'm guessing it could prevent crashes in fuzz
tests similar to Patch A... so, I'm leaning towards yes...

roc, what do you think?
Assignee

Comment 20

13 years ago
(I filed bug 350267 to followup the branch crash in comment 17, the trunk
assertion+warning is covered by bug 321896 I think)
Depends on: 350267
Assignee

Updated

13 years ago
Attachment #234906 - Flags: approval1.8.1?
Attachment #234906 - Flags: approval1.8.0.7?
Comment on attachment 234906 [details] [diff] [review]
Patch B

a=dveditz for 1.8.0 branch.
Attachment #234906 - Flags: approval1.8.0.7? → approval1.8.0.7+
Assignee

Comment 22

13 years ago
Patch B landed on MOZILLA_1_8_0_BRANCH at 2006-08-28 00:49 PDT.
verified on bon echo build from 20060829 on Windows and Mac 1.5.0.7 build from 20060829

verified trunk per comment #17

note still seeing crash for bug 350267 with the original testcase
Status: RESOLVED → VERIFIED
Flags: blocking1.7.14-
Flags: blocking-aviary1.0.9-
Whiteboard: needs approval requests → [sg:critical?]
Assignee

Comment 24

13 years ago
Although this bug is marked "verified1.8.1" there is still a second patch
that requests "approval1.8.1". It has already landed on trunk and 1.8.0.7.
Patch B is not needed to fix the crash in this bug, it's just to cover a
potential similar crash at a different place (see comment 18/19).
There are no known regressions, bug 350267 is just a followup on an unrelated
crash.

Comment 25

13 years ago
Comment on attachment 234906 [details] [diff] [review]
Patch B

a=schrep for drivers.
Attachment #234906 - Flags: approval1.8.1? → approval1.8.1+
Assignee

Comment 26

13 years ago
Patch B checked in to MOZILLA_1_8_BRANCH at 2006-09-11 17:46 PDT.
Flags: blocking1.9?
Group: security

Comment 27

10 years ago
crash test landed
http://hg.mozilla.org/mozilla-central/rev/fd6a6e73be23
Flags: in-testsuite+
Crash Signature: [@ nsFrameList::DestroyFrames]
You need to log in before you can comment on or make changes to this bug.