Code review comment for lp://qastaging/~ev/compiz/call-apport-on-hangs

this looks okay, although the design feels a bit weird to me.

145 +#ifdef USE_APPORT
146 + Time serverTime;
147 +
148 + if (!window->alive () && !forceQuitTriggered) {
149 + serverTime = screen->getCurrentTime ();
150 + screen->toolkitAction (Atoms::toolkitActionForceQuitDialog, serverTime, window->id (), true, 0, 0);
151 + forceQuitTriggered = true;
152 + }
153 +#endif
154 + if (window->alive ())
155 + forceQuitTriggered = false;
156
157 value = fScreen->optionGetUnresponsiveBrightness ();
158 if (value != 100)
159 @@ -361,7 +375,8 @@
160 fadeTime (0),
161 opacityDiff (0),
162 brightnessDiff (0),
163 - saturationDiff (0)
164 + saturationDiff (0),
165 + forceQuitTriggered (false)

Currently it pops up the apport dialog as soon as the application becomes unresponsive. There are quite a few cases where applications can become temporarily unresponsive and it isn't a bug because the system is just under really heavy load and the UI thread of every application is starved. It doesn't make sense to report those.

I noticed that because of this, you need to have checks in both gtk-window-decorator and in the fade plugin to see if the apport dialog has already come up.

What I'd suggest is just showing the dialog when the force quit dialog normally comes up - except it might be better to add a checkbox which says "report this unresponsive application"

Then you can make the code slightly more modular too by delegating the call through a standard interface that all reporters might accept.

This way you only have check if the apport dialog has appeared once, because the user will be closing and reporting the hang at the same time.

Also, some indentation notes:

72 + app = wnck_window_get_application (win);
73 + if (!app)
74 + return FALSE;

You need a one-tab indent for the return FALSE;

76 + pid = wnck_application_get_pid (app);
77 + if (d->apport_dialog) {
78 + /* Is Apport already running? */
79 + if (kill (d->apport_dialog, 0) >= 0 && errno != ESRCH)
80 + return TRUE;
81 + }

pid is indented incorrectly, the brace should fall on the next line, return TRUE needs an indent (4 spaces)

85 + if (error) {
86 + g_printerr ("Could not spawn apport-gtk: %s\n", error->message);
87 + g_error_free (error);
88 + }

brace needs to fall on the next line.