190558 - crash on page using lots of W3C DOM stuff

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jacob Kjome]

Using build 2003012408 on Win2k (sp3)

The page in the URL field crashes after manipulating some form elements which
trigger DOM modifications.  The source to this page is here:
http://javascript.internet.com/forms/dynamic-table.html

Steps to reproduce:
1.  Open page ( http://javascript.internet.com/forms/dynamic-table-demo.html )
2.  Click on a column and modify an entry.
3.  Click on the "Update Table" button to update the entry
4.  Click on the "Scroll Up" and "Scroll Down" buttons in various ways
5.  Watch Mozilla crash

Talkback ID: TB16577833K

Jake

Comment 1

[Jacob Kjome]

There is an easier way to reproduce this bug...

1. Open page
2. Click on "Scroll Up" button once
3. Click on "Scroll Down" button once
4. Click on "Scroll Up" button once again

result is a crash

You can also flip the order around as well.  The key is to alternate.  As soon
as one scroll button has been hit twice while alternating, Mozilla always crashes.

Jake

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

nsStyleFont::CalcDifference
[c:/builds/seamonkey/mozilla/content/shared/src/nsStyleStruct.cpp, line 253]
nsStyleContext::CalcStyleDifference
[c:/builds/seamonkey/mozilla/content/base/src/nsStyleContext.cpp, line 659]
CaptureChange
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsFrameManager.cpp, line 1668]
FrameManager::ReResolveStyleContext
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsFrameManager.cpp, line 1794]
FrameManager::ComputeStyleChangeFor
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsFrameManager.cpp, line 2075]
nsCSSFrameConstructor::ContentStatesChanged
[c:/builds/seamonkey/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,
line 10546]
StyleSetImpl::ContentStatesChanged
[c:/builds/seamonkey/mozilla/content/base/src/nsStyleSet.cpp, line 1632]
PresShell::ContentStatesChanged
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 5139]
nsDocument::ContentStatesChanged
[c:/builds/seamonkey/mozilla/content/base/src/nsDocument.cpp, line 2042]
nsEventStateManager::SetContentState
[c:/builds/seamonkey/mozilla/content/events/src/nsEventStateManager.cpp, line 4025]
nsHTMLButtonElement::HandleDOMEvent
[c:/builds/seamonkey/mozilla/content/html/content/src/nsHTMLButtonElement.cpp,
line 565]
PresShell::HandleEventInternal
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6207]
PresShell::HandleEvent
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6130]
nsViewManager::HandleEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 2208]
nsView::HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 304]
nsViewManager::DispatchEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 1948]
HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 83]
nsWindow::DispatchEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1119]
nsWindow::DispatchWindowEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1136]
nsWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5378]
ChildWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5633]
nsWindow::ProcessMessage
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 4133]
nsWindow::WindowProc
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1403]
USER32.dll + 0x2a2b8 (0x77e3a2b8)
USER32.dll + 0x45b1 (0x77e145b1)
USER32.dll + 0xa752 (0x77e1a752)
nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 480]
main1 [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1289]
main [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1639]
WinMain [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1660]
WinMainCRTStartup()
KERNEL32.dll + 0x2847c (0x77ea847c) 

Comment 3

[Boris Zbarsky [:bzbarsky]]

To style system.

Comment 4

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

I can't reproduce the crash.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I can... here's what I see (this is a vanilla build, btw, not one with any
patches I have):

#0  0x40d71f12 in nsStyleFont::CalcDifference (this=0x88ae710, aOther=@0x0)
    at /home/bzbarsky/mozilla/xlib/mozilla/content/shared/src/nsStyleStruct.cpp:252
#1  0x410290b5 in nsStyleContext::CalcStyleDifference (this=0x8964114,
aOther=0x8964398, 
    aHint=@0xbfffe50c)
    at /home/bzbarsky/mozilla/xlib/mozilla/content/base/src/nsStyleContext.cpp:654
#2  0x40bca11f in CaptureChange (aOldContext=0x8964114, aNewContext=0x8964398, 
    aFrame=0x8963f00, aContent=0x87474a8, aChangeList=@0xbfffe7cc, 
    aMinChange=nsChangeHint_None)
    at
/home/bzbarsky/mozilla/xlib/mozilla/layout/html/base/src/nsFrameManager.cpp:1667

(gdb) frame 0
#0  0x40d71f12 in nsStyleFont::CalcDifference (this=0x88ae710, aOther=@0x0)
    at /home/bzbarsky/mozilla/xlib/mozilla/content/shared/src/nsStyleStruct.cpp:252
252       if (mSize == aOther.mSize) {
(gdb) p aOther
$6 = (nsStyleFont &) @0x0: Cannot access memory at address 0x0
(gdb) frame 1
#1  0x410290b5 in nsStyleContext::CalcStyleDifference (this=0x8964114,
aOther=0x8964398, 
    aHint=@0xbfffe50c)
    at /home/bzbarsky/mozilla/xlib/mozilla/content/base/src/nsStyleContext.cpp:654
654               NS_UpdateHint(aHint, font->CalcDifference(*otherFont));
(gdb) p otherFont
$7 = (nsStyleFont *) 0x0

David, do you have your patch to make sure that style structs never end up null
in the tree you tested with?

Comment 6

[Andrew Hagen]

Critical severity.

Comment 7

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

(gdb) frame 10
#10 0x413e3c99 in ComputeLineHeight (aPresContext=0x8f8b020,
    aRenderingContext=0x8f33050, aStyleContext=0x8ed1174)
    at /builds2/clean3/mozilla/layout/html/base/src/nsHTMLReflowState.cpp:2348
2348        deviceContext->GetMetricsFor(font->mFont, langGroup,
*getter_AddRefs(fm));

(gdb) p font
$1 = (const nsStyleFont *) 0x0
(gdb) p aStyleContext
$2 = (nsIStyleContext *) 0x8ed1174
(gdb) p (class nsStyleContext*)$
$3 = (nsStyleContext *) 0x8ed1174
(gdb) p *$
$4 = {<nsIStyleContext> = {<nsISupports> = {
      _vptr.nsISupports = 0x41ad6c48}, <No data fields>}, mRefCnt = {
    mValue = 3}, _mOwningThread = {mThread = 0x80d3478}, mParent = 0x8f9dbbc,
  mChild = 0x0, mEmptyChild = 0x8eac550, mPrevSibling = 0x8f9da64,
  mNextSibling = 0x8f9da64, mPseudoTag = {mRawPtr = 0x81d1e48},
  mRuleNode = 0x8f9da38, mCachedStyleData = {static gInfo = 0x0,
    mInheritedData = 0x8ef82a4, mResetData = 0x0}, mBits = 257}
(gdb) p/x $4.mBits
$5 = 0x101
(gdb) p $4.mParent
$8 = (nsStyleContext *) 0x8f9dbbc
(gdb) p *$
$9 = {<nsIStyleContext> = {<nsISupports> = {
      _vptr.nsISupports = 0x41ad6c48}, <No data fields>}, mRefCnt = {
    mValue = 3}, _mOwningThread = {mThread = 0x80d3478}, mParent = 0x8eac1b8,
  mChild = 0x8f9da64, mEmptyChild = 0x0, mPrevSibling = 0x8f9e140,
  mNextSibling = 0x8eac2f4, mPseudoTag = {mRawPtr = 0x0},
  mRuleNode = 0x8eac524, mCachedStyleData = {static gInfo = 0x0,
    mInheritedData = 0x9007ae0, mResetData = 0x0}, mBits = 1049856}
(gdb) p/x $9.mBits
$10 = 0x100500
(gdb) p *$9.mCachedStyleData.mInheritedData
$11 = {mFontData = 0x0, mColorData = 0x0, mListData = 0x0,
  mTextData = 0x8ef8338, mVisibilityData = 0x8fa1470, mQuotesData = 0x8ed104c,
  mUserInterfaceData = 0x9007dc4, mTableBorderData = 0x0, mSVGData = 0x8ed1010}

Comment 8

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

Er, there's nothing wrong with that (except the null).  More shortly...

Comment 9

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

Er, no, there is, but it's not what I pasted.  (I was getting backwards the way
rule nodes and style contexts handle setting dependent/inherit bits -- style
contexts copy the struct, but rule nodes don't.)

$4 had data with a null mFontData.

(And yes, I was testing in my build with the patch to bug 154751.

Comment 10

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

When I run the testcase in a *DEBUG* build with my patch on 154741 (the above
debugging was in a build without the patch, my first testing in an opt build
with the patch), I get the assertions:

    NS_ASSERTION(ruleNode->mDependentBits &
                 nsCachedStyleData::GetBitForSID(aSID),
                 "dependent bits improperly set");

in nsRuleNode::GetParentData.

Comment 11

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

I think really understanding what's going on requires simplifying the testcase.
 That looks complicated.

However, I can certainly imagine problems.  For example, I think our
ClearStyleData code will break when rule nodes are unreachable at the time of
the clear but then become reachable again (and they depend on data from the
parents, which were cleared properly).

Adding dependency to bug 188003.

Comment 12

[Andrew Schultz]

Attachment: testcase

crashes linux trunk 2003020122 every time.  sometimes crashes on the first
click, but usually after the second.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Hmm... This is that bug I needed to point hyatt to regarding clearing inline
style.  ;)

In my build with the patch for bug 171830 (the patch that makes us no longer
clear out style data on inline style changes) I do _not_ crash on that testcase
(or on the original page).

Comment 14

[Andrew Schultz]

Attachment: stacktrace from the testcase

stack from the testcase is completely different from the URL, but both
regressed between trunk builds 2002110508 and 2002110604.

Comment 15

[Andrew Schultz]

works on current trunk (20030313).  fixed by bug 171830