117667 - NS_ASSERTION(aIndex < Count(),"nsVoidArray::ElementAt(index past end array) - note on bug 96108");

Status: VERIFIED FIXED

(Core :: DOM: Events, defect)

Description

[timeless]

NTDLL! 77fa018c()
nsDebug::Assertion(const char * 0x1010d518 `string', const char * 0x1010d568 
`string', const char * 0x1010d5d4 `string', int 78) line 290 + 13 bytes
nsVoidArray::ElementAt(int 0) line 78 + 35 bytes
nsDocShell::GetChildAt(nsDocShell * const 0x043a1308, int 0, 
nsIDocShellTreeItem * * 0x0012f518) line 1921 + 16 bytes
nsEventStateManager::GetNextDocShell(nsIDocShellTreeNode * 0x043a1308, 
nsIDocShellTreeItem * * 0x0012f518) line 4281
nsEventStateManager::ShiftFocusByDoc(int 1) line 4374 + 41 bytes
nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x0494d058, 
nsIPresContext * 0x04947780, nsEvent * 0x0012f928, nsIFrame * 0x04c30cf0, 
nsEventStatus * 0x0012f894, nsIView * 0x049665c0) line 1684
PresShell::HandleEventInternal(nsEvent * 0x0012f928, nsIView * 0x049665c0, 
unsigned int 1, nsEventStatus * 0x0012f894) line 6079 + 43 bytes
PresShell::HandleEvent(PresShell * const 0x04948d54, nsIView * 0x049665c0, 
nsGUIEvent * 0x0012f928, nsEventStatus * 0x0012f894, int 0, int & 1) line 5984 
+ 25 bytes
nsView::HandleEvent(nsView * const 0x049665c0, nsGUIEvent * 0x0012f928, 
unsigned int 0, nsEventStatus * 0x0012f894, int 0, int & 1) line 387
nsView::HandleEvent(nsView * const 0x049668c0, nsGUIEvent * 0x0012f928, 
unsigned int 0, nsEventStatus * 0x0012f894, int 0, int & 1) line 344
nsView::HandleEvent(nsView * const 0x04423670, nsGUIEvent * 0x0012f928, 
unsigned int 0, nsEventStatus * 0x0012f894, int 1, int & 1) line 344
nsViewManager::DispatchEvent(nsViewManager * const 0x04420ce0, nsGUIEvent * 
0x0012f928, nsEventStatus * 0x0012f894) line 1930
HandleEvent(nsGUIEvent * 0x0012f928) line 83
nsWindow::DispatchEvent(nsWindow * const 0x04966784, nsGUIEvent * 0x0012f928, 
nsEventStatus & nsEventStatus_eIgnore) line 846 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f928) line 867
nsWindow::DispatchKeyEvent(unsigned int 131, unsigned short 0, unsigned int 9) 
line 2549 + 15 bytes
nsWindow::OnKeyDown(unsigned int 9, unsigned int 15) line 2595
nsWindow::ProcessMessage(unsigned int 256, unsigned int 9, long 983041, long * 
0x0012fd24) line 3311 + 32 bytes
nsWindow::WindowProc(HWND__ * 0x003b08a2, unsigned int 256, unsigned int 9, 
long 983041) line 1111 + 27 bytes
USER32! 77e12e98()
USER32! 77e130e0()
USER32! 77e15824()
nsAppShellService::Run(nsAppShellService * const 0x005401f0) line 303
main1(int 2, char * * 0x00484320, nsISupports * 0x00000000) line 1264 + 32 bytes
main(int 2, char * * 0x00484320) line 1594 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e97d08()

mImpl == 0x0

###!!! ASSERTION: Bad docshell ChildAt index: 'aIndex >= 0 && aIndex < mChildren
.Count()', file f:\build\mozilla\docshell\base\nsDocShell.cpp, line 1920
###!!! ASSERTION: nsVoidArray::ElementAt(index past end array) - note on bug 961
08: 'aIndex < Count()', file ..\ds\nsVoidArray.h, line 78
###!!! Break: at file ..\ds\nsVoidArray.h, line 78

This is silly. We only need one assertion.  The code is fully aware of what 
it's doing. Since i don't want the VoidArray code to need those asserts, i'm 
going to supply a patch that returns ERROR FAILURE in nsDocShell::GetChildAt 
(of course, the callers don't check that, but it'll make me feel better).

Comment 1

[timeless]

Attachment: return failure

Comment 2

[timeless]

ctrl-tab in pageinfo and mail composer triggers this :-)

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Taking this bug, will attach a patch to stop nsEventStateManager from asking for
a bad index.  I don't care about the double-assertion so I'll leave that be.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: patch to stop bad indexes, ready for review

ready for r=/sr=
I don't check the rv from GetChildCount() because nothing else here does.  I do
set numChildren to 0 before calling it for paranoia though.
I also checked in LXR all other uses of GetChildAt() for docshells and they all
seem correct.

Comment 5

[joki (gone)]

r=joki

Comment 6

[Chris Waterson]

This seems reasonable, but -- would it be better just to do the check inside the
implementation of nsIDocShellTreeNode to avoid two virtual calls? (From every
place that wants to GetChildAt?)

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

We could, of course, use SafeElementAt in GetChildAt().  Since almost all the
references to docshell::GetChildAt's are in loops, the number of children is
checked once before entry to the loop, and then all the iterations just use
GetChildAt (with the faster inlined ElementAt).  In reality, it's not anything
big either way; I considered switching it.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 67116 [details] [diff] [review]
patch to stop bad indexes, ready for review

sr=jst either way.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Fix checked in (as posted).

Comment 10

[Vladimir Ermakov]

verifying build 2002-02-22-03-trunk win 2000... tried to ctrl-tab in page info
and mail composer, no crash.