GNOME Bugzilla – Bug 726224
W32: GDK tries to set GWLP_HWNDPARENT to NULL when it's NULL already
Last modified: 2014-08-06 07:41:06 UTC
Opening a context menu will always produce this warning: (gtk3-demo.exe:10376): Gdk-WARNING **: ../../../gtk+-3.11.5/gdk/win32/gdkwindow-win32.c:1834: SetWindowLongPtr failed: Invalid window handle. My understanding of the problem, after taking cursory glance at the code: 1) GDK creates a W32 window (with a handle) for the popup menu 2) GDK makes it transient for the parent window 3) Menu pops up 4) Menu pops down 5) GTK makes menu window transient for nothing. Interestingly, gdk_win32_window_set_transient_for() gets a non-NULL pointer as "parent", when it's called indirectly by gtk_window_transient_parent_unrealized (), even though gtk_window_set_transient_for() had parent=0x0. 6) GDK does not forget about menu window 7) Independently of GDK, W32 menu window is destroyed 8) User calls for a context menu again 9) GDK remembers the menu window it had, and destroys it 10) During window destruction it's made transient for nothing by GDK (for real this time, parent=0x0 in gdk_win32_window_set_transient_for()). 11) Because the W32 window handle that GDK kept around refers to a window that no longer exists, SetWindowLongPtr() fails.
Created attachment 271720 [details] [review] Suppress a warning from SetWindowLongPtr() I was wrong, that's not what is happening. The window is destroyed _after_ transient change, so it's not that the window handle is invalid, something else is triggering an error. It seems that the error happens when window owner is already NULL. This patch avoids re-setting the owner handle to 0 if it's already 0.
Created attachment 271721 [details] [review] Suppress a warning from SetWindowLongPtr() Basically, the same patch (as far as intentions go), but with cleaner code.
Review of attachment 271721 [details] [review]: See comments. ::: gdk/win32/gdkwindow-win32.c @@ +1827,3 @@ + old_ptr = GetWindowLongPtr (window_id, GWLP_HWNDPARENT); + w32_error = GetLastError (); + if (old_ptr == 0 && w32_error != NO_ERROR) no need for {} when it is one single call @@ +1844,3 @@ + w32_error = GetLastError (); + if (old_ptr == 0 && w32_error != NO_ERROR) + { same here
Created attachment 282344 [details] [review] Suppress a warning from SetWindowLongPtr() v2: fixed code style as suggested
Review of attachment 282344 [details] [review]: See minor nitpick ::: gdk/win32/gdkwindow-win32.c @@ +1806,3 @@ + w32_error = GetLastError (); + + if (old_ptr == 0 && w32_error != NO_ERROR) should old_ptr use NULL? @@ +1822,3 @@ + w32_error = GetLastError (); + + if (old_ptr == 0 && w32_error != NO_ERROR) same here.
LONG_PTR old_ptr; MSDN says: typedef __int3264 LONG_PTR; 2.2.1 __int3264 An alias that is resolved to either: An __int32 in a 32-bit translation and execution environment, or An __int64 in a 64-bit translation and execution environment. For backward compatibility, it is 32-bit on the wire. The higher 4 bytes MUST be truncated on the sender side during marshaling and MUST be extended appropriately (signed or unsigned), as specified in [C706] section 14.2.5, on the receiving side during unmarshaling.
Review of attachment 282344 [details] [review]: makes sense to me. nitpick below ::: gdk/win32/gdkwindow-win32.c @@ +1807,3 @@ + + if (old_ptr == 0 && w32_error != NO_ERROR) + WIN32_API_FAILED ("GetWindowLongPtr"); as discussed on irc it is a bit tricky that in case of error we do not bail out, but it is intentional. Maybe it is clearer to write as: if (no error && old == parent) return; if (error) FAILED; /* failed to get the Ptr, just try to set it anyway */
Created attachment 282438 [details] [review] Suppress a warning from SetWindowLongPtr() v2: rearranged and commented as suggested
Attachment 282438 [details] pushed as d43fb29 - Suppress a warning from SetWindowLongPtr()
Hi, Just wondering, shall we push this patch, along with my other patch that I accidentally had at bug 733768 (my fault) to gtk-3-12, as the issue here hits all GTK+-3.x releases, AFAICT? Thanks, with blessings!
I guess we should. Who can authorize that?