224643 - [FIX]nsContentIterator::PositionAt triggered ASSERTION: RemoveElementsAt(negative index): 'aIndex >= 0'

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[timeless]

the only mention of this assert is bug 183987, but there's no stack, so i have
no idea whether it's related.

I'm not 100% certain about this, but i think i loaded
http://webtools.mozilla.org/registry/file.cgi?cvsroot=/cvsroot&file=Bugzilla-Guide.pdf&dir=mozilla/webtools/bugzilla/docs/pdf
or something like it, and then clicked "View Diff's".
note that this assert happened hours ago, i'm only now getting around to filing.
the stack is still in msdev if someone wants something from it today.

-	this	0x046c5ab0
\-	mIndexes	{...}
 \-	nsVoidArray	{...}
  \-	mImpl	0x046c5ad4
   |	mBits	8
   \	mCount	1
+	lastNode	{0x04a7c8c0}
+	newCurNode	{0x04a7c988}
+	firstNode	{0x04a7c9ac}
	firstOffset	0
	i	2
+	parent	{0x04a7c988}
-	oldParentStack	{...}
\-	nsVoidArray	{...}
 \-	mImpl	0x046c5d88
  |	mBits	2147483650
  \	mCount	1
	lastOffset	1
+	tempNode	{0x04a7cf18}

###!!! ASSERTION: RemoveElementsAt(negative index): 'aIndex >= 0', file
i:/build/mozilla/xpcom/ds/nsVoidArray.cpp, line 561
Break: at file i:/build/mozilla/xpcom/ds/nsVoidArray.cpp, line 561
nsDebug::Assertion(const char * 0x002d65a0, const char * 0x002d6594, const char
* 0x002d6568, int 561) line 109
nsVoidArray::RemoveElementsAt(int -1, int 1) line 561 + 31 bytes
nsContentIterator::PositionAt(nsContentIterator * const 0x046c5ab0, nsIContent *
0x04a7c988) line 1165 + 6 bytes
nsContentIterator::Next(nsContentIterator * const 0x046c5ab0) line 1026 + 1 byte
nsHTMLAnchorElement::GetText(nsHTMLAnchorElement * const 0x04a7c9b0, nsAString &
{...}) line 596 + 13 bytes
XPTC_InvokeByIndex(nsISupports * 0x04a7c9b0, unsigned int 17, unsigned int 1,
nsXPTCVariant * 0x0012db58) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_GETTER) line 2023 + 42 bytes
0 [native frame]
1 js_file_menu(
 repos = "/cvsroot",
 dir = "mozilla/webtools/bugzilla/docs/pdf",
 file = "Bugzilla-Guide.pdf",
 rev = "1.5",
 branch = "HEAD",
 d = [object Event @ 0x4ae0b98])
["http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla2.617490e-308btoolsbugzilla&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=week&mindate=&maxdate=&cvsroot=
 
 evsroot":30]
    fileName = ""
    i = 0
    this = [object Window @ 0x3432c10]
2 onclick(event = [object Event @ 0x4ae0b98]) ["<unknown>":0]
    this =
http://bonsai.mozilla.org/cvsview2.cgi?subdir=mozilla/webtools/bugzilla/docs/pdf&files=Bugzilla-Guide.pdf&command=DIRECTORY&branch=HEAD&root=/cvsroot
3 [native frame]
XPCWrappedNative::GetAttribute(XPCCallContext & {...}) line 1886 + 14 bytes
XPC_WN_GetterSetter(JSContext * 0x0421e950, JSObject * 0x04059e70, unsigned int
0, long * 0x048c5110, long * 0x0012de24) line 1301 + 12 bytes
js_Invoke(JSContext * 0x0421e950, unsigned int 0, unsigned int 2) line 912 + 23
bytes
js_InternalInvoke(JSContext * 0x0421e950, JSObject * 0x04059e70, long 68617688,
unsigned int 0, unsigned int 0, long * 0x00000000, long * 0x0012e724) line 1006
+ 20 bytes
js_InternalGetOrSet(JSContext * 0x0421e950, JSObject * 0x04059e70, long
44337872, long 68617688, int 4, unsigned int 0, long * 0x00000000, long *
0x0012e724) line 1049 + 31 bytes
js_GetProperty(JSContext * 0x0421e950, JSObject * 0x04059e70, long 44337872,
long * 0x0012e724) line 2665 + 51 bytes
js_Interpret(JSContext * 0x0421e950, long * 0x0012e8c4) line 2763 + 1795 bytes
js_Invoke(JSContext * 0x0421e950, unsigned int 1, unsigned int 2) line 929 + 13
bytes
js_InternalInvoke(JSContext * 0x0421e950, JSObject * 0x04059e70, long 68617144,
unsigned int 0, unsigned int 1, long * 0x0012eb1c, long * 0x0012e9ec) line 1006
+ 20 bytes
JS_CallFunctionValue(JSContext * 0x0421e950, JSObject * 0x04059e70, long
68617144, unsigned int 1, long * 0x0012eb1c, long * 0x0012e9ec) line 3572 + 31 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x0421e8d8, void * 0x04059e70,
void * 0x041703b8, unsigned int 1, void * 0x0012eb1c, int * 0x0012eb20, int 0)
line 1297 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x04a7ca68, nsIDOMEvent
* 0x04673168) line 180 + 77 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x04a7cb48,
nsIDOMEvent * 0x04673168, nsIDOMEventTarget * 0x04547a08, unsigned int 4,
unsigned int 7) line 1423 + 13 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x04a7ca00,
nsIPresContext * 0x03f83ad8, nsEvent * 0x0012f280, nsIDOMEvent * * 0x0012eef0,
nsIDOMEventTarget * 0x04547a08, unsigned int 7, nsEventStatus * 0x0012f5bc) line
1524 + 8 bytes
nsGenericElement::HandleDOMEvent(nsGenericElement * const 0x04a7c988,
nsIPresContext * 0x03f83ad8, nsEvent * 0x0012f280, nsIDOMEvent * * 0x0012eef0,
unsigned int 7, nsEventStatus * 0x0012f5bc) line 2001 + 16 bytes
nsGenericHTMLElement::HandleDOMEventForAnchors(nsIPresContext * 0x03f83ad8,
nsEvent * 0x0012f280, nsIDOMEvent * * 0x00000000, unsigned int 1, nsEventStatus
* 0x0012f5bc) line 1416
GKLAYOUT! 0134a7af()
PresShell::HandleEventInternal(nsEvent * 0x0012f280, nsIView * 0x00000000,
unsigned int 1, nsEventStatus * 0x0012f5bc) line 6184 + 42 bytes
PresShell::HandleEventWithTarget(PresShell * const 0x04527aa8, nsEvent *
0x0012f280, nsIFrame * 0x04b32e94, nsIContent * 0x04a7c988, unsigned int 1,
nsEventStatus * 0x0012f5bc) line 6141 + 22 bytes
nsEventStateManager::CheckForAndDispatchClick(nsIPresContext * 0x03f83ad8,
nsMouseEvent * 0x0012f7d4, nsEventStatus * 0x0012f5bc) line 2912 + 13 bytes
nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x044156a8,
nsIPresContext * 0x03f83ad8, nsEvent * 0x0012f7d4, nsIFrame * 0x04b32e94,
nsEventStatus * 0x0012f5bc, nsIView * 0x04650ea0) line 1901 + 5 bytes
PresShell::HandleEventInternal(nsEvent * 0x0012f7d4, nsIView * 0x04650ea0,
unsigned int 1, nsEventStatus * 0x0012f5bc) line 6236 + 49 bytes
PresShell::HandleEvent(PresShell * const 0x04527ac0, nsIView * 0x04650ea0,
nsGUIEvent * 0x0012f7d4, nsEventStatus * 0x0012f5bc, int 0, int & 1) line 6079 +
25 bytes
nsViewManager::HandleEvent(nsView * 0x045df570, nsGUIEvent * 0x0012f7d4, int 0)
line 2303
GKLAYOUT! 01237e6b()
nsViewManager::DispatchEvent(nsViewManager * const 0x03f83d20, nsGUIEvent *
0x0012f7d4, nsEventStatus * 0x0012f6cc) line 2044 + 11 bytes
nsView::SetZIndex(int 1243092, int 2809328, int 0) line 690 + 4 bytes
nsWindow::DispatchEvent(nsWindow * const 0x045df63c, nsGUIEvent * 0x0012f7d4,
nsEventStatus & nsEventStatus_eIgnore) line 1049 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f7d4) line 1070
nsWindow::DispatchMouseEvent(unsigned int 301, unsigned int 0, nsPoint *
0x00000000) line 5189 + 21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 301, unsigned int 0, nsPoint *
0x00000000) line 5446
nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 13369623, long *
0x0012fc34) line 3979 + 28 bytes
nsWindow::WindowProc(HWND__ * 0x00130b2c, unsigned int 514, unsigned int 0, long
13369623) line 1332 + 27 bytes
USER32! SetTimer + 1077 bytes
USER32! DispatchMessageW + 278 bytes
USER32! DispatchMessageW + 11 bytes
nsAppShellService::Run(nsAppShellService * const 0x00c67968) line 476
main1(int 1, char * * 0x00444300, nsISupports * 0x00baaaa0) line 1292 + 32 bytes
main(int 1, char * * 0x00444300) line 1679 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! SetUnhandledExceptionFilter + 92 bytes

Comment 1

[Joe Francis]

snarf....

This looks like a bug in the optimization stuff bz and i did for the iterator. 
I can't repro this but I may be able to deduce from the stack what the bug is.

Comment 2

[Joe Francis]

Attachment: patch to content/base/src/nsContentIterator.cpp

if we get into a state in PositionAt where we are trying to save time by
piecing together how to change the array of cached indices, only do this if we
have a set of cached indices!  :-)

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 134826 [details] [diff] [review]
patch to content/base/src/nsContentIterator.cpp

This won't actually fix this assertion though, since mIndexes.Count() == 1 at
the time.

Comment 4

[Joe Francis]

I don't think the original logic of the problem line makes much sense.  I
changed it to simply use IndexOf on the void array to find the right point to
prune from.  Since these arrays are typically short, this should be fine.

Comment 5

[Joe Francis]

Attachment: content/base/src/nsContentIterator.cpp patch

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 134828 [details] [diff] [review]
content/base/src/nsContentIterator.cpp patch

Ok, I thought about this and I think this is the right patch.  If we get
asserts after this still, then there really is something seriously screwed up
somewhere.

Comment 7

[kinmoz]

Comment on attachment 134828 [details] [diff] [review]
content/base/src/nsContentIterator.cpp patch

So I'm a bit curious, would the bug be fixed if we just removed the +1 being
added here:

  // plus one for the node we're currently on.
  for (PRInt32 i = mIndexes.Count()+1; i > 0 && tempNode; i--)


and here?


      // All we need to do is drop some indexes.  Shortcut here.
      mIndexes.RemoveElementsAt(mIndexes.Count() - (oldParentStack.Count()+1),
				oldParentStack.Count());

Comment 8

[timeless]

*** Bug 254329 has been marked as a duplicate of this bug. ***

Comment 9

[Boris Zbarsky [:bzbarsky]]

*** Bug 194151 has been marked as a duplicate of this bug. ***

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Equivalent but slightly faster patch

I also added some comments to make it clearer what's going on (in particular
why one of the +1 things is actually needed)

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 171001 [details] [diff] [review]
Equivalent but slightly faster patch

r+sr=jst

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk for 1.8b