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

VERIFIED FIXED

Status

()

Core
Layout
--
critical
VERIFIED FIXED
11 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Mats Palmgren (vacation - back in August))

Tracking

(5 keywords)

Trunk
x86
All
crash, regression, testcase, verified1.8.0.7, verified1.8.1
Points:
---
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

11 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 1

11 years ago
Created attachment 234041 [details]
testcase (crashes on reload)
(Reporter)

Comment 2

11 years ago
Created attachment 234042 [details]
inbetween testcase that freezes and eats memory
(Reporter)

Comment 3

11 years ago
Created attachment 234043 [details]
original testcase
(Reporter)

Comment 4

11 years ago
Doesn't crash with a 2004-05-26 build, crashes directly with a 2004-10-10 build.
###!!! 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
Created attachment 234124 [details] [diff] [review]
Patch A

<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

11 years ago
Putting on the 1.8.1/FF2 radar...
Flags: blocking1.8.1? → blocking1.8.1+
Comment on attachment 234124 [details] [diff] [review]
Patch A

I like
Attachment #234124 - Flags: superreview+
Attachment #234124 - Flags: review+
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+
Attachment #234124 - Attachment description: wip → Patch A
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
Last Resolved: 11 years ago
Resolution: --- → FIXED
Created attachment 234906 [details] [diff] [review]
Patch B

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?
Comment on attachment 234906 [details] [diff] [review]
Patch B

Agree
Attachment #234906 - Flags: superreview+
Attachment #234906 - Flags: review+
Whiteboard: needs approval requests
Attachment #234124 - Flags: approval1.8.1?
Attachment #234124 - Flags: approval1.8.0.7?
"Patch B" checked in to trunk at 2006-08-24 21:17 PDT.

Comment 15

11 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+
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.
Keywords: fixed1.8.0.7, fixed1.8.1
(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?
I think yes.
(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
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+
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
Keywords: fixed1.8.0.7, fixed1.8.1 → verified1.8.0.7, verified1.8.1
Flags: blocking1.7.14-
Flags: blocking-aviary1.0.9-
Whiteboard: needs approval requests → [sg:critical?]
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

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

a=schrep for drivers.
Attachment #234906 - Flags: approval1.8.1? → approval1.8.1+
Patch B checked in to MOZILLA_1_8_BRANCH at 2006-09-11 17:46 PDT.
Flags: blocking1.9?
Group: security

Comment 27

8 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.