The default bug view has changed. See this FAQ.

[FIX]crash when switching off style sheet [@0x00000000 - DoDeletingFrameSubtree]

VERIFIED FIXED in mozilla1.8alpha5

Status

()

Core
Layout: R & A Pos
P1
critical
VERIFIED FIXED
13 years ago
6 years ago

People

(Reporter: Sekundes, Assigned: bz)

Tracking

({crash, testcase})

Trunk
mozilla1.8alpha5
crash, testcase
Points:
---
Bug Flags:
blocking-aviary1.0 -

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040816 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040816 Firefox/0.9.1+

It could be reproduced on Firefox nightly with Gecko 1.7.2, and Mozilla 1.8alpha3.

Reproducible: Always
Steps to Reproduce:
1. Go to http://literarymoose.info/
2. View -> Use Style -> None.
3. Crashes.

Comment 1

13 years ago
Confirm, 1.8a3 PC/WinXP
Talkback TB598661X
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash
Summary: crash when switching off style sheet on specific site → crash when switching off style sheet [@0x00000000 - DoDeletingFrameSubtree]
0x00000000
DoDeletingFrameSubtree 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 9125]
DoDeletingFrameSubtree 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 9155]
DeletingFrameSubtree 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 9201]
nsCSSFrameConstructor::RemoveFixedItems 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 13306]
nsCSSFrameConstructor::ReconstructDocElementHierarchy 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 7353]
nsCSSFrameConstructor::RecreateFramesForContent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 11353]
nsCSSFrameConstructor::ProcessRestyledFrames 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 9869]
PresShell::ReconstructStyleData 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5283]
PresShell::SetAuthorStyleDisabled 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 2137]
DocumentViewerImpl::SetAuthorStyleDisabled 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocumentViewer.cpp,
line 2360]
XPTC_InvokeByIndex 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2030]
XPC_WN_GetterSetter 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1312]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1283]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1379]
js_InternalGetOrSet 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1422]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3219]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1302]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1379]
JS_CallFunctionValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
3686]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1352]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/events/nsJSEventListener.cpp,
line 180]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1513]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1590]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2823]
PresShell::HandleDOMEventWithTarget 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6090]
nsMenuFrame::Execute 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsMenuFrame.cpp,
line 1637]
nsMenuFrame::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/nsMenuFrame.cpp,
line 436]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6060]
PresShell::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5925]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2295]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2025]
HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsView.cpp,
line 79]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1101]
nsWindow::DispatchWindowEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1118]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5404]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5655]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 4159]
nsWindow::WindowProc 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1380]
USER32.dll + 0x3a50 (0x77d43a50)
USER32.dll + 0x3b1f (0x77d43b1f)
USER32.dll + 0x3d79 (0x77d43d79)
USER32.dll + 0x3ddf (0x77d43ddf)
nsAppShellService::Run 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/appshell/src/nsAppShellService.cpp,
line 489]
main1 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1331]
main 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1801]
WinMain 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1827]
WinMainCRTStartup()
kernel32.dll + 0x214c7 (0x77e814c7)

Comment 3

13 years ago
(It crashes also under Linux)
OS: Windows 2000 → All
Hardware: PC → All

Comment 4

13 years ago
Created attachment 156494 [details]
Minimized testcase

Comment 5

13 years ago
I think from the Talkback that this is a Layout bug.
Assignee: general → nobody
Component: Browser-General → Layout
Keywords: testcase
QA Contact: general → core.layout
(Reporter)

Comment 6

13 years ago
Since "Page Style" feature is back in Firefox, I'd nominate for Firefox 1.0.
Flags: blocking-aviary1.0?
So I debugged this a bit.  The basic problem here is markup like this:

<div style="position: fixed">
  <div style="position: fixed"></div>
</div>

combined with something that causes us to reframe the root (the change in
::after content, in this case, but other thing could be used too).  When we
reframe the root (in ReconstructDocElementHierarchy), we call RemoveFixedItems,
which goes through the list of frames in the nsLayoutAtoms::fixedList of the
viewport and calls DeletingFrameSubtree on each one, then destroys it.

The problem is that the frame for the inner div comes _before_ the frame for the
outer div in the fixedList.  So we destroy it, then go on to the outer div's
frame, try to traverse through placeholders in DoDeletingFrameSubtree, and
crash, since the placeholder points to a deleted frame.

The reason that fixed frames end up out of order is that we put them in the
fixed list as we recurse up out of frame construction, and the inner one
naturally finishes constructing first...

Possible solutions here:

1)  Make the fixed frames be in the right order in the fixed list (by delaying
    adding them to the viewport until after we've recursed out of frame
    construction or something).
2)  Set the placeholder frames for the fixed frames to point to null before
    calling RemoveFixedItems() in ReconstructDocElementHierarchy (this means
    doing it before ClearPlaceholderFrameMap()).

I think we should do both, really...
Component: Layout → Layout: R & A Pos
Hmmm... it looks like mFixedItems should already be handling the delay thing I
mentioned.  I wonder why that's not working.
Don't we already maintain some sort of list of out-of-flow frames that have been
destroyed?  Why aren't we using that?
> Don't we already maintain some sort of list of out-of-flow frames that have
> been destroyed?

We have something like that in ProcessRestyledFrames().  I suppose we could
reuse the same idea here.  Other than that, I don't think we have anything, no.
 DeletingFrameSubtree() keeps track of out-of-flows it intends to destroy, but
we're dealing with two separate calls into DeletingFrameSubtree() here...

I found why the mFixedItems thing isn't working -- people are processing
children _before_ adding frames to aState.mFixedItems.  That seems wrong to me
(and is painful-ish to fix because of the multiple codepaths that happen in some
places...).
Created attachment 160106 [details] [diff] [review]
Patch fixing issue #2 in comment 7
Created attachment 160107 [details] [diff] [review]
Patch as diff -w

Previous patch was not quite right.
Attachment #160106 - Attachment is obsolete: true
Created attachment 160108 [details] [diff] [review]
Patch for checkin
Comment on attachment 160107 [details] [diff] [review]
Patch as diff -w

David, would you review?
Attachment #160107 - Flags: superreview?(dbaron)
Attachment #160107 - Flags: review?(dbaron)
I filed bug 261605 on the first issue I mention in comment 7.
Taking.
Assignee: nobody → bzbarsky
Priority: -- → P1
Summary: crash when switching off style sheet [@0x00000000 - DoDeletingFrameSubtree] → [FIX]crash when switching off style sheet [@0x00000000 - DoDeletingFrameSubtree]
Target Milestone: --- → mozilla1.8beta

Comment 17

13 years ago
bz/dbaron,  is this avaiary 1.7 branch kind of changes?
I'd be very wary of landing it on a branch....

Updated

13 years ago
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Comment on attachment 160107 [details] [diff] [review]
Patch as diff -w

r+sr=dbaron, although a cleaner approach would be nice in the future
Attachment #160107 - Flags: superreview?(dbaron)
Attachment #160107 - Flags: superreview+
Attachment #160107 - Flags: review?(dbaron)
Attachment #160107 - Flags: review+
Fixed.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta → mozilla1.8alpha5

Comment 21

13 years ago
V 20041006 PC/WinXP
Status: RESOLVED → VERIFIED
*** Bug 284122 has been marked as a duplicate of this bug. ***
Crash Signature: [@0x00000000 - DoDeletingFrameSubtree]
You need to log in before you can comment on or make changes to this bug.