Assertion failure: mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), at .../layout/xul/tree/nsTreeBodyFrame.cpp:4100 I get the assertion failure during |make mozmill| test run of DEBUG version of C-C TB. (This has been seen in the last couple of days and I think changes by the patch in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1066459 seem to have caused an issue. (Maybe a subtle semantic change or something extra needs to be dealt with.) I put a dump statement just before the MOZ_ASSERT() in the code snipet in the end to trace the the relvant values of variables just before the crash. Here are the two dumps from TB's crashes during |make mozmill| crashed TB, and one from a manual invocation of TB. I sanitized the stack trace using |fix_linux_stack.py|. For the manual invocation, crash occured when I try to delete a message by selecting it and using the context menu. That is, a message is deleted, and redrawing of the message list would have occurred when the crash occurs. In this particular case, I chose the last message in the list, but if I choose other messages TB crashes sometimes, and sometimes it keeps on running(!). It is interesting to note that there seem to be at least a couple of different paths leading to this crash. Crashes from |make mozmill| (I only leave relevant lines here. Other lines were removed from the excerpt.) No. 1: [...] ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=8 mRowCount=21, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=8 mRowCount=21, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=7 mRowCount=20, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=7 mRowCount=20, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=7 mRowCount=20, mPageLength=13 aRow=7 ScrollInternal: mTopRowIndex = 7 mozilla::clamped()=6, maxTopRowIndex=6 mRowCount=19, mPageLength=13 aRow=7 Assertion failure: mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), at /REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4100 #01: nsTreeBodyFrame::ScrollToRow(int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4048) #02: nsSliderFrame::AttributeChanged(int, nsIAtom*, int) (/REF-OBJ-DIR/objdir-tb3/layout/xul/../../dist/include/nsCOMPtr.h:816) #03: mozilla::RestyleManager::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/RestyleManager.cpp:1145) #04: PresShell::AttributeChanged(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/nsPresShell.cpp:4406) #05: nsNodeUtils::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/nsNodeUtils.cpp:110) #06: mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const&, nsAttrValue&, unsigned char, bool, bool, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/Element.cpp:2092) #07: mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/Element.cpp:1959) #08: nsXBLPrototypeBinding::AttributeChanged(nsIAtom*, int, bool, nsIContent*, nsIContent*, bool) (/REF-OBJ-DIR/objdir-tb3/dom/xbl/../../dist/include/nsIContent.h:329) #09: nsXBLBinding::AttributeChanged(nsIAtom*, int, bool, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/dom/xbl/nsXBLBinding.cpp:601) ... No.2: ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=0 mRowCount=2, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=0 mRowCount=2, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=0 mRowCount=2, mPageLength=13 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=1 mRowCount=10, mPageLength=9 aRow=1 ScrollInternal: mTopRowIndex = 1 mozilla::clamped()=0, maxTopRowIndex=0 mRowCount=10, mPageLength=13 aRow=0 Assertion failure: mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), at /REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4100 #01: nsTreeBodyFrame::ReflowFinished() (/REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4054) #02: PresShell::HandlePostedReflowCallbacks(bool) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/nsPresShell.cpp:4067) #03: PresShell::DidDoReflow(bool, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/nsPresShell.cpp:8722) #04: PresShell::ResizeReflowIgnoreOverride(int, int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/nsPresShell.cpp:2069) #05: nsViewManager::DoSetWindowDimensions(int, int) (/REF-OBJ-DIR/objdir-tb3/view/../dist/include/nsRect.h:54) #06: nsView::WindowResized(nsIWidget*, int, int) (/REF-COMM-CENTRAL/comm-central/mozilla/view/nsView.cpp:1000) #07: nsWindow::Resize(double, double, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/widget/gtk/nsWindow.cpp:1042) #08: nsXULWindow::SetSize(int, int, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/xpfe/appshell/nsXULWindow.cpp:564) #09: nsGlobalWindow::ResizeTo(int, int, mozilla::ErrorResult&) (/REF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:6971) #10: nsGlobalWindow::ResizeTo(int, int, mozilla::ErrorResult&) (/REF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:6972 (discriminator 3)) Crash from manual invocation of TB from the console. I chose the last message shown in the message pane, and hit DELETE when the crash occurred. ScrollInternal: mTopRowIndex = 5451 mozilla::clamped()=5451, maxTopRowIndex=5451 mRowCount=5461, mPageLength=10 aRow=5454 ScrollInternal: mTopRowIndex = 5451 mozilla::clamped()=5451, maxTopRowIndex=5451 mRowCount=5461, mPageLength=10 aRow=5454 ScrollInternal: mTopRowIndex = 5451 mozilla::clamped()=5451, maxTopRowIndex=5451 mRowCount=5461, mPageLength=10 aRow=5454 ScrollInternal: mTopRowIndex = 5451 mozilla::clamped()=5451, maxTopRowIndex=5451 mRowCount=5461, mPageLength=10 aRow=5454 ScrollInternal: mTopRowIndex = 5451 mozilla::clamped()=5451, maxTopRowIndex=5451 mRowCount=5461, mPageLength=10 aRow=5454 ScrollInternal: mTopRowIndex = 5451 mozilla::clamped()=5450, maxTopRowIndex=5450 mRowCount=5460, mPageLength=10 aRow=5451 Assertion failure: mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), at /REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4100 Assertion failure: mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), at /REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4100 #01: nsTreeBodyFrame::ScrollToRow(int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp:4048) #02: nsSliderFrame::AttributeChanged(int, nsIAtom*, int) (/REF-OBJ-DIR/objdir-tb3/layout/xul/../../dist/include/nsCOMPtr.h:816) #03: mozilla::RestyleManager::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/RestyleManager.cpp:1145) #04: PresShell::AttributeChanged(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int) (/REF-COMM-CENTRAL/comm-central/mozilla/layout/base/nsPresShell.cpp:4406) #05: nsNodeUtils::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/nsNodeUtils.cpp:110) #06: mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const&, nsAttrValue&, unsigned char, bool, bool, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/Element.cpp:2092) #07: mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/Element.cpp:1959) #08: nsXBLPrototypeBinding::AttributeChanged(nsIAtom*, int, bool, nsIContent*, nsIContent*, bool) (/REF-OBJ-DIR/objdir-tb3/dom/xbl/../../dist/include/nsIContent.h:329) #09: nsXBLBinding::AttributeChanged(nsIAtom*, int, bool, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/dom/xbl/nsXBLBinding.cpp:601) #10: mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const&, nsAttrValue&, unsigned char, bool, bool, bool) (/REF-COMM-CENTRAL/comm-central/mozilla/content/base/src/Element.cpp:2061) ... Dump statement Patch to ScrollInternal(): nsresult nsTreeBodyFrame::ScrollInternal(const ScrollParts& aParts, int32_t aRow) { if (!mView) { return NS_OK; } int32_t maxTopRowIndex = std::max(0, mRowCount - mPageLength); #if DEBUG fflush(stdout); fprintf(stderr, "ScrollInternal: mTopRowIndex = %d mozilla::clamped()=%d, " "maxTopRowIndex=%d " "mRowCount=%d, mPageLength=%d " "aRow=%d\n", mTopRowIndex, mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), maxTopRowIndex, mRowCount, mPageLength, aRow); #endif MOZ_ASSERT(mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex)); aRow = mozilla::clamped(aRow, 0, maxTopRowIndex); if (aRow == mTopRowIndex) { return NS_OK; } mTopRowIndex = aRow; Invalidate(); PostScrollEvent(); return NS_OK; } I hope someone can investigate this issue. If necessary, I can list full stack trace. TIA

> but if I choose other messages TB crashes > sometimes, and sometimes it keeps on running(!). If I choose the top-most message, for example, I can delete it and TB still keeps on running. This is the dump of scrollInternal from the start until the first message that is at the top position of the message window is deleted. (At the beginning, the last message was shown in the message window, and so I scrolled up to the top-most first message and then deleted it.) --- begin quote --- ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=0 mRowCount=0, mPageLength=0 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=0 mRowCount=0, mPageLength=0 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=5422 mRowCount=5459, mPageLength=37 aRow=5422 ScrollInternal: mTopRowIndex = 5422 mozilla::clamped()=5422, maxTopRowIndex=5449 mRowCount=5459, mPageLength=10 aRow=5449 ScrollInternal: mTopRowIndex = 5449 mozilla::clamped()=5449, maxTopRowIndex=5449 mRowCount=5459, mPageLength=10 aRow=5449 ScrollInternal: mTopRowIndex = 5449 mozilla::clamped()=5449, maxTopRowIndex=5449 mRowCount=5459, mPageLength=10 aRow=5449 ScrollInternal: mTopRowIndex = 5449 mozilla::clamped()=5449, maxTopRowIndex=5449 mRowCount=5459, mPageLength=10 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=5448 mRowCount=5458, mPageLength=10 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=5448 mRowCount=5458, mPageLength=10 aRow=0 ScrollInternal: mTopRowIndex = 0 mozilla::clamped()=0, maxTopRowIndex=5448 mRowCount=5458, mPageLength=10 aRow=3 ScrollInternal: mTopRowIndex = 3 mozilla::clamped()=3, maxTopRowIndex=5448 mRowCount=5458, mPageLength=10 aRow=6 --- end quote I think the crash occurs if I scroll down toward the end, and pick up a message there and delete it, but I have no idea of the exact pattern. TIA

My assumption that mTopRowIndex would have already been adjusted elsewhere when rows are removed doesn't hold. It's a bit ugly that ScrollInternal does that job but I don't want to mess with this code too much, so let's just leave it like that.

(In reply to Mats Palmgren (:mats) from comment #2) > Created attachment 8507506 [details] [diff] [review] > fix > > My assumption that mTopRowIndex would have already been adjusted elsewhere > when rows are removed doesn't hold. It's a bit ugly that ScrollInternal > does that job but I don't want to mess with this code too much, so let's > just leave it like that. Getting rid of assertion certainly lets TB run past the issue, and TB seems to delete a message and update the display accordingly, *BUT* undoing the deletion does not seem to work any more. I see the following message when I try to undelete the message and restore the message list: [19967] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80550005: file /REF-COMM-CENTRAL/comm-central/mailnews/local/src/nsLocalUndoTxn.cpp, line 183 ^C Will investigate more. TIA

(In reply to ISHIKAWA, Chiaki from comment #3) > (In reply to Mats Palmgren (:mats) from comment #2) > > Created attachment 8507506 [details] [diff] [review] > > fix > > > > My assumption that mTopRowIndex would have already been adjusted elsewhere > > when rows are removed doesn't hold. It's a bit ugly that ScrollInternal > > does that job but I don't want to mess with this code too much, so let's > > just leave it like that. > > Getting rid of assertion certainly lets TB run past the issue, > and TB seems to delete a message and update the display accordingly, > *BUT* undoing the deletion does not seem to work any more. > I see the following message when I try to undelete the message and restore > the > message list: > > [19967] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80550005: > file /REF-COMM-CENTRAL/comm-central/mailnews/local/src/nsLocalUndoTxn.cpp, > line 183 > ^C > Will investigate more. > > TIA It is possible that the previous crashes due to MOZ_ASSERT() left the external (.msf) files in corrupt state. After compacting to re-create the files, I think the problem went away, but I think we need more testing and verification. TIA

At least |make mozmill| seems to run without complaining loudly (and that is why I wondered why my manual testing did not work out.). mozmill sets up its own test account in its virtual environment and so was not affected by the corrupt .msf file issues, I think.

Well, I think a corrupt .msf file fails undo (undeleting a message) transaction is probably worth recording in bugzilla. I will file a bug. TIA

Comment on attachment 8507506 [details] [diff] [review] fix Review of attachment 8507506 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me, as mTopRowIndex will be effectively clamped by the function anyways.

Comment on attachment 8507506 [details] [diff] [review] fix Approval Request Comment [Feature/regressing bug #]: bug 1066459 [User impact if declined]: none, only debug builds are affected [Describe test coverage new/current, TBPL]: on m-c since 2014-10-21, but clearly we have no automatic test coverage for this [Risks and why]: zero risk [String/UUID change made/needed]: none

Comment on attachment 8507506 [details] [diff] [review] fix Beta+ Aurora+

https://hg.mozilla.org/releases/mozilla-aurora/rev/9f1722745a18 https://hg.mozilla.org/releases/mozilla-beta/rev/4fd0f4651a61