GNOME Bugzilla – Bug 620899
Fix ShellAppSystem's use of no_focus_window, clean up state handling
Last modified: 2010-06-17 19:22:55 UTC
First, we were passing an incorrect timestamp to meta_display_focus_the_no_focus_window - fix that. The invocation of set_focus_app to the started app there couldn't really work, because (if the above call had worked) we'd get the X reply *after* the started app. What we need to untangle here is the distinction that's now made in ShellApp between _STATE_STARTING and _STATE_RUNNING. A nice way to start doing this is to rebase ShellWindowTracker to only be concerned with app states. Concretely, the current "has windows implies running" logic now lives just inside shell-app.c. Rename the app-running-changed signal to be app-state-changed. This will ultimately be useful so that inside the panel, we can track the last started app.
Created attachment 162972 [details] [review] Fix ShellAppSystem's use of no_focus_window, clean up state handling
Created attachment 162976 [details] [review] Fix ShellAppSystem's use of no_focus_window, clean up state handling Minor fix: remove duplicated removal of running app
Created attachment 163040 [details] [review] correct app state handling app should not change SHELL_APP_STATE_STARTING state, when window add. If doesn't have active window, It use app from active startup sequence.
Review of attachment 162976 [details] [review]: Patch looks good to me, except that it seems incorrectly split out of a larger patch ::: src/Makefile.am @@ +118,3 @@ shell-embedded-window-private.h \ + shell-global-private.h \ + shell-window-tracker-private.h Missing ::: src/shell-app.c @@ +634,3 @@ g_object_notify (G_OBJECT (app), "state"); + + _shell_window_tracker_notify_app_state_changed (shell_window_tracker_get_default (), app); No #include of shell-widow-tracker-private.h ::: src/shell-window-tracker.c @@ +65,3 @@ GObject parent; + ShellApp *last_starting_app; Stray
Attachment 162976 [details] pushed as e4a6bf9 - Fix ShellAppSystem's use of no_focus_window, clean up state handling
Oops, still one more patch here, I'll get to it soon.
Created attachment 163280 [details] [review] correct app state handling merge with HEAD
Review of attachment 163280 [details] [review]: ::: src/shell-app.c @@ +694,1 @@ shell_app_state_transition (app, SHELL_APP_STATE_RUNNING); I'm very confused by this. The old check was basically a no-op, because _state_transition does: if (app->state == state) return; What you've added here will *stop* us from going from STARTING->RUNNING when adding a window, right? Which seems broken. ::: src/shell-window-tracker.c @@ +66,2 @@ GObject parent; + ShellApp *last_starting_app; Can this live in panel.js? It'd be cleaner I think. Basically if we don't have an X window focus, use the last app that changed state to STARTING.
Review of attachment 163280 [details] [review]: ::: src/shell-app.c @@ +694,1 @@ shell_app_state_transition (app, SHELL_APP_STATE_RUNNING); I think, App is RUNNING only after startup sequence complete (For example: gimp show splashscreen). ::: src/shell-window-tracker.c @@ +66,2 @@ GObject parent; + ShellApp *last_starting_app; Why? I think, it is better hide startup sequences(remove startup-sequence-changed signal, ...).