GNOME Bugzilla – Bug 644252
Add MUTTER_WM_CLASS_FILTER environment variable
Last modified: 2011-03-18 18:57:18 UTC
Here's a patch that I'm using to make the GNOME Shell perf tests run with a predictable set of windows, rather than an undefined set of windows. Items of question: - Is doing it an environment variable right as compared to a command line option or an API? I liked the environment variable because it made this patch really confined and it didn't pollute the --help output with something that's a development facility, but it does have the issue that it could leak into child processes. - Should we make an attempt to unhide windows on exit? This approach where we just make sure that they have a NormalState WM_STATE property and let the next WM remap them works well if the next WM is Metacity or Mutter, and *probably* in generall, though I haven't checked, and is more crash and abnormal-exit resistant than anything more explicit.
Created attachment 182890 [details] [review] Add MUTTER_WM_CLASS_FILTER environment variable In a performance or regression testing environment, we may want to only manage windows from a particular test program, and ignore all other windows. The MUTTER_WM_CLASS_FILTER environment variable is a list of WM_CLASS values that should be managed; other windows will be unmapped and ignored.
Reassigning to Colin for review since I'm giving him the gnome-shell bugs that depend on this.
(In reply to comment #0) > - Is doing it an environment variable right as compared to a command > line option or an API? I liked the environment variable because it made > this patch really confined and it didn't pollute the --help output > with something that's a development facility, but it does have the issue > that it could leak into child processes. I'd go with the environment variable for the same reasons you give; I don't think leaking into child processes is that significant for a debug feature like this.
Review of attachment 182890 [details] [review]: ::: src/core/window.c @@ +546,3 @@ + const char *filter_string = g_getenv ("MUTTER_WM_CLASS_FILTER"); + if (filter_string) + filter_wm_classes = g_strsplit(filter_string, ",", -1); Space after identifier. @@ +548,3 @@ + filter_wm_classes = g_strsplit(filter_string, ",", -1); + else + filter_wm_classes = g_strsplit("", ",", -1); Kind of lame to have a permanent malloc()'d block that's never touched. g_once_init() seems better for this sort of thing. @@ +557,3 @@ + + meta_error_trap_push (display); + success = XGetClassHint(display->xdisplay, xwindow, &class_hint); Space @@ +597,3 @@ + + /* Make sure filtered windows are hidden from view */ + XUnmapWindow(display->xdisplay, xwindow); Space @@ +670,3 @@ + { + meta_verbose ("Not managing filtered window\n"); + return NULL; It looks like in the case of a MapNotify, we will end up passing the event down into clutter still (meta_compositor_process_event -> clutter_x11_handle_event). Ok, that's probably harmless because in this case we never call meta_compositor_add_window(). But I do wonder if for sanity's sake we should explicitly return out of display.c:event_callback and not call meta_compositor_process_event()
(In reply to comment #4) > Review of attachment 182890 [details] [review]: > > ::: src/core/window.c > @@ +546,3 @@ > + const char *filter_string = g_getenv ("MUTTER_WM_CLASS_FILTER"); > + if (filter_string) > + filter_wm_classes = g_strsplit(filter_string, ",", -1); > > Space after identifier. > > @@ +548,3 @@ > + filter_wm_classes = g_strsplit(filter_string, ",", -1); > + else > + filter_wm_classes = g_strsplit("", ",", -1); > > Kind of lame to have a permanent malloc()'d block that's never touched. > g_once_init() seems better for this sort of thing. Probably cheaper than another static == relocation, but I've added a 'gboolean initialized'. Don't really see using g_once_init() in non-threaded situation. > @@ +557,3 @@ > + > + meta_error_trap_push (display); > + success = XGetClassHint(display->xdisplay, xwindow, &class_hint); > > Space > > @@ +597,3 @@ > + > + /* Make sure filtered windows are hidden from view */ > + XUnmapWindow(display->xdisplay, xwindow); > > Space > > @@ +670,3 @@ > + { > + meta_verbose ("Not managing filtered window\n"); > + return NULL; > > It looks like in the case of a MapNotify, we will end up passing the event down > into clutter still (meta_compositor_process_event -> clutter_x11_handle_event). > > Ok, that's probably harmless because in this case we never call > meta_compositor_add_window(). But I do wonder if for sanity's sake we should > explicitly return out of display.c:event_callback and not call > meta_compositor_process_event() I don't think so. The basic philosophy of event handling is that we pass all events to the compositor except for stuff that is "once handled" like key presses. For stuff that's just observed like MapNotify events, it should get passed through. Will push with the gboolean initializd and the added spaces.
Attachment 182890 [details] pushed as 9043191 - Add MUTTER_WM_CLASS_FILTER environment variable