538082 - "ASSERTION: We should have processed mPendingRestyles to completion" with MathML and XUL

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: We should have processed mPendingRestyles to completion: 'mPendingRestyles.Count() == 0', file layout/base/nsCSSFrameConstructor.cpp, line 11163

###!!! ASSERTION: We should not have posted new non-animation restyles while processing animation restyles: 'mPendingRestyles.Count() == 0', file layout/base/nsCSSFrameConstructor.cpp, line 11185

bz added this pair of assertions in rev 2e580c431f4e (bug 528306 part 3, hook up restyle processing to nsRefreshDriver).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hmm.  I though I could assert these because ProcessPendingRestyleTable loops until the table is empty, but in fact it calls EndUpdate() after the loop and EndUpdate() can trigger a restyle post; here's a stack:

#0  nsCSSFrameConstructor::PostRestyleEvent (this=0x202de9d0, aContent=0x202f26c0, aRestyleHint=eReStyle_Self, aMinChangeHint=0) at nsCSSFrameConstructor.h:235
#1  0x128b66da in nsCSSFrameConstructor::DoContentStateChanged (this=0x202de9d0, aContent=0x202f26c0, aStateMask=8388608) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:7663
#2  0x128b66ff in nsCSSFrameConstructor::ContentStatesChanged (this=0x202de9d0, aContent1=0x202f26c0, aContent2=0x0, aStateMask=8388608) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:7612
#3  0x12945448 in PresShell::ContentStatesChanged (this=0x202de5d0, aDocument=0x1194e00, aContent1=0x202f26c0, aContent2=0x0, aStateMask=8388608) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsPresShell.cpp:4892
#4  0x12bfc57a in nsDocument::ContentStatesChanged (this=0x1194e00, aContent1=0x202f26c0, aContent2=0x0, aStateMask=8388608) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsDocument.cpp:4068
#5  0x131bbb6c in nsMathMLElement::SetIncrementScriptLevel (this=0x202f26c0, aIncrementScriptLevel=1, aNotify=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/mathml/content/src/nsMathMLElement.cpp:432
#6  0x1319be3e in nsMathMLContainerFrame::SetIncrementScriptLevel (this=0x17cae78, aChildIndex=0, aIncrement=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/mathml/nsMathMLContainerFrame.cpp:1358
#7  0x131a9819 in nsMathMLmfracFrame::TransmitAutomaticData (this=0x17cae78) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/mathml/nsMathMLmfracFrame.cpp:136
#8  0x1319b892 in nsMathMLContainerFrame::RebuildAutomaticDataForChildren (aParentFrame=0x17cae78) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/mathml/nsMathMLContainerFrame.cpp:714
#9  0x1319b846 in nsMathMLContainerFrame::RebuildAutomaticDataForChildren (aParentFrame=0x17c9158) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/mathml/nsMathMLContainerFrame.cpp:709
#10 0x131a31f2 in nsMathMLmoFrame::MarkIntrinsicWidthsDirty (this=0x17c9438) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/mathml/nsMathMLmoFrame.cpp:992
#11 0x1293fd93 in PresShell::FrameNeedsReflow (this=0x202de5d0, aFrame=0x17cafd8, aIntrinsicDirty=eStyleChange, aBitToAdd=1024) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsPresShell.cpp:3341
#12 0x12a0e1d8 in nsTextFrame::CharacterDataChanged (this=0x17cafd8, aInfo=0xbfffc514) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/generic/nsTextFrameThebes.cpp:3808
#13 0x128c9718 in nsCSSFrameConstructor::CharacterDataChanged (this=0x202de9d0, aContent=0x202f4a50, aInfo=0xbfffc514) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:7422
#14 0x12945659 in PresShell::CharacterDataChanged (this=0x202de5d0, aDocument=0x1194e00, aContent=0x202f4a50, aInfo=0xbfffc514) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsPresShell.cpp:4877
#15 0x12c5e254 in nsNodeUtils::CharacterDataChanged (aContent=0x202f4a50, aInfo=0xbfffc514) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsNodeUtils.cpp:101
#16 0x12c36b90 in nsGenericDOMDataNode::SetTextInternal (this=0x202f4a50, aOffset=0, aCount=0, aBuffer=0x1e7e7a88, aLength=1, aNotify=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsGenericDOMDataNode.cpp:441
#17 0x12c37233 in nsGenericDOMDataNode::SetData (this=0x202f4a50, aData=@0x202f0120) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsGenericDOMDataNode.cpp:284
#18 0x12c87ea4 in nsTextNode::SetData (this=0x202f4a50, aData=@0x202f0120) at nsTextNode.h:70
#19 0x1294efb2 in nsQuoteList::RecalcAll (this=0x202de9fc) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsQuoteList.cpp:116

We could try to move the begin/end of the update inside the loop, or just change these asserts to not assert that the restyle tables are empty....  David, do you have a preference?  The latter is probably safer and likely better...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Remove the bogus assert

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Remove the assert and make that non-bogus

Turns out, the assert was asserting that the mInStyleRefresh set was in the right place.  Clearly it wasn't!

Comment 4

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

Comment on attachment 427820 [details] [diff] [review]
Remove the assert and make that non-bogus

r=dbaron

Comment 5

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/87156411e1e4