Closed Bug 1085050 Opened 10 years ago Closed 10 years ago

Assertion failure: mTopRowIndex == mozilla::clamped(mTopRowIndex, 0, maxTopRowIndex), at .../layout/xul/tree/nsTreeBodyFrame.cpp

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox33 --- unaffected
firefox34 --- fixed
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: ishikawa, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: regression)

Attachments

(1 file)

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
See Also: → 1066459
Attached patch fixSplinter Review
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.
Assignee: nobody → mats
Status: NEW → ASSIGNED
Attachment #8507506 - Flags: review?(kgilbert)
Blocks: 1066459
Severity: critical → normal
Keywords: regression
OS: Linux → All
Hardware: x86_64 → All
See Also: 1066459
(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.
Attachment #8507506 - Flags: review?(kgilbert) → review+
https://hg.mozilla.org/mozilla-central/rev/2a688c2925bd
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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
Attachment #8507506 - Flags: approval-mozilla-beta?
Attachment #8507506 - Flags: approval-mozilla-aurora?
Comment on attachment 8507506 [details] [diff] [review]
fix

Beta+
Aurora+
Attachment #8507506 - Flags: approval-mozilla-beta?
Attachment #8507506 - Flags: approval-mozilla-beta+
Attachment #8507506 - Flags: approval-mozilla-aurora?
Attachment #8507506 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: