It appears to be unused and unnecessary for OS/2 and Windows.

Also want to do this to os/2 (which is similar to the windows widget backend) so that bug 503879 can remove more code.

How sure are you that this can go away? Would we ever receive a destroy message on the wrong thread? (Have you looked at where this code came from?)

This code has been there since day 1. See http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.cpp&rev=1.1&root=/cvsroot I don't think we ever get calls to widget on other threads, given how thread adverse xpcom (nsIToolkit/nsIWidget) and the rest of the codebase are. Also, this interface doesn't exist on the newer cocoa and gtk2 backends so I suspect it has not been used or needed for quite some time.

Comment on attachment 388231 [details] [diff] [review] os/2 patch > MRESULT EXPENTRY nsToolkitWindowProc(HWND hWnd, ULONG msg, MPARAM mp1, > MPARAM mp2) > { >- switch (msg) { >- case WM_CALLMETHOD: >- { >- MethodInfo *info = (MethodInfo *)mp2; >- return (MRESULT)info->Invoke(); >- } >- } >+ // XXX: is this needed anymore? > return ::WinDefWindowProc(hWnd, msg, mp1, mp2); I don't think it is still needed. I completely removed this function and used NULL instead in the class registration call. That didn't seem to change anything. >--- a/widget/src/os2/nsWindow.h >+++ b/widget/src/os2/nsWindow.h [...] > class nsWindow : public nsBaseWidget, >- public nsSwitchToUIThread Please remove the comma at the end of the line.

Rich, do you have a deeper understanding of the nsToolkit class on OS/2 to comment on the necessity of the nsToolkitWindowProc() function? I have only run an OS/2 build with this patch for 20 minutes or so, and I'm a bit surprised that it didn't seem to have any effect. But if it works, this is great cleanup! :-)

(In reply to comment #5) > Rich, do you have a deeper understanding of the nsToolkit class on > OS/2 to comment on the necessity of the nsToolkitWindowProc() function? Not really. I did some basic testing (viewed various pages & dialogs, dragged tabs off to open a new window, watched Flash & Ogg videos, etc) and never once saw the toolkit window proc invoked. I also confirmed that Firefox never creates more than one message queue (on thread 1, as expected). Some comments on the patch: - the OS/2 nsToolkitWindowProc() is no longer used & should be eliminated - it might not be a bad idea to include an assertion or whatever in Create() and Destroy() to ensure that they're being invoked on thread 1 (I assume that TID 1 will remain the one & only GUI thread). - to forestall future problems, it should be clearly documented (especially in the Win32 version) that the GUI functions formerly handled by toolkit must be invoked from thread 1 only

(In reply to comment #6) > Some comments on the patch: > - the OS/2 nsToolkitWindowProc() is no longer used & should be eliminated Will do, gladly. > - it might not be a bad idea to include an assertion or whatever in Create() > and Destroy() to ensure that they're being invoked on thread 1 (I assume that > TID 1 will remain the one & only GUI thread). I'm not sure if this is necessary. All of widget/ (well ok, maybe not BeOS) is written to assume a single thread and it would probably not be beneficial to write this assert. > - to forestall future problems, it should be clearly documented (especially in > the Win32 version) that the GUI functions formerly handled by toolkit must be > invoked from thread 1 only I think it's just understood by convention/a lack of locks (this is true for most of the mozilla codebase). All our widget backends work this way. This code is buggy anyways - there's a race condition in the initialization of the TLS index for nsToolkit.

Windows patch pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/da889626b008 OS/2 patch and bug expanded to bug 508397