325218 - Crash with evil xul testcase, using box, tooltip, object, etc [@ DoDeletingFrameSubtree]

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase, which crashes upon load in current trunk builds.
This looks like a regression, doesn't crash in 2006-01-26 build, but crashes in 2006-01-27 build, very likely a regression from bug 310638.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase (crashes on load)

Talkback ID TB14548920Z: 
DoDeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9809]
DoDeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9815]
DeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9878]
nsCSSFrameConstructor::ContentRemoved  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10064]
nsCSSFrameConstructor::ReinsertContent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9740]
nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 11646]
nsCSSFrameConstructor::ProcessRestyledFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10538]
nsCSSFrameConstructor::RestyleElement  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10608]
nsCSSFrameConstructor::ProcessOneRestyle  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13415]
nsCSSFrameConstructor::ProcessPendingRestyles  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13467]
nsCSSFrameConstructor::RestyleEvent::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13534]
SHELL32.dll + 0x520c24 (0x778b0c24)

Comment 2

[Steve England [:stevee]]

Not related to bug 288357?

Comment 3

[Martijn Wargers (dead)]

No, not likely.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Mats, do you have time to look into this?

Is the issue here that we're failing to tear down the special siblings as you've commented elsewhere?  Note that in general we don't run into it because we reframe the containing block, but in this case there is no containing block...

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Frame tree

Comment 6

[Mats Palmgren (inactive)]

(In reply to comment #4)
> Is the issue here that we're failing to tear down the special siblings as
> you've commented elsewhere?

Yes, this is basically the same thing. We have two pending restyles
in ProcessPendingRestyles() for content that are descendents of a special
sibling (not the first in the chain), so the first restyle would destroy
the first special in the chain but since we don't destroy the following
the second restyle ended up doing a second DeletingFrameSubtree(), now on the
second special sibling that we left around.
In this frame there is a placeholder (the tooltip) that now have a null
out-of-flow.

Your patch for bug 322678 should fix this bug also.
I'm still a little worried that there are other (potentially bogus) frame
trees that would trigger this crash, so...

Comment 7

[Mats Palmgren (inactive)]

Attachment: wallpaper, just in case...

... so maybe we should take this also, just in case.
(Until bug 323105 removes this function, but that might not land on branches?)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Fixed by patch in bug 322678.

Mats, I suspect the wallpaper would just delay crashes, not remove them, no?

Comment 9

[Mats Palmgren (inactive)]

(In reply to comment #8)
> Mats, I suspect the wallpaper would just delay crashes, not remove them, no?

I think it would actually avoid crashes that is caused by traversing the same
(sub)tree twice. Not that I have any reason to believe that it will occur though.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Hmm.. If you think that's possible, might be worth taking the patch on branch, I guess.

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Using the testcase at https://bugzilla.mozilla.org/attachment.cgi?id=210137&action=view

this is Verified FIXED with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060209 Firefox/1.6a1

Comment 12

[Boris Zbarsky [:bzbarsky]]

Bug 322678 is on branches.

Comment 13

[Dave Liebreich [:davel]]

Marking [rft-dl] (ready for testing in Firefox 1.5.0.2 release candidates)

Comment 14

[Marcia Knous [:marcia]]

Verified on the 1.8.0.2 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060306 Firefox/1.5.0.2. No crash using the testcase. Adding relevant keyword.

Comment 15

[Tracy Walker [:tracy]]

verified with Fx 2.0b2 builds from 22060821