Merge lp:~3v1n0/bamf/libreoffice-fixes into lp:bamf/0.4

[Marco Trevisan (Treviño)]

Fixed LibreOffice and OpenOffice compatibility, remapping windows to their real type

When a window can be mapped as a LibreOffice / OpenOffice application we look for the right hint for the given window, we set that hint by default (if valid) and we connect to the window's name-changed event.
So, every time the window name changes, we check again for the window type and if it has changed (i.e. we moved from writer to start-center), then we simulate a window open/close event that cause the window to be re-mapped to
the proper application.
This works very well and allows to change the application type on run-time.

Also, every recognized libreoffice window that doesn't match any main application, is now considered as a "Standard" libreoffice application window, using the libreoffice-startcenter icon.
This allows to correctly match not only the start center, but also dialog windows such as the restore window that shows up after a crash.
I also tried to introduce matching based on window class names (using libreoffice-writer, libreoffice-calc ... values), but since these parameters get updated only after the window name change, this check could be confusing (also if theoretically more secure).

The splash screen and the toolbars are now ignored, and so don't respect the rules above.

I've also included some optimizations, fixes (memleaks) and code cleanup.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Firstly - I LOVE you for getting rid of all the 'key = g_new (gint, 1);' that just gross in so many ways :-) Now for some comments:

8 - icon = g_icon_to_string (gicon);
9 +
10 + if (gicon)
11 + icon = g_icon_to_string (gicon);

The function you're updating here, bamf_application_setup_icon_and_name(), seems to leak self->priv->icon right at the end if it's set before we assign it. Can you plug that while you're here?

30 void
31 +bamf_legacy_screen_inject_window (BamfLegacyScreen *self, guint xid)

It's not totally clear to me what this function is intended to do. Can you add a short docstring? (I assume most ppl wont bother to dig out of the revision history like me ;-))

94 +void
95 +bamf_legacy_window_reopen (BamfLegacyWindow *self)

Same as the last new function :-)

101 + g_object_weak_ref (G_OBJECT (self), (GWeakNotify) handle_destroy_notify,
102 + GUINT_TO_POINTER (xid));

It's not very clear what the magic behind this weak ref is, could you add an explanatory comment?

373 + g_list_free_full (possible_apps, g_free);

The introduction of g_list_free_full() is nice, but requires glib >=2.28. Please add that check to configure.in.

423 + if (!g_hash_table_lookup (registered_pids, key))
424 + {
425 + g_hash_table_insert (registered_pids, key, g_strdup (window_hint));
426 + }

Only updating if we don't already know the pid seems like it could cause things to go askew? If that's not the case I think this logic requires a comment in the code.

628 static void
629 +bamf_matcher_dispose (GObject *object)

This function does not look like it's safe to run N times on the same object. That is the contract of dispose(). Fx. looking at this block makes me suspicious:

642 + g_array_free (priv->bad_prefixes, TRUE);
643 + g_array_free (priv->known_pids, TRUE);
644 + g_hash_table_destroy (priv->desktop_id_table);
645 + g_hash_table_destroy (priv->desktop_file_table);
646 + g_hash_table_destroy (priv->desktop_class_table);
647 + g_hash_table_destroy (priv->registered_pids);

You must add NULL-ification and NULL-checking on all members. If you don't want that override finalize() instead which is guaranteed to run only once.

695 + GObject *old_object = dbus_g_connection_lookup_g_object (bus, path);
696 + if (G_IS_OBJECT (old_object))
697 + {
698 + dbus_g_connection_unregister_g_object (bus, old_object);
699 + }

The commit message for this chunk mentions that you log a g_critical()when you replace an object. I don't see that critical being logged - is that intentional?

review: Needs Fixing

[lp:~3v1n0/bamf/libreoffice-fixes]

436. By Marco Trevisan (Treviño) on 2011-12-14

BamfApplication: don't leak the icon string

437. By Marco Trevisan (Treviño) on 2011-12-14

Configure.in: set glib-2.0 >= 2.28 to support g_list_free_full

438. By Marco Trevisan (Treviño) on 2011-12-14

BamfMatcher: use finalize function, instead of dispose.

439. By Marco Trevisan (Treviño) on 2011-12-14

BamfView: emit a g_critical message when unregistering a DBus object

440. By Marco Trevisan (Treviño) on 2011-12-14

BamfLegacyScreen: added docs to bamf_legacy_screen_inject_window

[Marco Trevisan (Treviño) (3v1n0)]

> Firstly - I LOVE you for getting rid of all the 'key = g_new (gint, 1);' that
> just gross in so many ways :-)

Yes, I'm sure that there are also some other leaks around, but I've not read everything yet. :)

Now for some comments:

> The function you're updating here, bamf_application_setup_icon_and_name(),
> seems to leak self->priv->icon right at the end if it's set before we assign
> it. Can you plug that while you're here?

Done. Free'd also on dispose.

> 30 void
> 31 +bamf_legacy_screen_inject_window (BamfLegacyScreen *self, guint xid)
>
> It's not totally clear to me what this function is intended to do. Can you add
> a short docstring? (I assume most ppl wont bother to dig out of the revision
> history like me ;-))

Done.

> 94 +void
> 95 +bamf_legacy_window_reopen (BamfLegacyWindow *self)
>
> Same as the last new function :-)

Done.

> 101 + g_object_weak_ref (G_OBJECT (self), (GWeakNotify)
> handle_destroy_notify,
> 102 + GUINT_TO_POINTER (xid));
>
> It's not very clear what the magic behind this weak ref is, could you add an
> explanatory comment?

Ok.

> 373 + g_list_free_full (possible_apps, g_free);
>
> The introduction of g_list_free_full() is nice, but requires glib >=2.28.
> Please add that check to configure.in.

Done.

> 423 + if (!g_hash_table_lookup (registered_pids, key))
> 424 + {
> 425 + g_hash_table_insert (registered_pids, key, g_strdup (window_hint));
> 426 + }
>
> Only updating if we don't already know the pid seems like it could cause
> things to go askew? If that's not the case I think this logic requires a
> comment in the code.

Well, this code was already there and I just kept the old behavior alive. However it seems to work well, also because generally a pid is always connected to just one application, except particular cases like this one (libreoffice or chromium).
However, I'll study more on this and eventually I'll change this code.

> 628 static void
> 629 +bamf_matcher_dispose (GObject *object)
>
> This function does not look like it's safe to run N times on the same object.
> That is the contract of dispose(). Fx.

Right, moving to finalize().

> The commit message for this chunk mentions that you log a g_critical()when you
> replace an object. I don't see that critical being logged - is that
> intentional?

Ehehe... Really it was there, but I don't know why I've not committed it.
Re-added! ;)

[lp:~3v1n0/bamf/libreoffice-fixes]

441. By Marco Trevisan (Treviño) on 2011-12-14

BamfLegacyWindow: added docs to bamf_legacy_window_reopen

442. By Marco Trevisan (Treviño) on 2011-12-15

BamfMatcher: correctly match the libreoffice transient windows.

We'll use the class name for matching them. This allows to be even more precise.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Ok, looking good now. Awesome work! Just one line to fix before I can approve this:

773 + object_class->dispose = bamf_matcher_finalize;

You mean

 + object_class->finalize = bamf_matcher_finalize;

:-)

review: Needs Fixing

[lp:~3v1n0/bamf/libreoffice-fixes]

443. By Marco Trevisan (Treviño) on 2011-12-15

BamfMatcher: fixing typo s/dispose/finalize/

[Mikkel Kamstrup Erlandsen (kamstrup)]

I am happy! :-D

review: Approve

=== modified file 'configure.in'
--- configure.in	2011-11-28 10:17:05 +0000
+++ configure.in	2011-12-15 11:35:15 +0000
@@ -56,7 +56,7 @@
 #
 # glib
 #
-PKG_CHECK_MODULES(GLIB, glib-2.0 >= 2.16.0 gio-2.0 gio-unix-2.0)
+PKG_CHECK_MODULES(GLIB, glib-2.0 >= 2.28.0 gio-2.0 gio-unix-2.0)
 
 #
 # dbus
 
=== modified file 'src/bamf-application.c'
--- src/bamf-application.c	2011-08-30 20:47:15 +0000
+++ src/bamf-application.c	2011-12-15 11:35:15 +0000
@@ -145,7 +145,9 @@
       gicon = g_app_info_get_icon (G_APP_INFO (desktop));
 
       name = g_strdup (g_app_info_get_display_name (G_APP_INFO (desktop)));
-      icon = g_icon_to_string (gicon);
+
+      if (gicon)
+        icon = g_icon_to_string (gicon);
 
       if (g_key_file_has_key(keyfile, G_KEY_FILE_DESKTOP_GROUP, STUB_KEY, NULL)) {
         /* This will error to return false, which is okay as it seems
@@ -227,7 +229,12 @@
     }
 
   if (icon)
-    self->priv->icon = icon;
+    {
+      if (self->priv->icon)
+        g_free (self->priv->icon);
+
+      self->priv->icon = icon;
+    }
 
   if (name)
     bamf_view_set_name (BAMF_VIEW (self), name);
@@ -611,6 +618,12 @@
       priv->app_type = NULL;
     }
 
+  if (priv->icon)
+    {
+      g_free (priv->icon);
+      priv->icon = NULL;
+    }
+
   if (priv->wmclass)
     {
       g_free (priv->wmclass);
 
=== modified file 'src/bamf-legacy-screen.c'
--- src/bamf-legacy-screen.c	2011-08-01 23:25:01 +0000
+++ src/bamf-legacy-screen.c	2011-12-15 11:35:15 +0000
@@ -160,6 +160,7 @@
 handle_window_opened (WnckScreen *screen, WnckWindow *window, BamfLegacyScreen *legacy)
 {
   BamfLegacyWindow *legacy_window;
+  g_return_if_fail (WNCK_IS_WINDOW (window));
 
   legacy_window = bamf_legacy_window_new (window);
 
@@ -171,6 +172,36 @@
   g_signal_emit (legacy, legacy_screen_signals[WINDOW_OPENED], 0, legacy_window);
 }
 
+/* This function allows to push into the screen a window by its xid.
+ * If the window is already known, it's just ignored, otherwise it gets added
+ * to the windows list. The BamfLegacyScreen should automatically update its
+ * windows list when they are added/removed from the screen, but if a child
+ * BamfLegacyWindow is closed, then it could be possible to re-add it.        */
+void
+bamf_legacy_screen_inject_window (BamfLegacyScreen *self, guint xid)
+{
+  g_return_if_fail (BAMF_IS_LEGACY_SCREEN (self));
+  BamfLegacyWindow *window;
+  GList *l;
+
+  for (l = self->priv->windows; l; l = l->next)
+    {
+      window = l->data;
+
+      if (bamf_legacy_window_get_xid (window) == xid)
+        {
+          return;
+        }
+    }
+
+  WnckWindow *legacy_window = wnck_window_get(xid);
+
+  if (WNCK_IS_WINDOW (legacy_window))
+    {
+      handle_window_opened(NULL, legacy_window, self);
+    }
+}
+
 void
 bamf_legacy_screen_set_state_file (BamfLegacyScreen *self,
                                    const char *file)
 
=== modified file 'src/bamf-legacy-screen.h'
--- src/bamf-legacy-screen.h	2010-05-25 13:47:38 +0000
+++ src/bamf-legacy-screen.h	2011-12-15 11:35:15 +0000
@@ -67,4 +67,6 @@
 
 BamfLegacyScreen * bamf_legacy_screen_get_default        (void);
 
+void               bamf_legacy_screen_inject_window      (BamfLegacyScreen *screen, guint xid);
+
 #endif
 
=== modified file 'src/bamf-legacy-window.c'
--- src/bamf-legacy-window.c	2011-11-29 08:04:22 +0000
+++ src/bamf-legacy-window.c	2011-12-15 11:35:15 +0000
@@ -328,13 +328,41 @@
 {
   g_return_if_fail (BAMF_IS_LEGACY_WINDOW (self));
   g_return_if_fail (WNCK_IS_WINDOW (window));
-  
+
+  if (self->priv->legacy_window == window)
+    {
+      self->priv->is_closed = TRUE;
+      g_signal_emit (self, legacy_window_signals[CLOSED], 0);
+    }
+}
+
+static void
+handle_destroy_notify (gpointer *data, BamfLegacyWindow *self_was_here)
+{
+  BamfLegacyScreen *screen = bamf_legacy_screen_get_default ();
+  bamf_legacy_screen_inject_window (screen, GPOINTER_TO_UINT (data));
+}
+
+/* This utility function allows to set a BamfLegacyWindow as closed, notifying
+ * all its owners, and to reopen it once the current window has been destroyed.
+ * This allows to remap particular windows to different applications.         */
+void
+bamf_legacy_window_reopen (BamfLegacyWindow *self)
+{
+  g_return_if_fail (BAMF_IS_LEGACY_WINDOW (self));
+  g_return_if_fail (WNCK_IS_WINDOW (self->priv->legacy_window));
+
+  guint xid = bamf_legacy_window_get_xid (self);
+
+  /* Adding a weak ref to this object, causes to get notified after the object
+   * destruction, so once this BamfLegacyWindow has been closed and drestroyed
+   * the handle_destroy_notify() function will be called, and that will
+   * provide to iniject another window like this one to the BamfLegacyScreen  */
+  g_object_weak_ref (G_OBJECT (self), (GWeakNotify) handle_destroy_notify,
+                                                    GUINT_TO_POINTER (xid));
+
   self->priv->is_closed = TRUE;
-
-  if (window == self->priv->legacy_window)
-    {
-      g_signal_emit (self, legacy_window_signals[CLOSED], 0);
-    }
+  g_signal_emit (self, legacy_window_signals[CLOSED], 0);
 }
 
 static void
@@ -381,7 +409,7 @@
   screen = wnck_screen_get_default ();
 
   priv->closed_id = g_signal_connect (G_OBJECT (screen), "window-closed",
-    		                       (GCallback) handle_window_closed, self);
+                                      (GCallback) handle_window_closed, self);
 }
 
 static void
 
=== modified file 'src/bamf-legacy-window.h'
--- src/bamf-legacy-window.h	2011-09-26 11:44:10 +0000
+++ src/bamf-legacy-window.h	2011-12-15 11:35:15 +0000
@@ -112,6 +112,8 @@
 
 BamfLegacyWindow * bamf_legacy_window_get_transient     (BamfLegacyWindow *self);
 
+void               bamf_legacy_window_reopen            (BamfLegacyWindow *self);
+
 BamfLegacyWindow * bamf_legacy_window_new               (WnckWindow *legacy_window);
 
 #endif
 
=== modified file 'src/bamf-matcher.c'
--- src/bamf-matcher.c	2011-09-29 20:31:13 +0000
+++ src/bamf-matcher.c	2011-12-15 11:35:15 +0000
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Jason Smith <[e-mail]>
+ *              Marco Trevisan (Treviño) <[e-mail]>
  *
  */
 
@@ -65,7 +66,6 @@
   GHashTable      * desktop_id_table;
   GHashTable      * desktop_file_table;
   GHashTable      * desktop_class_table;
-  GHashTable      * exec_list;
   GHashTable      * registered_pids;
   GList           * views;
   GList           * monitors;
@@ -135,12 +135,12 @@
     }
 }
 
-static void bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view);
+static void bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view, gboolean unref);
 
 static void
 on_view_closed (BamfView *view, BamfMatcher *self)
 {
-  bamf_matcher_unregister_view (self, view);
+  bamf_matcher_unregister_view (self, view, TRUE);
 }
 
 static void
@@ -168,7 +168,7 @@
 }
 
 static void
-bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view)
+bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view, gboolean unref)
 {
   const char * path;
   char * type;
@@ -181,8 +181,11 @@
   g_signal_handlers_disconnect_by_func (G_OBJECT (view), on_view_closed, self);
   g_signal_handlers_disconnect_by_func (G_OBJECT (view), on_view_active_changed, self);
 
-  self->priv->views = g_list_remove (self->priv->views, view);
-  g_object_unref (view);
+  if (unref)
+    {
+      self->priv->views = g_list_remove (self->priv->views, view);
+      g_object_unref (view);
+    }
 
   g_free (type);
 }
@@ -191,7 +194,7 @@
 get_open_office_window_hint (BamfMatcher * self, BamfLegacyWindow * window)
 {
   const gchar *name;
-  char *exec;
+  char *exec = NULL;
   GHashTable *desktopFileTable;
   GList *list;
 
@@ -199,18 +202,44 @@
   g_return_val_if_fail (BAMF_IS_LEGACY_WINDOW (window), NULL);
 
   name = bamf_legacy_window_get_name (window);
+  const gchar *class = bamf_legacy_window_get_class_name (window);
+  BamfWindowType type = bamf_legacy_window_get_window_type (window);
 
-  if (name == NULL)
+  if (name == NULL && class == NULL)
     return NULL;
 
-  if (g_str_has_suffix (name, "OpenOffice.org Writer"))
+  if (g_str_has_suffix (name, "LibreOffice Writer"))
+    {
+      exec = "libreoffice -writer %U";
+    }
+  else if (g_str_has_suffix (name, "LibreOffice Calc"))
+    {
+      exec = "libreoffice -calc %U";
+    }
+  else if (g_str_has_suffix (name, "LibreOffice Impress"))
+    {
+      exec = "libreoffice -impress %U";
+    }
+  else if (g_str_has_suffix (name, "LibreOffice Math"))
+    {
+      exec = "libreoffice -math %U";
+    }
+  else if (g_str_has_suffix (name, "LibreOffice Draw"))
+    {
+      exec = "libreoffice -draw %U";
+    }
+  else if (g_strcmp0 (class, "libreoffice-startcenter") == 0)
+    {
+      exec = "libreoffice %U";
+    }
+  else if (g_strcmp0 (name, "LibreOffice") == 0 && type == BAMF_WINDOW_NORMAL)
+    {
+      exec = "libreoffice %U";
+    }
+  else if (g_str_has_suffix (name, "OpenOffice.org Writer"))
     {
       exec = "ooffice -writer %F";
     }
-  else if (g_str_has_suffix (name, "OpenOffice.org Math"))
-    {
-      exec = "ooffice -math %F";
-    }
   else if (g_str_has_suffix (name, "OpenOffice.org Calc"))
     {
       exec = "ooffice -calc %F";
@@ -219,41 +248,63 @@
     {
       exec = "ooffice -impress %F";
     }
+  else if (g_str_has_suffix (name, "OpenOffice.org Math"))
+    {
+      exec = "ooffice -math %F";
+    }
   else if (g_str_has_suffix (name, "OpenOffice.org Draw"))
     {
       exec = "ooffice -draw %F";
     }
-  else if (g_str_has_suffix (name, "LibreOffice Writer"))
-    {
-      exec = "libreoffice -writer %U";
-    }
-  else if (g_str_has_suffix (name, "LibreOffice Math"))
-    {
-      exec = "libreoffice -math %U";
-    }
-  else if (g_str_has_suffix (name, "LibreOffice Calc"))
-    {
-      exec = "libreoffice -calc %U";
-    }
-  else if (g_str_has_suffix (name, "LibreOffice Impress"))
-    {
-      exec = "libreoffice -impress %U";
-    }
-  else if (g_str_has_suffix (name, "LibreOffice Draw"))
-    {
-      exec = "libreoffice -draw %U";
+  else if (g_strcmp0 (name, "OpenOffice.org") == 0 && type == BAMF_WINDOW_NORMAL)
+    {
+      exec = "ooffice %F";
     }
   else
     {
-      return NULL;
+      if (type != BAMF_WINDOW_NORMAL || bamf_legacy_window_get_transient (window))
+      {
+        /* Child windows can generally easily be recognized by their class */
+        if (g_strcmp0 (class, "libreoffice-writer") == 0)
+          {
+            exec = "libreoffice -writer %U";
+          }
+        else if (g_strcmp0 (class, "libreoffice-calc") == 0)
+          {
+            exec = "libreoffice -calc %U";
+          }
+        else if (g_strcmp0 (class, "libreoffice-impress") == 0)
+          {
+            exec = "libreoffice -impress %U";
+          }
+        else if (g_strcmp0 (class, "libreoffice-math") == 0)
+          {
+            exec = "libreoffice -math %U";
+          }
+        else if (g_strcmp0 (class, "libreoffice-draw") == 0)
+          {
+            exec = "libreoffice -draw %U";
+          }
+      }
+
+      if (!exec)
+        {
+          /* By default fallback to the main launcher */
+          if (g_str_has_prefix (class, "OpenOffice"))
+            {
+              exec = "ooffice %F";
+            }
+          else
+            {
+              exec = "libreoffice %U";
+            }
+        }
     }
 
   desktopFileTable = self->priv->desktop_file_table;
   list = g_hash_table_lookup (desktopFileTable, exec);
 
-  g_return_val_if_fail (list, NULL);
-
-  return (char *) list->data;
+  return (list ? (char *) list->data : NULL);
 }
 
 /* Attempts to return the binary name for a particular execution string */
@@ -266,7 +317,7 @@
   gboolean regexFail;
   GRegex *regex;
 
-  g_return_val_if_fail (execString && strlen (execString) > 0, g_strdup (execString));
+  g_return_val_if_fail ((execString && execString[0] != '\0'), g_strdup (execString));
 
   exec = g_utf8_casefold (execString, -1);
   parts = g_strsplit (exec, " ", 0);
@@ -996,7 +1047,7 @@
                                        self->priv->desktop_id_table,
                                        self->priv->desktop_class_table);
       
-              g_list_free_full (dirs, (GDestroyNotify) g_free);
+              g_list_free_full (dirs, g_free);
             }
         }
       else if (filetype != G_FILE_TYPE_UNKNOWN)
@@ -1098,15 +1149,18 @@
   fill_desktop_file_table (self, directories, *desktop_file_table,
                            *desktop_id_table, *desktop_class_table);
   
-  g_list_free_full (directories, (GDestroyNotify) g_free);
+  g_list_free_full (directories, g_free);
 }
 
 static gboolean
 is_open_office_window (BamfMatcher * self, BamfLegacyWindow * window)
 {
-  return g_str_has_prefix (bamf_legacy_window_get_class_name (window), "OpenOffice") ||
-         g_str_has_prefix (bamf_legacy_window_get_class_name (window), "LibreOffice") ||
-         g_str_has_prefix (bamf_legacy_window_get_class_name (window), "libreoffice");
+  const char *class_name = bamf_legacy_window_get_class_name (window);
+
+  return (g_str_has_prefix (class_name, "LibreOffice") ||
+          g_str_has_prefix (class_name, "libreoffice") ||
+          g_str_has_prefix (class_name, "OpenOffice") ||
+          g_str_has_prefix (class_name, "openoffice"));
 }
 
 static char *
@@ -1298,7 +1352,7 @@
       
       if (trimmed)
         {
-          if (strlen (trimmed) > 0)
+          if (trimmed[0] != '\0')
             {
               table_list = g_hash_table_lookup (priv->desktop_file_table, trimmed);
               
@@ -1371,7 +1425,7 @@
   hint = get_window_hint (self, window, _NET_WM_DESKTOP_FILE);
   const char *window_class = bamf_legacy_window_get_class_name (window);
 
-  if (hint && strlen (hint) > 0 && !is_web_app_window(self, window))
+  if (hint && hint[0] != '\0' && !is_web_app_window(self, window))
     {
       desktop_files = g_list_prepend (desktop_files, hint);
       /* whew, hard work, didn't even have to make a copy! */
@@ -1569,13 +1623,7 @@
       g_object_unref (best);
     }
 
-  for (l = possible_apps; l; l = l->next)
-    {
-      char *str = l->data;
-      g_free (str);
-    }
-
-  g_list_free (possible_apps);
+  g_list_free_full (possible_apps, g_free);
 
   bamf_view_add_child (BAMF_VIEW (best), BAMF_VIEW (bamf_window));
 }
@@ -1590,7 +1638,7 @@
   GHashTable *registered_pids;
   char *window_hint = NULL;
   gint i, pid;
-  gint *key;
+  gpointer key;
 
   g_return_if_fail (BAMF_IS_MATCHER (self));
   g_return_if_fail (BAMF_IS_LEGACY_WINDOW (window));
@@ -1604,9 +1652,7 @@
 
       if (pid > 0)
         {
-          key = g_new (gint, 1);
-          *key = pid;
-
+          gpointer key = GINT_TO_POINTER (pid);
           const char* result = g_hash_table_lookup (registered_pids, key);
           if (result && g_str_has_prefix (result, "/home/"))
             {
@@ -1618,48 +1664,45 @@
     }
 
   window_hint = get_window_hint (self, window, _NET_WM_DESKTOP_FILE);
-  if (window_hint && strlen (window_hint) > 0)
+  if (window_hint)
     {
-      /* already set, make sure we know about this
-       * fact for future windows of this applications */
-      pid = bamf_legacy_window_get_pid (window);
-
-      if (pid > 0)
+      if (window_hint[0] != '\0')
         {
-          key = g_new (gint, 1);
-          *key = pid;
+          /* already set, make sure we know about this
+           * fact for future windows of this applications */
+          pid = bamf_legacy_window_get_pid (window);
 
-          if (!g_hash_table_lookup (registered_pids, key))
+          if (pid > 0)
             {
-              g_hash_table_insert (registered_pids, key, g_strdup (window_hint));
+              key = GINT_TO_POINTER (pid);
+
+              if (!g_hash_table_lookup (registered_pids, key))
+                {
+                  g_hash_table_insert (registered_pids, key, g_strdup (window_hint));
+                }
             }
+
+          g_free (window_hint);
+          return;
         }
 
       g_free (window_hint);
-      return;
-    }
-
-  if (is_open_office_window (self, window))
-    {
-      window_hint = get_open_office_window_hint (self, window);
-    }
-  else
-    {
-      pids = pid_parent_tree (self, bamf_legacy_window_get_pid (window));
-
-      for (i = 0; i < pids->len; i++)
-        {
-          pid = g_array_index (pids, gint, i);
-
-          key = g_new (gint, 1);
-          *key = pid;
-
-          window_hint = g_hash_table_lookup (registered_pids, key);
-          if (window_hint != NULL && strlen (window_hint) > 0)
-            break;
-        }
-      g_array_free (pids, TRUE);
-    }
+      window_hint = NULL;
+    }
+
+  pids = pid_parent_tree (self, bamf_legacy_window_get_pid (window));
+
+  for (i = 0; i < pids->len; i++)
+    {
+      pid = g_array_index (pids, gint, i);
+      key = GINT_TO_POINTER (pid);
+
+      window_hint = g_hash_table_lookup (registered_pids, key);
+      if (window_hint != NULL && window_hint[0] != '\0')
+        break;
+    }
+
+  g_array_free (pids, TRUE);
 
   if (window_hint)
     set_window_hint (self, window, _NET_WM_DESKTOP_FILE, window_hint);
@@ -1690,24 +1733,31 @@
   bamf_matcher_setup_window_state (self, bamfwindow);
 }
 
-static gboolean
-open_office_window_setup_timer (OpenOfficeTimeoutArgs *args)
+static void
+on_open_office_window_name_changed (BamfLegacyWindow *window, BamfMatcher* self)
 {
-  if (bamf_legacy_window_is_closed (args->window))
-  {
-    g_object_unref (args->window);
-    return FALSE;
-  }
-
-  args->count++;
-  if (args->count > 20 || get_open_office_window_hint (args->matcher, args->window))  
+  g_return_if_fail (BAMF_IS_MATCHER (self));
+  g_return_if_fail (BAMF_IS_LEGACY_WINDOW (window));
+
+  char *old_hint;
+  const char *new_hint;
+
+  old_hint = get_window_hint (self, window, _NET_WM_DESKTOP_FILE);
+  new_hint = get_open_office_window_hint (self, window);
+
+  if (new_hint && g_strcmp0 (new_hint, old_hint) != 0)
     {
-      g_object_unref (args->window);
-      handle_raw_window (args->matcher, args->window);
-      return FALSE;
+      bamf_legacy_window_reopen (window);
     }
-  
-  return TRUE;
+
+  g_free (old_hint);
+}
+
+static void
+on_open_office_window_closed (BamfLegacyWindow *window, BamfMatcher* self)
+{
+  g_signal_handlers_disconnect_by_func (window, on_open_office_window_name_changed, self);
+  g_object_unref (window);
 }
 
 static void
@@ -1724,24 +1774,35 @@
       bamf_matcher_register_view (self, BAMF_VIEW (bamfwindow));
       g_object_unref (bamfwindow);
       
-      return;    
+      return;
     }
 
-  if (is_open_office_window (self, window) && get_open_office_window_hint (self, window) == NULL)
+  if (is_open_office_window (self, window))
     {
-      OpenOfficeTimeoutArgs* args = (OpenOfficeTimeoutArgs*) g_malloc0 (sizeof (OpenOfficeTimeoutArgs));
-      args->matcher = self;
-      args->window = window;
-      
+      BamfWindowType win_type = bamf_legacy_window_get_window_type (window);
+
+      if (win_type == BAMF_WINDOW_SPLASHSCREEN || win_type == BAMF_WINDOW_TOOLBAR)
+        {
+          return;
+        }
+
+      char *old_hint = get_window_hint (self, window, _NET_WM_DESKTOP_FILE);
+      const char *new_hint = get_open_office_window_hint (self, window);
+
+      if (new_hint && g_strcmp0 (old_hint, new_hint) != 0)
+        {
+          set_window_hint (self, window, _NET_WM_DESKTOP_FILE, new_hint);
+        }
+
       g_object_ref (window);
-      /* we have an open office window who is not ready to match yet */
-      g_timeout_add (100, (GSourceFunc) open_office_window_setup_timer, args);
-    }
-  else
-    {
-      /* we have a window who is ready to be matched */
-      handle_raw_window (self, window); 
-    }
+      g_signal_connect (window, "name-changed", (GCallback) on_open_office_window_name_changed, self);
+      g_signal_connect (window, "closed", (GCallback) on_open_office_window_closed, self);
+
+      g_free (old_hint);
+    }
+
+  /* we have a window who is ready to be matched */
+  handle_raw_window (self, window);
 }
 
 static void
@@ -1806,14 +1867,7 @@
         }
     }
 
- for (l = possible_apps; l; l = l->next)
-  {
-    char *str = l->data;
-    g_free (str);
-  }
-
-
-  g_list_free (possible_apps);
+  g_list_free_full (possible_apps, g_free);
 
   if (best)
     bamf_view_add_child (BAMF_VIEW (best), BAMF_VIEW (indicator));
@@ -1847,20 +1901,16 @@
                                             const char * desktopFile,
                                             gint pid)
 {
-  gint *key;
+  gpointer key;
   BamfLegacyScreen *screen;
   GList *glist_item;
   GList *windows;
-  char *dup;
 
   g_return_if_fail (BAMF_IS_MATCHER (self));
   g_return_if_fail (desktopFile);
 
-  key = g_new (gint, 1);
-  *key = pid;
-
-  dup = g_strdup (desktopFile);
-  g_hash_table_insert (self->priv->registered_pids, key, dup);
+  key = GINT_TO_POINTER (pid);
+  g_hash_table_insert (self->priv->registered_pids, key, g_strdup (desktopFile));
 
   /* fixme, this is a bit heavy */
 
@@ -2217,8 +2267,8 @@
 
   priv->known_pids = g_array_new (FALSE, TRUE, sizeof (gint));
   priv->bad_prefixes = g_array_new (FALSE, TRUE, sizeof (GRegex *));
-  priv->registered_pids =
-    g_hash_table_new ((GHashFunc) g_int_hash, (GEqualFunc) g_int_equal);
+  priv->registered_pids = g_hash_table_new_full (g_direct_hash, g_direct_equal,
+                                                 NULL, g_free);
 
   prefixstrings = prefix_strings (self);
   for (i = 0; i < prefixstrings->len; i++)
@@ -2252,13 +2302,61 @@
 }
 
 static void
+bamf_matcher_finalize (GObject *object)
+{
+  BamfMatcher *self = (BamfMatcher *) object;
+  BamfMatcherPrivate *priv = self->priv;
+  GList *l;
+  int i;
+
+  for (i = 0; i < priv->bad_prefixes->len; i++)
+    {
+      GRegex *regex = g_array_index (priv->bad_prefixes, GRegex *, i);
+      g_regex_unref (regex);
+    }
+
+  g_array_free (priv->bad_prefixes, TRUE);
+  g_array_free (priv->known_pids, TRUE);
+  g_hash_table_destroy (priv->desktop_id_table);
+  g_hash_table_destroy (priv->desktop_file_table);
+  g_hash_table_destroy (priv->desktop_class_table);
+  g_hash_table_destroy (priv->registered_pids);
+
+  for (l = priv->views; l; l = l->next)
+    {
+      bamf_matcher_unregister_view (self, (BamfView*)l->data, FALSE);
+    }
+  g_list_free_full (priv->views, g_object_unref);
+  priv->views = NULL;
+
+  for (l = priv->monitors; l; l = l->next)
+    {
+      g_signal_handlers_disconnect_by_func (G_OBJECT (l->data), 
+                                            (GCallback) on_monitor_changed, self);
+    }
+  g_list_free_full (priv->monitors, g_object_unref);
+  priv->monitors = NULL;
+
+  g_list_free_full (priv->favorites, g_free);
+  priv->favorites = NULL;
+
+  priv->active_app = NULL;
+  priv->active_win = NULL;
+
+  G_OBJECT_CLASS (bamf_matcher_parent_class)->finalize (object);
+}
+
+static void
 bamf_matcher_class_init (BamfMatcherClass * klass)
 {
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
   g_type_class_add_private (klass, sizeof (BamfMatcherPrivate));
 
   dbus_g_object_type_install_info (BAMF_TYPE_MATCHER,
                                    &dbus_glib_bamf_matcher_object_info);
 
+  object_class->finalize = bamf_matcher_finalize;
+
   matcher_signals [VIEW_OPENED] =
     g_signal_new ("view-opened",
                   G_OBJECT_CLASS_TYPE (klass),
@@ -2294,7 +2392,7 @@
                   bamf_marshal_VOID__STRING_STRING,
                   G_TYPE_NONE, 2,
                   G_TYPE_STRING, G_TYPE_STRING);
-                  
+ 
   matcher_signals [FAVORITES_CHANGED] =
     g_signal_new ("favorites-changed",
                   G_OBJECT_CLASS_TYPE (klass),
 
=== modified file 'src/bamf-view.c'
--- src/bamf-view.c	2011-11-29 08:06:28 +0000
+++ src/bamf-view.c	2011-12-15 11:35:15 +0000
@@ -470,8 +470,14 @@
 
       g_return_val_if_fail (bus, NULL);
 
+      GObject *old_object = dbus_g_connection_lookup_g_object (bus, path);
+      if (G_IS_OBJECT (old_object))
+      {
+        g_critical ("BAMF has already registered an object on path \"%s`, this should never happen!", path);
+        dbus_g_connection_unregister_g_object (bus, old_object);
+      }
+
       dbus_g_connection_register_g_object (bus, path, G_OBJECT (view));
-
       g_signal_emit (view, view_signals[EXPORTED], 0);
     }