25339 - crash in nsXULTreeElement::SetCurrentItem

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

if I have multiple 4.x profiles, and I remove Users50 and mozregistry.dat and 
start up 5.0, I get this crash, caused by deferencing null.

here's the stack

nsXULTreeElement::SetCurrentItem(nsIDOMXULElement * 0x00000000) line 793 + 44 
bytes
nsXULTreeElement::SelectItem(nsXULTreeElement * const 0x01e89208, 
nsIDOMXULElement * 0x00000000) line 174
XULTreeElementSelectItem(JSContext * 0x01d3a530, JSObject * 0x00d98ba0, unsigned 
int 1, long * 0x00d43d40, long * 0x0012d650) line 277 + 24 bytes
js_Invoke(JSContext * 0x01d3a530, unsigned int 1, unsigned int 0) line 666 + 26 
bytes
js_Interpret(JSContext * 0x01d3a530, long * 0x0012decc) line 2262 + 15 bytes
js_Invoke(JSContext * 0x01d3a530, unsigned int 0, unsigned int 0) line 682 + 13 
bytes
js_Interpret(JSContext * 0x01d3a530, long * 0x0012e704) line 2262 + 15 bytes
js_Invoke(JSContext * 0x01d3a530, unsigned int 0, unsigned int 0) line 682 + 13 
bytes
js_Interpret(JSContext * 0x01d3a530, long * 0x0012ef3c) line 2262 + 15 bytes
js_Invoke(JSContext * 0x01d3a530, unsigned int 1, unsigned int 2) line 682 + 13 
bytes
js_InternalInvoke(JSContext * 0x01d3a530, JSObject * 0x00d25e48, long 13846472, 
unsigned int 0, unsigned int 1, long * 0x0012f0c4, long * 0x0012f070) line 759 + 
19 bytes
JS_CallFunctionValue(JSContext * 0x01d3a530, JSObject * 0x00d25e48, long 
13846472, unsigned int 1, long * 0x0012f0c4, long * 0x0012f070) line 2771 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x01d3a6c0, void * 0x00d25e48, 
void * 0x00d347c8, unsigned int 1, void * 0x0012f0c4, int * 0x0012f0c0) line 562 
+ 33 bytes
nsJSEventListener::HandleEvent(nsIDOMEvent * 0x01eab3d4) line 128 + 57 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x01e52410, 
nsIDOMEvent * 0x01eab3d4, unsigned int 1) line 680 + 19 bytes
nsEventListenerManager::HandleEvent(nsIPresContext * 0x01d3af10, nsEvent * 
0x0012fad8, nsIDOMEvent * * 0x0012f4c0, unsigned int 7, nsEventStatus * 
0x0012fb18) line 1231 + 31 bytes
GlobalWindowImpl::HandleDOMEvent(GlobalWindowImpl * const 0x01d3ad24, 
nsIPresContext * 0x01d3af10, nsEvent * 0x0012fad8, nsIDOMEvent * * 0x0012f4c0, 
unsigned int 1, nsEventStatus * 0x0012fb18) line 3350
nsWebShell::OnEndDocumentLoad(nsWebShell * const 0x019629a0, nsIDocumentLoader * 
0x01960a20, nsIChannel * 0x01d35720, unsigned int 0) line 3141 + 34 bytes
nsDocLoaderImpl::FireOnEndDocumentLoad(nsDocLoaderImpl * 0x01960a20, nsIChannel 
* 0x01d35720, unsigned int 0) line 834
nsDocLoaderImpl::DocLoaderIsEmpty(unsigned int 0) line 725
nsDocLoaderImpl::OnStopRequest(nsDocLoaderImpl * const 0x01960a24, nsIChannel * 
0x01f36c00, nsISupports * 0x00000000, unsigned int 0, const unsigned short * 
0x00000000) line 669
nsLoadGroup::RemoveChannel(nsLoadGroup * const 0x019667b0, nsIChannel * 
0x01f36c00, nsISupports * 0x00000000, unsigned int 0, const unsigned short * 
0x00000000) line 535 + 42 bytes
nsFileChannel::OnStopRequest(nsFileChannel * const 0x01f36c04, nsIChannel * 
0x01f367d0, nsISupports * 0x00000000, unsigned int 0, const unsigned short * 
0x00000000) line 455
nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x01f33400) line 
279
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x01f374d0) line 93 + 12 bytes
PL_HandleEvent(PLEvent * 0x01f374d0) line 526 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00c74b90) line 487 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x0087014c, unsigned int 49434, unsigned int 0, 
long 13061008) line 975 + 9 bytes
USER32! 77e71820()
00c74b90()

Comment 1

[(not reading, please use [e-mail] instead)]

here's a bulletproofing patch to prevent the error.

hyatt, can you review?

I think the main problem is that unmigrate profiles are showing up in the tree, 
but there may be js that is trying to select an element in the tree.

I need to look at the profile.  maybe ben knows off the top of his head.

I think the "unmigrated profiles not showing up" is a known bug.  but if I can't 
find it, I'll log it.

Index: content/src/nsXULTreeElement.cpp
===================================================================
RCS file: /cvsroot/mozilla/rdf/content/src/nsXULTreeElement.cpp,v
retrieving revision 1.36
diff -r1.36 nsXULTreeElement.cpp
153a154,156
>   NS_ASSERTION(aTreeItem, "trying to select a null tree item");
>   if (!aTreeItem) return NS_OK;
>
253a257,259
>   NS_ASSERTION(aTreeItem,"attepting to add a null tree item to the selection")
;
>   if (!aTreeItem) return;
>

Comment 2

[David Hyatt]

We should make it so that this doesn't crash, but someone must have changed the 
JS to pass me null.  Why are people trying to select null?

Comment 3

[(not reading, please use [e-mail] instead)]

I'll consider that a review from hyatt.

I'll check in (to prevent the crash) and then we'll look at why we are trying to 
select null.

Comment 4

[(not reading, please use [e-mail] instead)]

patch checked in.  now it long longer crashes.

in addition to how I reproduced the problem, runnning with -ProfileManager with 
no profiles would cause the same problem.

I think the problem is in profileSelection.js

we get the current profile from the profile service, and call selectItem.

my guess is that in the cases I describe, there is no current profile, but we 
fail silently, and never check before calling selectItem.

verifying my theory now, and if so, I'll fix the js.

Comment 5

[(not reading, please use [e-mail] instead)]

yep, that was it.

here's the diff that I'm checking in:

Index: profileSelection.js
===================================================================
RCS file: /cvsroot/mozilla/profile/resources/content/profileSelection.js,v
retrieving revision 1.9
diff -r1.9 profileSelection.js
51,53c51,58
<     var currentProfileItem = document.getElementById( ( "profileName_" + curre
ntProfile ) );
<     var profileList = document.getElementById( "profiles" );
<     profileList.selectItem( currentProfileItem );
---
>     if (currentProfile) {
>       var currentProfileItem = document.getElementById( ( "profileName_" + cur
rentProfile ) );
>       var profileList = document.getElementById( "profiles" );
>       profileList.selectItem( currentProfileItem );
>     }
>     else {
>       dump("no current profile, can't highlight anything\n");
>     }

taking bug from hyatt.

Comment 6

[(not reading, please use [e-mail] instead)]

ben beat me to it, and fixed the js.

marking fixed.

Comment 7

[Grace Bush]

verified code fixed