556114 - crash [@ activateWindow]

Status: RESOLVED FIXED

(Thunderbird :: OS Integration, defect)

Description

[Wayne Mery (:wsmwk)]

[@ activateWindow]
doesn't appear prior to v3.0.4, so speculatively labeling this a regression. if it's really a regression, hard to say when it started because it doesn't appear for 3.1 branch and the 4 crashes in FF of the past couple months are not a match. currently at #22, not a topcrash for v3. 

most crashes show uptime in range of 10-100 minutes

bp-87482254-f499-4964-b9d8-9e0d32100321  (no extensions)
0	thunderbird.exe	activateWindow	 mailnews/base/src/nsMessengerWinIntegration.cpp:133
1	thunderbird.exe	nsMessengerWinIntegration::AlertClicked	mailnews/base/src/nsMessengerWinIntegration.cpp:616
2	thunderbird.exe	nsMessengerWinIntegration::Observe	mailnews/base/src/nsMessengerWinIntegration.cpp:635
3	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
4	thunderbird.exe	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2456
5	thunderbird.exe	XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1590
6	js3250.dll	js_Invoke	js/src/jsinterp.cpp:1386
7	js3250.dll	js_Interpret	js/src/jsinterp.cpp:5179
8	js3250.dll	js_Invoke	js/src/jsinterp.cpp:1394
9	js3250.dll	js_InternalInvoke	js/src/jsinterp.cpp:1447
10	js3250.dll	JS_CallFunctionValue	js/src/jsapi.cpp:5188
11	thunderbird.exe	nsJSContext::CallEventHandler	dom/src/base/nsJSEnvironment.cpp:2107
12	thunderbird.exe	nsJSEventListener::HandleEvent	dom/src/events/nsJSEventListener.cpp:247
13	thunderbird.exe	nsXBLPrototypeHandler::ExecuteHandler	content/xbl/src/nsXBLPrototypeHandler.cpp:341
14	thunderbird.exe	nsXBLEventHandler::HandleEvent	content/xbl/src/nsXBLEventHandler.cpp:88
15	thunderbird.exe	nsEventListenerManager::HandleEventSubType	content/events/src/nsEventListenerManager.cpp:1098
16	thunderbird.exe	nsEventListenerManager::HandleEvent	content/events/src/nsEventListenerManager.cpp:1206
17	thunderbird.exe	nsEventTargetChainItem::HandleEvent	content/events/src/nsEventDispatcher.cpp:236
18	thunderbird.exe	nsEventTargetChainItem::HandleEventTargetChain	content/events/src/nsEventDispatcher.cpp:324
19	thunderbird.exe	nsEventDispatcher::Dispatch	content/events/src/nsEventDispatcher.cpp:514
20	thunderbird.exe	PresShell::HandleEventInternal	layout/base/nsPresShell.cpp:6323
21	thunderbird.exe	PresShell::HandleEventWithTarget	layout/base/nsPresShell.cpp:6228
22	thunderbird.exe	nsEventStateManager::CheckForAndDispatchClick	content/events/src/nsEventStateManager.cpp:4073
23	thunderbird.exe	nsEventStateManager::PostHandleEvent	content/events/src/nsEventStateManager.cpp:3036
24	thunderbird.exe	PresShell::HandleEventInternal	layout/base/nsPresShell.cpp:6344
25	thunderbird.exe	PresShell::HandlePositionedEvent	layout/base/nsPresShell.cpp:6211
26	thunderbird.exe	PresShell::HandleEvent	layout/base/nsPresShell.cpp:6071

Comment 1

[Wayne Mery (:wsmwk)]

#18 crash for 3.1. about the same crash rate for 3.0.5.
socorro lists a correlation to module msacm32.drv

bp-7abb455c-62a9-42f1-b1c7-765a82100627 (alfred)
bp-68023f6d-964d-434d-b27c-ac0342100707 (williamtang)
bp-b77c77f3-de55-4f65-aac9-c13ed2100604 (saulius) 
 clicked on the notification area icon, and clicked on a message. 

other comments: New e-mail notification popped up on the bottom-left. After clicking on it Thunderbird crashed.

ludo can you translate comment in bp-f418b57a-6b7f-4e3a-9dd5-941fa2100707

Comment 2

[Ludovic Hirlimann [:Usul]]

copy of a reservation on compagnie oceane.fr

THe geramn comment doesn't say anything and I don't read .cz.

Comment 3

[timeless]

319 NS_IMETHODIMP nsMsgWindow::GetDomWindow(nsIDOMWindowInternal **aWindow)
320 {
321   NS_ENSURE_ARG_POINTER(aWindow);
322   if (mDomWindow)
323     CallQueryReferent(mDomWindow.get(), aWindow);
324   else
325     *aWindow = nsnull;
326   return NS_OK;
327 }

602 nsresult nsMessengerWinIntegration::AlertClicked()
603 {
604 #ifdef MOZ_THUNDERBIRD
605   nsresult rv;
606   nsCOMPtr<nsIMsgMailSession> mailSession = do_GetService(NS_MSGMAILSESSION_CONTRACTID, &rv);
607   NS_ENSURE_SUCCESS(rv,rv);
608   nsCOMPtr<nsIMsgWindow> topMostMsgWindow;
609   rv = mailSession->GetTopmostMsgWindow(getter_AddRefs(topMostMsgWindow));
610   if (topMostMsgWindow)
611   {
612     nsCOMPtr<nsIDOMWindowInternal> domWindow;
613     rv = topMostMsgWindow->GetDomWindow(getter_AddRefs(domWindow));
614     NS_ENSURE_SUCCESS(rv, rv);

this always succeeds

615
616     activateWindow(domWindow); 

121 static void activateWindow( nsIDOMWindowInternal *win )
122 {
123   // Try to get native window handle.
124   HWND hwnd = hwndForDOMWindow( win );

the hwnd for a null window is null

125   if ( hwnd )
126   {
127     // Restore the window if it is minimized.
128     if ( ::IsIconic( hwnd ) )
129       ::ShowWindow( hwnd, SW_RESTORE );
130     // Use the OS call, if possible.
131     ::SetForegroundWindow( hwnd );
132   } else // Use internal method.

and we crash here:

133     win->Focus();
134 }

Comment 4

[timeless]

Attachment: patch

Comment 5

[[e-mail]]

Comment on attachment 457551 [details] [diff] [review]
patch

>     nsCOMPtr<nsIDOMWindowInternal> domWindow;
...
>+      nsCOMPtr<nsIDOMWindowInternal> domWindow;
Duplicate? r=me with this removed.

>@@ -589,18 +593,19 @@ nsresult nsMessengerWinIntegration::Aler
This is #ifdef THUNDERBIRD so I can't review it.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 457551 [details] [diff] [review]
patch

Sorry for the delay, I've given this a test and it looks good. I've also checked this in for you:

http://hg.mozilla.org/comm-central/rev/e6fe3769d285

Comment 7

[Mark Banner (:standard8)]

Ludovic: a thought occurs for a Litmus test or two; something along the lines of:

- With main window open
- Send yourself email
- Check for Prompt
- click on prompt

=> Check you get taken to the email

- Open address book window, close main window
- Send yourself email
- check for prompt
- click on prompt

=> Check the main window opens again at the message/folder.

I think this should work the same on all platforms (or very similar, e.g. open the message in a window rather than in the folder), and would have possibly picked up something like this or other errors in that area.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 457551 [details] [diff] [review]
patch

We should consider this for the next stability releases (req for 3.1.3 as the 3.1.4 flag should be coming later).

Comment 9

[Ludovic Hirlimann [:Usul]]

12693
12694

Comment 10

[Wayne Mery (:wsmwk)]

(In reply to comment #9)
> 12693
> 12694

does the test description contain prerequisite settings that notification must be enabled? (many people have it turned off)

Comment 11

[Ludovic Hirlimann [:Usul]]

(In reply to comment #10)
> (In reply to comment #9)
> > 12693
> > 12694
> 
> does the test description contain prerequisite settings that notification must
> be enabled? (many people have it turned off)

No as it's on by default ....

Comment 12

[Wayne Mery (:wsmwk)]

so only guaranteed correct if you have a new profile

Comment 13

[Mark Banner (:standard8)]

Checked in to 1.9.2: http://hg.mozilla.org/releases/comm-1.9.2/rev/bca26d5d7710

Comment 14

[Mark Banner (:standard8)]

Also checked into relbranch:

http://hg.mozilla.org/releases/comm-1.9.2/rev/73543a88d274

Comment 15

[Mark Banner (:standard8)]

Checked into 1.9.1: http://hg.mozilla.org/releases/comm-1.9.1/rev/24eb33b5214d