Merge lp:~mkhu/compiz/compiz.fix_1075451 into lp:compiz/0.9.9

[Micheal Hsu]

For an application window without decorations, pressing Alt+Space failed to activate the action menu on the top left corner of the window and also causes the gtk-window-decorator to crash.
(LP: #1075451)

The crash is caused by a miscalculation in actionmenu.c: position_action_menu(...)
function (*theme_get_button_position) is not suitable for windows without decorations.
so first we need to check if the window is decorated or not.
most (non-overrided) windows use _MOTIF_WM_HINTS to indicate they're not to be decorated.
thus get_mwm_prop(...) in util.c is quite handy.

NOTE: (*theme_get_button_position) is actually get_button_position(...) in cairo.c

[Daniel van Vugt (vanvugt)]

Excellent, thanks.

Please:
1. Log a bug for the crash and link the branch to the bug.
2. Change the indentation to 4 spaces (not tabs except every second level to replace 8-spaces).
3. Propose for merging to lp:compiz only. That's 0.9.9

review: Needs Resubmitting

[Daniel van Vugt (vanvugt)]

Yep, looks correct in theory and no obvious regressions. I still can't reproduce the crash myself though.

review: Approve

[Sam Spilsbury (smspillaz)]

Possibly better would be to use

if (d->decorated)
{
    ...
}

get_mwm_prop involves a round-trip, and is not always a guaruntee that the window is actually decorated.

[Daniel van Vugt (vanvugt)]

Sam's got a good point. Needs Information until changed or proven otherwise.

review: Needs Information

[Micheal Hsu (mkhu)]

> Sam's got a good point. Needs Information until changed or proven otherwise.

Today, I checkout lp:compiz and now I can reproduce the bug.

previously I forgot to mention that I build compiz with following cmake option:
BUILD_KDE=off, BUILD_METACITY=off (set by ccmake)
in this way, gtk-window-decoration uses functions provided by cairo.c

run gtk-window-decorator within gdb a tty(after set relevant env variables.
DISPLAY=:0
LD_LIBRARY_PATH=/home/hooke/GIT/compizinstall/lib (my compiz install directory).

after to change to tty7.
pressing Alt+Space on a number of applications(e.g. nautilus), an action menu shows up.
pressing Alt+Space on google-chrome, no action menu showup, gtk-window-decorator crashes.

here is the log file gdb.txt:
///////////////////////////////////////////////////////////////////
Starting program: /home/hooke/GIT/compizinstall/bin/gtk-window-decorator
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7fffefa6a700 (LWP 29231)]
[New Thread 0x7fffef269700 (LWP 29233)]

Program received signal SIGSEGV, Segmentation fault.
0x0000000000410f20 in get_button_position (d=0x76c380, i=3, width=1222, height=580, x=0x7fffffffdda0, y=0x7fffffffdda4,
    w=0x7fffffffdda8, h=0x7fffffffddac) at /home/hooke/GIT/compiz/gtk/window-decorator/cairo.c:852
852 (d->frame->titlebar_height - 17);
#0 0x0000000000410f20 in get_button_position (d=0x76c380, i=3, width=1222, height=580, x=0x7fffffffdda0, y=0x7fffffffdda4,
    w=0x7fffffffdda8, h=0x7fffffffddac) at /home/hooke/GIT/compiz/gtk/window-decorator/cairo.c:852
#1 0x00000000004172ec in position_action_menu (menu=0x77a2c0, x=0x7fffffffde18, y=0x7fffffffde1c, push_in=0x77a418,
    user_data=0x7611a0) at /home/hooke/GIT/compiz/gtk/window-decorator/actionmenu.c:44
#2 0x00007ffff6f57024 in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#3 0x00007ffff6f5954b in gtk_menu_popup () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#4 0x000000000041752e in action_menu_map (win=0x7611a0, button=0, time=21881723)
    at /home/hooke/GIT/compiz/gtk/window-decorator/actionmenu.c:117
#5 0x0000000000416078 in event_filter_func (gdkxevent=0x7fffffffe290, event=0x785d90, data=0x0)
    at /home/hooke/GIT/compiz/gtk/window-decorator/events.c:1069
#6 0x00007ffff6bbfe26 in ?? () from /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#7 0x00007ffff6bc1bd6 in ?? () from /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#8 0x00007ffff6bc1c7e in ?? () from /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
#9 0x00007ffff5b14d53 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007ffff5b150a0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007ffff5b1549a in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00007ffff6f4c2f7 in gtk_main () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#13 0x000000000040d9d4 in main (argc=1, argv=0x7fffffffe688)
    at /home/hooke/GIT/compiz/gtk/window-decorator/gtk-window-decorator.c:410
A debugging session is active.

 Inferior 1 [process 29228] will be killed.

Quit anyway? (y or n)
///////////////////////////////////...

[Daniel van Vugt (vanvugt)]

Yes, certainly please rewrite using the approach Sam mentioned.

review: Needs Fixing

Unmerged revisions

3452. By Hu Kang <email address hidden> on 2012-11-06

fix gtk-window-decorator crash when pressing Alt+Space on windows without decorations (e.g. google-chrome)

=== modified file 'gtk/window-decorator/actionmenu.c'
--- gtk/window-decorator/actionmenu.c	2011-05-07 08:58:10 +0000
+++ gtk/window-decorator/actionmenu.c	2012-11-06 08:02:35 +0000
@@ -41,9 +41,12 @@
 
     wnck_window_get_client_window_geometry (win, x, y, &width, &height);
 
-    if ((*theme_get_button_position) (d, BUTTON_MENU, width, height,
+    if (get_mwm_prop (wnck_window_get_xid (win)))
+    {
+	if ((*theme_get_button_position) (d, BUTTON_MENU, width, height,
 				      &bx, &by, &width, &height))
-	*x = *x - frame->win_extents.left + bx;
+	    *x = *x - frame->win_extents.left + bx;
+    }
 
     gwd_decor_frame_unref (frame);