683702 - Crash [@ nsFrame::DestroyFrom] with position: absolute; -moz-column-count: 2;

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build after 100ms.

I assume this is a regression (in the end, everything is a regression).
There is bug 673931, which might be the same, that would indicate this bug also occurs in Firefox 5, at least.

https://crash-stats.mozilla.com/report/index/bp-515fb476-2d33-4cf1-a809-c40602110831
0 	xul.dll 	nsFrame::DestroyFrom 	
1 	xul.dll 	nsBlockFrame::DestroyFrom 	layout/generic/nsBlockFrame.cpp:344
2 	xul.dll 	nsBlockFrame::DestroyFrom 	layout/generic/nsBlockFrame.cpp:344
3 	xul.dll 	nsBlockFrame::DestroyFrom 	layout/generic/nsBlockFrame.cpp:322
4 	xul.dll 	nsBlockFrame::DeleteNextInFlowChild 	
5 	xul.dll 	nsIFrame::Destroy 	layout/generic/nsIFrame.h:569
6 	xul.dll 	nsPlaceholderFrame::DestroyFrom 	layout/generic/nsPlaceholderFrame.cpp:169
7 	xul.dll 	nsBlockFrame::DestroyFrom 	layout/generic/nsBlockFrame.cpp:322
8 	xul.dll 	nsContainerFrame::DestroyFrom 	layout/generic/nsContainerFrame.cpp:270
9 	xul.dll 	nsContainerFrame::DestroyFrom 	layout/generic/nsContainerFrame.cpp:270
10 	xul.dll 	nsContainerFrame::DestroyFrom 	layout/generic/nsContainerFrame.cpp:270
11 	xul.dll 	nsContainerFrame::DestroyFrom 	layout/generic/nsContainerFrame.cpp:270
12 	xul.dll 	nsTableFrame::DestroyFrom 	layout/tables/nsTableFrame.cpp:269
13 	xul.dll 	nsContainerFrame::DestroyFrom 	layout/generic/nsContainerFrame.cpp:270
14 	xul.dll 	nsTableOuterFrame::DestroyFrom 	layout/tables/nsTableOuterFrame.cpp:218
15 	xul.dll 	nsIFrame::Destroy 	layout/generic/nsIFrame.h:569
16 	xul.dll 	nsBlockFrame::DoRemoveFrame 	layout/generic/nsBlockFrame.cpp:5442
17 	xul.dll 	nsBlockFrame::RemoveFrame 	layout/generic/nsBlockFrame.cpp:5011
18 	xul.dll 	nsCSSFrameConstructor::ContentRemoved 	layout/base/nsCSSFrameConstructor.cpp:7516
19 	xul.dll 	nsCSSFrameConstructor::RecreateFramesForContent 	layout/base/nsCSSFrameConstructor.cpp:9091
20 	xul.dll 	nsCSSFrameConstructor::WipeContainingBlock 	
etc..

Comment 1

[Mats Palmgren (inactive)]

Regression window using mozilla-central Linux64 builds:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e58e98a89827&tochange=e87454393401
which makes me suspect "c6e432ffd5e2	Daniel Holbert — Bug 679933: When a frame is destroyed, remove next special-sibling's pointer to it. r=roc"

Summary is similar to "Bug 682649 - Crash with rel&abs pos, -moz-column"

Comment 2

[Daniel Holbert [:dholbert]]

Yup, regression from bug 679933 (at least, it crashes while inside the code added there).

A bit odd, though.  Here's part of the code that bug added to nsFrame::Destroy:
>+++ b/layout/generic/nsFrame.cpp
>+  if (mState & NS_FRAME_IS_SPECIAL) {
>+    nsIFrame* nextSib = static_cast<nsIFrame*>
>+      (Properties().Get(nsIFrame::IBSplitSpecialSibling()));
>+    if (nextSib) {
>+      NS_WARN_IF_FALSE(this ==
>+         nextSib->Properties().Get(nsIFrame::IBSplitSpecialPrevSibling()),
>+         "Next-sibling / prev-sibling chain is inconsistent");

In this bug's testcase, we hit that line with |nextSib| being non-null -- however, it points to a destroyed frame (whose member data is all set to 0x7ffffffff0dea7ff or thereabouts).

Comment 3

[Daniel Holbert [:dholbert]]

So, bug 679933 was a case where an element's *next* IB sibling was outliving it, and so we needed to proactively clear that sibling's pointer.

This appears to be the same problem, except here, the *previous* IB sibling is the one that's living longer.

I imagine it'd be sufficient if we extended the chunk referenced in comment 2 to clear the pointer-to-ourself in *both* the prev & next siblings.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: fix v1

(fix along the lines of comment 3)

This fixes the testcase here as well as the one from bug 682649.  I've included both of them as crashtests.

Comment 5

[Daniel Holbert [:dholbert]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/893d04548dd9

Comment 7

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/893d04548dd9

Comment 8

[Martijn Wargers (dead)]

Verified fixed, using current trunk build.