671792 - "ASSERTION: ImmAssociateContextEx failed to restore default IMC"

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Jesse Ruderman]

1. In the address bar, enter http://www.squarefree.com/bug662261/x.test
2. Right-click Firefox's taskbar icon and select "close all windows"

Result:

###!!! ASSERTION: ImmAssociateContextEx failed to restore default IMC: 'ret || !nsIMM32Handler::IsIMEAvailable()', file widget/src/windows/nsWindow.cpp, line 8038

Comment 1

[Brian R. Bondy [:bbondy]]

I can reproduce this so I'll take it.

Comment 2

[Brian R. Bondy [:bbondy]]

Attachment: Patch for ASSERTION: ImmAssociateContextEx failed to restore default IMC

The assertion was failing because of a GetLastError() of invalid handle on the Win32 API call to "::ImmAssociateContextEx(mWnd, .._)".
The problem was that we were destroying the window twice.

The call "DispatchStandardEvent(NS_DESTROY);" inside "nsWindow::OnDestroy" was sometimes leading to "nsXULWindow::Destroy" which would in turn call "mWindow->Destroy()" a second time. Where the error happened.
  
We were already checking to make sure we didn't destroy twice with "if (nsnull == mWnd) return NS_OK" in "nsWindow::Destroy()" but this would only check that we finished the WM_DESTROY message, not that we already started the WM_DESTROY message.  

Instead we should check that we have started the WM_DESTROY message to ensure the above situation doens't happen.  This can be done by changing: "if (nsnull == mWnd) return NS_OK" to " if (mOnDestroyCalled)
 return NS_OK;"

Comment 3

[Brian R. Bondy [:bbondy]]

This patch also fixes a different similar error in Bug 622312.

Comment 4

[[e-mail]]

Aha, so OnDestroy calls DispatchStandardEvent(NS_DESTROY) which normally does nothing, because something else has been destroyed first. But sometimes it hasn't, in which case nsXULWindow tries to destroy the nsWindow again...

Comment 5

[Brian R. Bondy [:bbondy]]

Right, basically from within the destroy it can lead to another destroy.  We had a check for double destroy but the way we checked it was if the destroy was done, and not if the destroy had already started.

Comment 6

[Brian R. Bondy [:bbondy]]

Comment on attachment 546938 [details] [diff] [review]
Patch for ASSERTION: ImmAssociateContextEx failed to restore default IMC

Going to run through try before checkin?

Comment 7

[Brian R. Bondy [:bbondy]]

pushed to mozilla-inbound.

Comment 8

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/72459db22a34