Created attachment 208761 [details] [review]
prefs: Remove live-hidden-windows preference

The preference existed, even though it was hard-coded to true. Just
remove it for good.

Created attachment 208762 [details] [review]
prefs: Remove a dead declaration

Created attachment 208763 [details] [review]
Remove session saving stuff

It never worked properly, it's been broken for a while, and it's
probably never going to work again. Just remove it, rather than
keep the never-fixed code around.

Created attachment 208764 [details] [review]
Remove meta_compositor_update_remove_geometry

The callback is just dead code

Created attachment 208765 [details] [review]
theme-parser: Remove a bunch of dead code

Created attachment 208766 [details] [review]
Remove a bunch of dead code

Code isn't version control. We don't need bug links and commented out
code to remind us of how things were done originally.

Created attachment 209561 [details] [review]
meta-window-actor: Remove some unused constants

Created attachment 209562 [details] [review]
theme: Remove unused "widget" parameter to frame style drawing

It seems that the only usage of the "widget" parameter throughout
the entire call chain was to pass between two function calls as
mutual recursion.

Created attachment 209563 [details] [review]
theme: Remove unused entry points

meta_draw_op_draw, meta_draw_op_list_draw, and meta_frame_style_draw were
all unused

Review of attachment 208761 [details] [review]:

OK - don't really want to support two modes of operation if nobody is using one of them

Review of attachment 208762 [details] [review]:

Yep. (Patch is going to have to be rebased, don't need to see it again.)

Review of attachment 208763 [details] [review]:

I don't know of evidence for the statement "it never worked properly" - it probably worked as well as any XSMP based session saving could have. I don't want to remove this complex code without a comprehensive plan to entirely remove XSMP session management from GNOME.

Review of attachment 208764 [details] [review]:

meta_plugin_manager_update_workspace and meta_plugin_manager_update_workspaces( are also unused declarations then

Review of attachment 208765 [details] [review]:

The #if 0 of the check-expression was some temporary hack from the compositor branch that was accidentally merged? Would like to know more of the history and motivation for this.

Review of attachment 209561 [details] [review]:

Sure

Review of attachment 209562 [details] [review]:

OK

Review of attachment 209563 [details] [review]:

OK

Review of attachment 209562 [details] [review]:

Marked needs-work by accident

(In reply to comment #12)
> I don't know of evidence for the statement "it never worked properly" - it
> probably worked as well as any XSMP based session saving could have. I don't
> want to remove this complex code without a comprehensive plan to entirely
> remove XSMP session management from GNOME.

Yes please.

Attachment 208761 [details] pushed as 2d6555c - prefs: Remove live-hidden-windows preference
Attachment 208762 [details] pushed as 41adbdd - prefs: Remove a dead declaration
Attachment 208766 [details] pushed as 0d794f2 - Remove a bunch of dead code
Attachment 209561 [details] pushed as c0b4d68 - meta-window-actor: Remove some unused constants
Attachment 209562 [details] pushed as b58366d - theme: Remove unused "widget" parameter to frame style drawing
Attachment 209563 [details] pushed as 6900128 - theme: Remove unused entry points


Pushed with suggested changes, excepting theme-parser and session management.

I'll look into theme-parser.

Review of attachment 208766 [details] [review]:

I don't necessarily agree with the philosophy behind all the removals here even though I often say "code isn't version control" myself. What you never want is a "backwards looking" #if 0 - once you figure out how best to do something, there's no reason to keep the old code around. But a forward-looking #if 0 where there's an open bug, and a description o something that would be better, and some code that shows how it would work but it doesn't quite is fine.

Actually, looking through my comments in detail on the removals, it appears that even backwards looking #if 0's are OK if the old code is a plausible competitor - if there's some reason someone might want to try and reintroduce it.

::: src/core/async-getprop.c
@@ -422,3 @@
-            error.errorCode = BadImplementation;
-
-            _XError (dpy, &error);

I'm indifferent to or slightly against this removal - the #if 0'ed code here doesn't represent anything we'd want to do in the future, but it is quite useful explaining the comment above - in making it clear what else could be done other than storing in task->error.

::: src/core/delete.c
@@ -167,3 @@
-    {
-      /* FIXME Clean this up someday 
-       * http://bugzilla.gnome.org/show_bug.cgi?id=108706

I think this removal is fine, since we went to a never-restack-on-closing-windows policy so the whole thing is pretty much irrelevant.

::: src/core/display.c
@@ -1527,3 @@
-static void
-handle_net_restack_window (MetaDisplay* display,
-                           XEvent *event)

Here this removal doesn't make sense to me - it's illustrative of what we probably should be doing but don't - if someone filed a "Mutter doesn't handle _NET_RESTACK_WINDOW" bug, we'd want this code here to understand the situation.

@@ -3995,3 @@
-  /* FIXME:115072 */
-  /* Don't grab at all unless in click to focus mode. In click to
-   * focus, we may sometimes be clever about intercepting and eating

While what was left here as a comment isn't ideal - there's no actual FIXME here - the conclusion the bug was left at was basically "this is the best we can do between bug 115072 and bug 102209" - but I think having something here is important for showing that there are dragons here and preventing loops- if you want to change this and do something different for mouse focus, you really need to read through the bugzilla bugs.

::: src/core/stack.c
@@ -543,3 @@
-              /* old way of doing it */
-              if (!(meta_window_is_ancestor_of_transient (w, group_window)) &&
-                  !WINDOW_TRANSIENT_FOR_WHOLE_GROUP (group_window))  /* note */;/*note*/

while I don't want to figure this out right now, I think the comments and old code are useful in explaining why it's done the way it is.

::: src/core/testboxes.c
@@ -359,3 @@
-#if 0
-static void
-test_merge_regions ()

I *think* this is just some code that was written while the code in boxes.c was being developed and not a useful test harness, so OK to remove.

@@ -642,3 @@
-  meta_rectangle_region_to_string (region, ", ", tmp_list);
-  printf ("%s vs. %s\n", region_list, tmp_list);
-#endif

This is the kind of code you'd probably comment back in if you were trying to work on this code, so I don't think deleting it is appropriate.

::: src/tools/mutter-window-demo.c
@@ -469,3 @@
-static void
-changing_icon_cb (GtkAction           *action,
-                  gpointer             callback_data)

Fine

theme-parser comes from:

http://git.gnome.org/browse/metacity/commit/src/theme-parser.c?h=iains-blingtastic-bucket-o-bling&id=1e5ba27a532225d1532fddef50db55343d98e632

So, speed up fixes. Whether we still want them, I don't know.

Created attachment 213143 [details] [review]
frames: Remove expose_delayed

This was introduced for the effects API and wireframe mode, and was
forgotten when that went the way of the dinosaur.

Review of attachment 213143 [details] [review]:

Looks good

Attachment 213143 [details] pushed as fc87a63 - frames: Remove expose_delayed