454751 - Crash [@ nsCSSFrameConstructor::ConstructXULFrame] with treecols, display:table

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Comment 1

[Boris Zbarsky [:bzbarsky]]

In nsCSSFrameConstructor::ConstructXULFrame we have aState.mPopupItems.containingBlock pointing to a deleted frame.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is more fun.  We're hitting nsCSSFrameConstructor::ReconstructDocElementHierarchyInternal which caches things like mRootBox and its popup set frame. Then we blow away the frame tree (and in particular the nsPopupSetFrame that is the mPopupItems.containingBlock of this state).  Then we use this state to construct the new frame tree and die.  The other containing blocks we explicitly init to null, so those don't run into this problem.

Not sure whether this is a regression from roc's root frame changes.

In any case, we should either create a new state for constructing the new frametree or do a better job os sanitizing the old state.

Given comment 1, closing up for now.

Comment 3

[Samuel Sidler (old account; do not CC)]

Can't reproduce using a Gran Paradiso nightly.

Comment 4

[Jesse Ruderman]

Indeed, today I found a related testcase that causes nsCSSFrameConstructor::ConstructFrameInternal to call 0xdddddddd.

Comment 5

[Daniel Holbert [:dholbert]]

The attached testcase crashes my linux mozilla-central debug build, for the reason bz mentioned in comment 1.

OS/Platform --> All.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Using a fresh nsFrameConstructorState when we construct the new frames is easy, and we don't crash anymore.

But we end up in a bad state where we have an assertion "Popup containing block is missing" ... that assertion is supposed to be suppressed if there's a root box, but there is in this case. Perhaps the problem there is that we decide to use NS_NewRootBoxFrame instead of NS_NewCanvasFrame based on whether the root element is a XUL element, instead of checking its display type. In any case, that's an existing bug on trunk, so I've filed it as bug 464409 and carry on with my patch for this bug here.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 347717 [details] [diff] [review]
fix

Looks good.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Landed on mozilla-central (and thus also on mozilla-1.9.1):
http://hg.mozilla.org/mozilla-central/rev/feb0232b6771

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with builds on OS X and Windows (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204020327)