Merge lp:~3v1n0/bamf/safe-win-reopen-on-new-desktop into lp:bamf/0.4

[Marco Trevisan (Treviño)]

BamfMatcher: don't reopen windows when iterating on view's list, add tests

Fixed a crash that happened on reopening bamf windows when a new desktop file is added to a monitored folder, LegacyWindow and LegacyScreen slightly refactored and improved to allow matcher testing.

Added first real unit tests for the matcher.

[lp:~3v1n0/bamf/safe-win-reopen-on-new-desktop]

500. By Marco Trevisan (Treviño) on 2012-07-30

test-matcher: fix data_dir path

501. By Marco Trevisan (Treviño) on 2012-07-30

BamfLegacyScreen: avoid close event duplication

[Michal Hruby (mhr3)]

243 g_signal_emit_by_name (self, "closed");
244 + self->is_closed = TRUE;

Shouldn't the is_closed be set before emitting the signal? That way calling the .is_closed method will return proper value even inside the handler.

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

Yes, true.

[lp:~3v1n0/bamf/safe-win-reopen-on-new-desktop]

502. By Marco Trevisan (Treviño) on 2012-07-31

BamfLegacyWindowTest: set the window as closed before emitting the signal

[Michal Hruby (mhr3)]

Looks fine, tests pass here...

review: Approve

=== modified file 'src/bamf-application.c'
--- src/bamf-application.c	2012-07-17 19:03:36 +0000
+++ src/bamf-application.c	2012-07-31 15:13:24 +0000
@@ -322,7 +322,7 @@
   const char *window_class = bamf_legacy_window_get_class_name (window);
   const char *instance_name = bamf_legacy_window_get_class_instance_name (window);
 
-  if (!window_class || !instance_name)
+  if (!window_class && !instance_name)
     return FALSE;
 
   children = bamf_view_get_children (BAMF_VIEW (self));

=== added file 'src/bamf-legacy-screen-private.h'
--- src/bamf-legacy-screen-private.h	1970-01-01 00:00:00 +0000
+++ src/bamf-legacy-screen-private.h	2012-07-31 15:13:24 +0000
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ *
+ */
+
+#ifndef __BAMF_LEGACY_SCREEN_PRIVATE_H__
+#define __BAMF_LEGACY_SCREEN_PRIVATE_H__
+
+#include "bamf-legacy-screen.h"
+#include "bamf-legacy-window-test.h"
+
+void _bamf_legacy_screen_open_test_window (BamfLegacyScreen *self, BamfLegacyWindowTest *window);
+void _bamf_legacy_screen_close_test_window (BamfLegacyScreen *self, BamfLegacyWindowTest *window);
+
+#endif

=== modified file 'src/bamf-legacy-screen.c'
--- src/bamf-legacy-screen.c	2012-01-10 17:48:03 +0000
+++ src/bamf-legacy-screen.c	2012-07-31 15:13:24 +0000
@@ -18,13 +18,15 @@
  */
 
 #include "bamf-legacy-screen.h"
-#include "bamf-legacy-window-test.h"
+#include "bamf-legacy-screen-private.h"
 #include <gio/gio.h>
 
 G_DEFINE_TYPE (BamfLegacyScreen, bamf_legacy_screen, G_TYPE_OBJECT);
 #define BAMF_LEGACY_SCREEN_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE(obj, \
 BAMF_TYPE_LEGACY_SCREEN, BamfLegacyScreenPrivate))
 
+static BamfLegacyScreen *static_screen = NULL;
+
 enum
 {
   WINDOW_OPENED,
@@ -100,14 +102,8 @@
       class = parts[3];
       exec  = parts[4];
 
-      window = BAMF_LEGACY_WINDOW (bamf_legacy_window_test_new (xid, name, class, exec));
-      self->priv->windows = g_list_append (self->priv->windows, window);
-      g_signal_emit (window, legacy_screen_signals[STACKING_CHANGED], 0);
-
-      g_signal_connect (G_OBJECT (window), "closed",
-                        (GCallback) handle_window_closed, self);
-
-      g_signal_emit (self, legacy_screen_signals[WINDOW_OPENED], 0, window);
+      BamfLegacyWindowTest *test_win = bamf_legacy_window_test_new (xid, name, class, exec);
+      _bamf_legacy_screen_open_test_window (self, test_win);
     }
   else if (g_strcmp0 (parts[0], "close") == 0 && parts_size == 2)
     {
@@ -116,7 +112,7 @@
           window = l->data;
           if (bamf_legacy_window_get_xid (window) == xid)
             {
-              bamf_legacy_window_test_close (BAMF_LEGACY_WINDOW_TEST (window));
+              _bamf_legacy_screen_close_test_window (self, BAMF_LEGACY_WINDOW_TEST (window));
               break;
             }
         }
@@ -382,9 +378,22 @@
 }
 
 static void
-bamf_legacy_screen_dispose (GObject *object)
+bamf_legacy_screen_finalize (GObject *object)
 {
-  G_OBJECT_CLASS (bamf_legacy_screen_parent_class)->dispose (object);
+  BamfLegacyScreen *self = BAMF_LEGACY_SCREEN (object);
+
+  if (self->priv->windows)
+    g_list_free_full (self->priv->windows, g_object_unref);
+
+  if (self->priv->file)
+    g_object_unref (self->priv->file);
+
+  if (self->priv->stream)
+    g_object_unref (self->priv->stream);
+
+  static_screen = NULL;
+
+  G_OBJECT_CLASS (bamf_legacy_screen_parent_class)->finalize (object);
 }
 
 static void
@@ -398,7 +407,7 @@
 {
   GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
-  object_class->dispose      = bamf_legacy_screen_dispose;
+  object_class->finalize = bamf_legacy_screen_finalize;
 
   g_type_class_add_private (klass, sizeof (BamfLegacyScreenPrivate));
 
@@ -441,15 +450,16 @@
                   G_TYPE_NONE, 0);
 }
 
-static BamfLegacyScreen *self = NULL;
-
 BamfLegacyScreen *
 bamf_legacy_screen_get_default ()
 {
-  if (self)
-    return self;
+  BamfLegacyScreen *self;
+
+  if (static_screen)
+    return static_screen;
 
   self = (BamfLegacyScreen *) g_object_new (BAMF_TYPE_LEGACY_SCREEN, NULL);
+  static_screen = self;
 
   self->priv->legacy_screen = wnck_screen_get_default ();
 
@@ -462,5 +472,46 @@
   g_signal_connect (G_OBJECT (self->priv->legacy_screen), "active-window-changed",
                     (GCallback) handle_active_window_changed, self);
 
-  return self;
+  return static_screen;
+}
+
+
+// Private functions for testing purposes
+
+void _bamf_legacy_screen_open_test_window (BamfLegacyScreen *self, BamfLegacyWindowTest *test_window)
+{
+  GList *l;
+  BamfLegacyWindow *window;
+  guint xid;
+
+  g_return_if_fail (BAMF_IS_LEGACY_SCREEN (self));
+  g_return_if_fail (BAMF_IS_LEGACY_WINDOW_TEST (test_window));
+
+  window = BAMF_LEGACY_WINDOW (test_window);
+  xid = bamf_legacy_window_get_xid (window);
+
+  for (l = self->priv->windows; l; l = l->next)
+    {
+      if (bamf_legacy_window_get_xid (BAMF_LEGACY_WINDOW (l->data)) == xid)
+        {
+          return;
+        }
+    }
+
+  self->priv->windows = g_list_append (self->priv->windows, window);
+  g_signal_emit (self, legacy_screen_signals[STACKING_CHANGED], 0);
+
+  g_signal_connect (G_OBJECT (window), "closed",
+                    (GCallback) handle_window_closed, self);
+
+  g_signal_emit (self, legacy_screen_signals[WINDOW_OPENED], 0, window);
+}
+
+void _bamf_legacy_screen_close_test_window (BamfLegacyScreen *self, BamfLegacyWindowTest *test_window)
+{
+  g_return_if_fail (BAMF_IS_LEGACY_SCREEN (self));
+  g_return_if_fail (BAMF_IS_LEGACY_WINDOW_TEST (test_window));
+
+  // This will cause handle_window_closed to be called
+  bamf_legacy_window_test_close (BAMF_LEGACY_WINDOW_TEST (test_window));
 }

=== modified file 'src/bamf-legacy-window-test.c'
--- src/bamf-legacy-window-test.c	2012-01-10 18:01:52 +0000
+++ src/bamf-legacy-window-test.c	2012-07-31 15:13:24 +0000
@@ -18,6 +18,7 @@
  */
 
 #include "bamf-legacy-window-test.h"
+#include "bamf-legacy-screen-private.h"
 
 G_DEFINE_TYPE (BamfLegacyWindowTest, bamf_legacy_window_test, BAMF_TYPE_LEGACY_WINDOW);
 
@@ -147,13 +148,23 @@
 }
 
 static const char *
-bamf_legacy_window_test_get_class (BamfLegacyWindow *legacy_window)
-{
-  BamfLegacyWindowTest *self;
-
-  self = BAMF_LEGACY_WINDOW_TEST (legacy_window);
-
-  return  self->klass;
+bamf_legacy_window_test_get_class_name (BamfLegacyWindow *legacy_window)
+{
+  BamfLegacyWindowTest *self;
+
+  self = BAMF_LEGACY_WINDOW_TEST (legacy_window);
+
+  return  self->wm_class_name;
+}
+
+static const char *
+bamf_legacy_window_test_get_class_instance_name (BamfLegacyWindow *legacy_window)
+{
+  BamfLegacyWindowTest *self;
+
+  self = BAMF_LEGACY_WINDOW_TEST (legacy_window);
+
+  return  self->wm_class_instance;
 }
 
 char *
@@ -229,6 +240,7 @@
 void
 bamf_legacy_window_test_close (BamfLegacyWindowTest *self)
 {
+  self->is_closed = TRUE;
   g_signal_emit_by_name (self, "closed");
 }
 
@@ -280,6 +292,42 @@
   self->dbus_menu_object_path = g_strdup (object_path);
 }
 
+gboolean
+bamf_legacy_window_test_is_closed (BamfLegacyWindow *window)
+{
+  g_return_val_if_fail (BAMF_IS_LEGACY_WINDOW_TEST (window), TRUE);
+
+  BamfLegacyWindowTest *self = BAMF_LEGACY_WINDOW_TEST (window);
+  return self->is_closed;
+}
+
+static void
+handle_destroy_notify (BamfLegacyWindowTest *copy, BamfLegacyWindowTest *self_was_here)
+{
+  BamfLegacyScreen *screen = bamf_legacy_screen_get_default ();
+  _bamf_legacy_screen_open_test_window (screen, copy);
+}
+
+void
+bamf_legacy_window_test_reopen (BamfLegacyWindow *window)
+{
+  g_return_if_fail (BAMF_IS_LEGACY_WINDOW_TEST (window));
+
+  BamfLegacyWindowTest *self = BAMF_LEGACY_WINDOW_TEST (window);
+  BamfLegacyWindowTest *copy = bamf_legacy_window_copy (self);
+  g_object_weak_ref (G_OBJECT (self), (GWeakNotify) handle_destroy_notify, copy);
+  bamf_legacy_window_test_close (self);
+}
+
+BamfWindowType
+bamf_legacy_window_test_get_window_type (BamfLegacyWindow *window)
+{
+  g_return_val_if_fail (BAMF_IS_LEGACY_WINDOW_TEST (window), TRUE);
+
+  BamfLegacyWindowTest *self = BAMF_LEGACY_WINDOW_TEST (window);
+  return self->window_type;
+}
+
 void
 bamf_legacy_window_test_dispose (GObject *object)
 {
@@ -294,7 +342,8 @@
 
   obj_class->dispose          = bamf_legacy_window_test_dispose;
   win_class->get_name         = bamf_legacy_window_test_get_name;
-  win_class->get_class_name   = bamf_legacy_window_test_get_class;
+  win_class->get_class_name   = bamf_legacy_window_test_get_class_name;
+  win_class->get_class_instance_name = bamf_legacy_window_test_get_class_instance_name;
   win_class->get_exec_string  = bamf_legacy_window_test_get_exec_string;
   win_class->get_xid          = bamf_legacy_window_test_get_xid;
   win_class->get_pid          = bamf_legacy_window_test_get_pid;
@@ -306,7 +355,10 @@
   win_class->get_unique_bus_name = bamf_legacy_window_test_get_unique_bus_name;
   win_class->get_menu_object_path = bamf_legacy_window_test_get_menu_object_path;
   win_class->get_geometry     = bamf_legacy_window_test_get_geometry;
+  win_class->get_window_type  = bamf_legacy_window_test_get_window_type;
   win_class->maximized        = bamf_legacy_window_test_maximized;
+  win_class->is_closed        = bamf_legacy_window_test_is_closed;
+  win_class->reopen           = bamf_legacy_window_test_reopen;
 }
 
 
@@ -315,18 +367,47 @@
 {
   self->pid = g_random_int_range (1, 100000);
   self->maximized = BAMF_WINDOW_FLOATING;
-}
-
-
-BamfLegacyWindowTest *
-bamf_legacy_window_test_new (guint32 xid, gchar *name, gchar *klass, gchar *exec)
+  self->is_closed = FALSE;
+}
+
+
+BamfLegacyWindowTest *
+bamf_legacy_window_copy (BamfLegacyWindowTest *self)
+{
+  BamfLegacyWindowTest *copy;
+
+  copy = g_object_new (BAMF_TYPE_LEGACY_WINDOW_TEST, NULL);
+  copy->xid = self->xid;
+  copy->pid = self->pid;
+  copy->name = g_strdup (self->name);
+  copy->wm_class_name = g_strdup (self->wm_class_name);
+  copy->wm_class_instance = g_strdup (self->wm_class_instance);
+  copy->exec = g_strdup (self->exec);
+  copy->application_id = g_strdup (self->application_id);
+  copy->unique_bus_name = g_strdup (self->unique_bus_name);
+  copy->dbus_menu_object_path = g_strdup (self->dbus_menu_object_path);
+  copy->needs_attention = self->needs_attention;
+  copy->is_desktop = self->is_desktop;
+  copy->is_skip = self->is_skip;
+  copy->is_active = self->is_active;
+  copy->is_closed = self->is_closed;
+  copy->geometry = self->geometry;
+  copy->maximized = self->maximized;
+  copy->window_type = self->window_type;
+
+  return copy;
+}
+
+BamfLegacyWindowTest *
+bamf_legacy_window_test_new (guint32 xid, gchar *name, gchar *wmclass_name, gchar *exec)
 {
   BamfLegacyWindowTest *self;
 
   self = g_object_new (BAMF_TYPE_LEGACY_WINDOW_TEST, NULL);
+  self->window_type = BAMF_WINDOW_NORMAL;
   self->xid = xid;
   self->name = g_strdup (name);
-  self->klass = g_strdup (klass);
+  self->wm_class_name = g_strdup (wmclass_name);
   self->exec = g_strdup (exec);
 
   return self;

=== modified file 'src/bamf-legacy-window-test.h'
--- src/bamf-legacy-window-test.h	2012-01-10 18:01:52 +0000
+++ src/bamf-legacy-window-test.h	2012-07-31 15:13:24 +0000
@@ -54,7 +54,8 @@
   guint32 xid;
   gint    pid;
   char * name;
-  char * klass;
+  char * wm_class_name;
+  char * wm_class_instance;
   char * exec;
   char * application_id;
   char * unique_bus_name;
@@ -63,8 +64,10 @@
   gboolean is_desktop;
   gboolean is_skip;
   gboolean is_active;
+  gboolean is_closed;
   GdkRectangle geometry;
   BamfWindowMaximizationType maximized;
+  BamfWindowType window_type;
 };
 
 struct _BamfLegacyWindowTestClass
@@ -136,6 +139,9 @@
 bamf_legacy_window_test_close (BamfLegacyWindowTest *self);
 
 BamfLegacyWindowTest *
-bamf_legacy_window_test_new (guint32 xid, gchar *name, gchar *klass, gchar *exec);
+bamf_legacy_window_test_new (guint32 xid, gchar *name, gchar *wmclass_name, gchar *exec);
+
+BamfLegacyWindowTest *
+bamf_legacy_window_copy (BamfLegacyWindowTest *self);
 
 #endif

=== modified file 'src/bamf-legacy-window.c'
--- src/bamf-legacy-window.c	2012-04-19 18:00:19 +0000
+++ src/bamf-legacy-window.c	2012-07-31 15:13:24 +0000
@@ -78,6 +78,10 @@
 bamf_legacy_window_get_window_type (BamfLegacyWindow *self)
 {
   g_return_val_if_fail (BAMF_IS_LEGACY_WINDOW (self), 0);
+
+  if (BAMF_LEGACY_WINDOW_GET_CLASS (self)->get_window_type)
+    return BAMF_LEGACY_WINDOW_GET_CLASS (self)->get_window_type (self);
+
   g_return_val_if_fail (self->priv->legacy_window, 0);
 
   return (BamfWindowType) wnck_window_get_window_type (self->priv->legacy_window);
@@ -88,7 +92,6 @@
 {
   g_return_val_if_fail (BAMF_IS_LEGACY_WINDOW (self), FALSE);
 
-
   if (BAMF_LEGACY_WINDOW_GET_CLASS (self)->needs_attention)
     return BAMF_LEGACY_WINDOW_GET_CLASS (self)->needs_attention (self);
 
@@ -356,11 +359,14 @@
   g_signal_emit (self, legacy_window_signals[GEOMETRY_CHANGED], 0);
 }
 
-gboolean 
+gboolean
 bamf_legacy_window_is_closed (BamfLegacyWindow *self)
 {
   g_return_val_if_fail (BAMF_IS_LEGACY_WINDOW (self), TRUE);
-  
+
+  if (BAMF_LEGACY_WINDOW_GET_CLASS (self)->is_closed)
+    return BAMF_LEGACY_WINDOW_GET_CLASS (self)->is_closed (self);
+
   return self->priv->is_closed;
 }
 
@@ -463,6 +469,10 @@
 bamf_legacy_window_reopen (BamfLegacyWindow *self)
 {
   g_return_if_fail (BAMF_IS_LEGACY_WINDOW (self));
+
+  if (BAMF_LEGACY_WINDOW_GET_CLASS (self)->reopen)
+    return BAMF_LEGACY_WINDOW_GET_CLASS (self)->reopen (self);
+
   g_return_if_fail (WNCK_IS_WINDOW (self->priv->legacy_window));
 
   guint xid = bamf_legacy_window_get_xid (self);
@@ -488,13 +498,13 @@
 
   g_signal_handler_disconnect (wnck_screen_get_default (),
                                self->priv->closed_id);
-                               
+
   if (self->priv->mini_icon_path)
     {
       file = g_file_new_for_path (self->priv->mini_icon_path);
       g_file_delete (file, NULL, NULL);
       g_object_unref (file);
-      
+
       g_free (self->priv->mini_icon_path);
       self->priv->mini_icon_path = NULL;
     }

=== modified file 'src/bamf-legacy-window.h'
--- src/bamf-legacy-window.h	2012-04-18 20:12:33 +0000
+++ src/bamf-legacy-window.h	2012-07-31 15:13:24 +0000
@@ -84,11 +84,13 @@
   gboolean     (*is_skip_tasklist)        (BamfLegacyWindow *legacy_window);
   gboolean     (*is_desktop)              (BamfLegacyWindow *legacy_window);
   gboolean     (*is_dialog)               (BamfLegacyWindow *legacy_window);
+  gboolean     (*is_closed)               (BamfLegacyWindow *legacy_window);
   BamfWindowMaximizationType (*maximized) (BamfLegacyWindow *legacy_window);
-
-  void         (*get_geometry)         (BamfLegacyWindow *self,
-                                        gint *x, gint *y,
-                                        gint *width, gint *height);
+  BamfWindowType (*get_window_type)       (BamfLegacyWindow *legacy_window);
+  void         (*get_geometry)            (BamfLegacyWindow *legacy_window,
+                                           gint *x, gint *y,
+                                           gint *width, gint *height);
+  void         (*reopen)                  (BamfLegacyWindow *legacy_window);
 
   /*< signals >*/
   void     (*name_changed)     (BamfLegacyWindow *legacy_window);

=== added file 'src/bamf-matcher-private.h'
--- src/bamf-matcher-private.h	1970-01-01 00:00:00 +0000
+++ src/bamf-matcher-private.h	2012-07-31 15:13:24 +0000
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ *
+ */
+
+#ifndef __BAMF_MATCHER_PRIVATE_H__
+#define __BAMF_MATCHER_PRIVATE_H__
+
+#include "bamf-view.h"
+#include "bamf-matcher.h"
+#include "bamf-application.h"
+#include "bamf-window.h"
+#include "bamf-legacy-window.h"
+
+struct _BamfMatcherPrivate
+{
+  GArray          * bad_prefixes;
+  GArray          * good_prefixes;
+  GArray          * known_pids;
+  GHashTable      * desktop_id_table;
+  GHashTable      * desktop_file_table;
+  GHashTable      * desktop_class_table;
+  GHashTable      * registered_pids;
+  GHashTable      * opened_closed_paths_table;
+  GList           * views;
+  GList           * monitors;
+  GList           * favorites;
+  BamfView        * active_app;
+  BamfView        * active_win;
+  guint             dispatch_changes_id;
+};
+
+BamfApplication * bamf_matcher_get_application_by_desktop_file (BamfMatcher *self, const char *desktop_file);
+BamfApplication * bamf_matcher_get_application_by_xid (BamfMatcher *self, guint xid);
+
+#endif

=== modified file 'src/bamf-matcher.c'
--- src/bamf-matcher.c	2012-05-22 17:02:28 +0000
+++ src/bamf-matcher.c	2012-07-31 15:13:24 +0000
@@ -21,10 +21,9 @@
 #include "config.h"
 
 #include "bamf-matcher.h"
+#include "bamf-matcher-private.h"
 #include "bamf-application.h"
 #include "bamf-window.h"
-#include "bamf-legacy-window.h"
-#include "bamf-legacy-window-test.h"
 #include "bamf-legacy-screen.h"
 #include "bamf-indicator-source.h"
 #include "bamf-xutils.h"
@@ -50,24 +49,6 @@
 static BamfMatcher *static_matcher;
 static guint matcher_signals[LAST_SIGNAL] = { 0 };
 
-struct _BamfMatcherPrivate
-{
-  GArray          * bad_prefixes;
-  GArray          * good_prefixes;
-  GArray          * known_pids;
-  GHashTable      * desktop_id_table;
-  GHashTable      * desktop_file_table;
-  GHashTable      * desktop_class_table;
-  GHashTable      * registered_pids;
-  GHashTable      * opened_closed_paths_table;
-  GList           * views;
-  GList           * monitors;
-  GList           * favorites;
-  BamfView        * active_app;
-  BamfView        * active_win;
-  guint             dispatch_changes_id;
-};
-
 static void
 on_view_active_changed (BamfView *view, gboolean active, BamfMatcher *matcher)
 {
@@ -127,7 +108,7 @@
 
 static void bamf_matcher_unregister_view (BamfMatcher *self, BamfView *view);
 
-static BamfApplication *
+BamfApplication *
 bamf_matcher_get_application_by_desktop_file (BamfMatcher *self, const char *desktop_file)
 {
   GList *l;
@@ -158,6 +139,30 @@
   return NULL;
 }
 
+BamfApplication *
+bamf_matcher_get_application_by_xid (BamfMatcher *self, guint xid)
+{
+  GList *l;
+  BamfView *view;
+
+  g_return_val_if_fail (BAMF_IS_MATCHER (self), NULL);
+
+  for (l = self->priv->views; l; l = l->next)
+    {
+      view = l->data;
+
+      if (!BAMF_IS_APPLICATION (view))
+        continue;
+
+      if (bamf_application_manages_xid (BAMF_APPLICATION (view), xid))
+        {
+          return BAMF_APPLICATION (view);
+        }
+    }
+
+  return NULL;
+}
+
 static gboolean
 emit_paths_changed (gpointer user_data)
 {
@@ -2238,7 +2243,7 @@
 {
   g_return_if_fail (BAMF_IS_MATCHER (self));
   g_return_if_fail (BAMF_IS_INDICATOR (indicator));
-  
+
   bamf_matcher_register_view_stealing_ref (self, BAMF_VIEW (indicator));
   bamf_matcher_setup_indicator_state (self, indicator);
 }
@@ -2258,7 +2263,12 @@
                               self->priv->desktop_class_table);
 
   /* If an application with no .desktop file has windows that matches
-   * the new added .desktop file, then we try to re-match them. */
+   * the new added .desktop file, then we try to re-match them.
+   * We use another list to save the windows that should be re-matched to avoid
+   * that the list that we're iterating is changed, since reopening a window
+   * makes it to be removed from the views. */
+  GList *to_rematch = NULL;
+
   for (vl = self->priv->views; vl; vl = vl->next)
     {
       if (!BAMF_IS_APPLICATION (vl->data))
@@ -2271,21 +2281,29 @@
           GList *children = bamf_view_get_children (BAMF_VIEW (app));
 
           for (wl = children; wl; wl = wl->next)
-          {
-            if (!BAMF_IS_WINDOW (wl->data))
-              continue;
-
-            BamfWindow *win = BAMF_WINDOW (wl->data);
-            GList *desktops = bamf_matcher_possible_applications_for_window (self, win, NULL);
-
-            if (g_list_find_custom (desktops, desktop_file, (GCompareFunc) g_strcmp0))
-              {
-                BamfLegacyWindow *legacy_window = bamf_window_get_window (win);
-                bamf_legacy_window_reopen (legacy_window);
-              }
-          }
+            {
+              if (!BAMF_IS_WINDOW (wl->data))
+                continue;
+
+              BamfWindow *win = BAMF_WINDOW (wl->data);
+              GList *desktops = bamf_matcher_possible_applications_for_window (self, win, NULL);
+
+              if (g_list_find_custom (desktops, desktop_file, (GCompareFunc) g_strcmp0))
+                {
+                  BamfLegacyWindow *legacy_window = bamf_window_get_window (win);
+                  to_rematch = g_list_prepend (to_rematch, legacy_window);
+                }
+            }
         }
     }
+
+  for (wl = to_rematch; wl; wl = wl->next)
+    {
+      BamfLegacyWindow *legacy_window = BAMF_LEGACY_WINDOW (wl->data);
+      bamf_legacy_window_reopen (legacy_window);
+    }
+
+  g_list_free (to_rematch);
 }
 
 void
@@ -2396,26 +2414,14 @@
 bamf_matcher_application_for_xid (BamfMatcher *matcher,
                                   guint32 xid)
 {
-  GList *l;
-  BamfView *view;
-  BamfMatcherPrivate *priv;
+  BamfApplication *app;
 
   g_return_val_if_fail (BAMF_IS_MATCHER (matcher), NULL);
 
-  priv = matcher->priv;
-
-  for (l = priv->views; l; l = l->next)
-    {
-      view = l->data;
-
-      if (!BAMF_IS_APPLICATION (view))
-        continue;
-
-      if (bamf_application_manages_xid (BAMF_APPLICATION (view), xid))
-        {
-          return bamf_view_get_path (view);
-        }
-    }
+  app = bamf_matcher_get_application_by_xid (matcher, xid);
+
+  if (BAMF_IS_APPLICATION (app))
+    return bamf_view_get_path (BAMF_VIEW (app));
 
   return "";
 }

=== modified file 'src/bamf-window.c'
--- src/bamf-window.c	2012-02-15 05:45:13 +0000
+++ src/bamf-window.c	2012-07-31 15:13:24 +0000
@@ -265,7 +265,7 @@
   g_return_val_if_fail (BAMF_IS_WINDOW (view), NULL);  
   self = BAMF_WINDOW (view);
 
-  return g_strdup_printf ("window%i", bamf_legacy_window_get_xid (self->priv->legacy_window));
+  return g_strdup_printf ("window%u", bamf_legacy_window_get_xid (self->priv->legacy_window));
 }
 
 gint

=== added file 'tests/bamfdaemon/data/test-bamf-app.desktop'
--- tests/bamfdaemon/data/test-bamf-app.desktop	1970-01-01 00:00:00 +0000
+++ tests/bamfdaemon/data/test-bamf-app.desktop	2012-07-31 15:13:24 +0000
@@ -0,0 +1,7 @@
+[Desktop Entry]
+Name=TestBamfApp
+Type=Application
+Exec=testbamfapp
+Icon=bamf
+StartupWMClass=test_bamf_app
+

=== modified file 'tests/bamfdaemon/test-bamf.c'
--- tests/bamfdaemon/test-bamf.c	2012-01-19 16:22:52 +0000
+++ tests/bamfdaemon/test-bamf.c	2012-07-31 15:13:24 +0000
@@ -28,7 +28,7 @@
 #include "bamf.h"
 
 void test_application_create_suite (GDBusConnection *connection);
-void test_matcher_create_suite (void);
+void test_matcher_create_suite (GDBusConnection *connection);
 void test_view_create_suite (GDBusConnection *connection);
 void test_window_create_suite (void);
 
@@ -39,7 +39,7 @@
 {
   GMainLoop *loop = data;
 
-  test_matcher_create_suite ();
+  test_matcher_create_suite (connection);
   test_view_create_suite (connection);
   test_window_create_suite ();
   test_application_create_suite (connection);

=== modified file 'tests/bamfdaemon/test-matcher.c'
--- tests/bamfdaemon/test-matcher.c	2010-05-05 10:07:48 +0000
+++ tests/bamfdaemon/test-matcher.c	2012-07-31 15:13:24 +0000
@@ -1,8 +1,8 @@
 /*
- * Copyright (C) 2009 Canonical Ltd
+ * Copyright (C) 2009-2012 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 3 as 
+ * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * This program is distributed in the hope that it will be useful,
@@ -13,30 +13,313 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by Neil Jagdish Patel <[e-mail]>
+ * Authored by: Neil Jagdish Patel <[e-mail]>
+ *              Marco Trevisan (Treviño) <[e-mail]>
  *
  */
 
 #include <glib.h>
 #include <stdlib.h>
 #include "bamf-matcher.h"
-
-static void test_allocation          (void);
+#include "bamf-matcher-private.h"
+#include "bamf-legacy-screen-private.h"
+#include "bamf-legacy-window.h"
+#include "bamf-legacy-window-test.h"
+
+static void test_allocation (void);
+static void test_load_desktop_file (void);
+static void test_open_windows (void);
+static void test_match_desktopless_application (void);
+static void test_match_desktop_application (void);
+static void test_new_desktop_matches_unmatched_windows (void);
+
+static GDBusConnection *gdbus_connection = NULL;
+
+#define DOMAIN "/Matcher"
+#define DATA_DIR TESTDIR "/bamfdaemon/data"
+#define TEST_BAMF_APP_DESKTOP DATA_DIR "/test-bamf-app.desktop"
 
 void
-test_matcher_create_suite (void)
+test_matcher_create_suite (GDBusConnection *connection)
 {
-#define DOMAIN "/Matcher"
+  gdbus_connection = connection;
 
   g_test_add_func (DOMAIN"/Allocation", test_allocation);
+  g_test_add_func (DOMAIN"/LoadDesktopFile", test_load_desktop_file);
+  g_test_add_func (DOMAIN"/OpenWindows", test_open_windows);
+  g_test_add_func (DOMAIN"/MatchDesktopLessApplication", test_match_desktopless_application);
+  g_test_add_func (DOMAIN"/MatchDesktopApplication", test_match_desktop_application);
+  g_test_add_func (DOMAIN"/NewMatchesUnmatchedWindows", test_new_desktop_matches_unmatched_windows);
+}
+
+static void
+export_matcher_on_bus (BamfMatcher *matcher)
+{
+  GError *error = NULL;
+  g_return_if_fail (BAMF_IS_MATCHER (matcher));
+
+  g_dbus_interface_skeleton_export (G_DBUS_INTERFACE_SKELETON (matcher),
+                                    gdbus_connection, BAMF_MATCHER_PATH"/Test",
+                                    &error);
+
+  g_assert (!error);
+  g_clear_error (&error);
+}
+
+static BamfWindow *
+find_window_in_matcher (BamfMatcher *matcher, BamfLegacyWindow *legacy)
+{
+  GList *l;
+  BamfWindow *found_window = NULL;
+
+  for (l = matcher->priv->views; l; l = l->next)
+    {
+      if (!BAMF_IS_WINDOW (l->data))
+        continue;
+
+      if (bamf_window_get_window (BAMF_WINDOW (l->data)) == legacy)
+      {
+        g_assert (!found_window);
+        found_window = l->data;
+      }
+    }
+
+  return found_window;
+}
+
+static BamfWindow *
+find_window_in_app (BamfApplication *app, BamfLegacyWindow *legacy)
+{
+  GList *l;
+  BamfWindow *found_window = NULL;
+
+  for (l = bamf_view_get_children (BAMF_VIEW (app)); l; l = l->next)
+    {
+      if (!BAMF_IS_WINDOW (l->data))
+        continue;
+
+      if (bamf_window_get_window (BAMF_WINDOW (l->data)) == legacy)
+      {
+        g_assert (!found_window);
+        found_window = l->data;
+      }
+    }
+
+  return found_window;
 }
 
 static void
 test_allocation (void)
 {
-  BamfMatcher    *matcher;
+  BamfMatcher *matcher;
 
   /* Check it allocates */
   matcher = bamf_matcher_get_default ();
   g_assert (BAMF_IS_MATCHER (matcher));
-}
+  g_object_unref (matcher);
+}
+
+static void
+test_load_desktop_file (void)
+{
+  BamfMatcher *matcher = bamf_matcher_get_default ();
+  BamfMatcherPrivate *priv = matcher->priv;
+
+  bamf_matcher_load_desktop_file (matcher, TEST_BAMF_APP_DESKTOP);
+
+  GList *l = g_hash_table_lookup (priv->desktop_file_table, "testbamfapp");
+  g_assert_cmpstr (l->data, ==, TEST_BAMF_APP_DESKTOP);
+
+  l = g_hash_table_lookup (priv->desktop_id_table, "test-bamf-app");
+  g_assert_cmpstr (l->data, ==, TEST_BAMF_APP_DESKTOP);
+
+  const char *desktop = g_hash_table_lookup (priv->desktop_class_table, TEST_BAMF_APP_DESKTOP);
+  g_assert_cmpstr (desktop, ==, "test_bamf_app");
+}
+
+static void
+test_open_windows (void)
+{
+  BamfMatcher *matcher;
+  BamfLegacyScreen *screen;
+  BamfLegacyWindow *window;
+  BamfLegacyWindowTest *test_win;
+  guint xid;
+  const int window_count = 500;
+
+  screen = bamf_legacy_screen_get_default();
+  matcher = bamf_matcher_get_default ();
+
+  export_matcher_on_bus (matcher);
+
+  for (xid = G_MAXUINT; xid > G_MAXUINT-window_count; xid--)
+    {
+      gchar *name = g_strdup_printf ("Test Window %u", xid);
+      gchar *class = g_strdup_printf ("test-class-%u", xid);
+      gchar *exec = g_strdup_printf ("test-class-%u", xid);
+
+      test_win = bamf_legacy_window_test_new (xid, name, class, exec);
+      window = BAMF_LEGACY_WINDOW (test_win);
+
+      _bamf_legacy_screen_open_test_window (screen, test_win);
+      g_assert (g_list_find (bamf_legacy_screen_get_windows (screen), test_win));
+      g_assert (find_window_in_matcher (matcher, window));
+      g_assert (bamf_matcher_get_application_by_xid (matcher, xid));
+
+      _bamf_legacy_screen_close_test_window (screen, test_win);
+      g_assert (!g_list_find (bamf_legacy_screen_get_windows (screen), test_win));
+      g_assert (!find_window_in_matcher (matcher, window));
+      g_assert (!bamf_matcher_get_application_by_xid (matcher, xid));
+
+      g_free (name);
+      g_free (class);
+      g_free (exec);
+    }
+
+  g_object_unref (matcher);
+  g_object_unref (screen);
+}
+
+static void
+test_match_desktopless_application (void)
+{
+  BamfMatcher *matcher;
+  BamfLegacyScreen *screen;
+  BamfLegacyWindow *window;
+  BamfLegacyWindowTest *test_win;
+  BamfApplication *app;
+  GList *test_windows = NULL, *l, *app_children;
+  guint xid;
+  const int window_count = 5;
+
+  screen = bamf_legacy_screen_get_default();
+  matcher = bamf_matcher_get_default ();
+  char *exec = "test-bamf-app";
+  char *class = "test-bamf-app";
+
+  export_matcher_on_bus (matcher);
+
+  for (xid = G_MAXUINT; xid > G_MAXUINT-window_count; xid--)
+    {
+      gchar *name = g_strdup_printf ("Test Window %u", xid);
+
+      test_win = bamf_legacy_window_test_new (xid, name, class, exec);
+      window = BAMF_LEGACY_WINDOW (test_win);
+      test_windows = g_list_prepend (test_windows, window);
+
+      _bamf_legacy_screen_open_test_window (screen, test_win);
+      g_free (name);
+    }
+
+  app = bamf_matcher_get_application_by_xid (matcher, G_MAXUINT);
+  g_assert (app);
+
+  app_children = bamf_view_get_children (BAMF_VIEW (app));
+  g_assert_cmpuint (g_list_length (app_children), ==, window_count);
+
+  for (l = test_windows; l; l = l->next)
+    {
+      g_assert (find_window_in_app (app, BAMF_LEGACY_WINDOW (l->data)));
+    }
+
+  g_list_free (test_windows);
+  g_object_unref (matcher);
+  g_object_unref (screen);
+}
+
+static void
+test_match_desktop_application (void)
+{
+  BamfMatcher *matcher;
+  BamfLegacyScreen *screen;
+  BamfLegacyWindow *window;
+  BamfLegacyWindowTest *test_win;
+  BamfApplication *app;
+  GList *test_windows = NULL, *l, *app_children;
+  guint xid;
+  const int window_count = 5;
+
+  screen = bamf_legacy_screen_get_default();
+  matcher = bamf_matcher_get_default ();
+  char *exec = "testbamfapp";
+  char *class = "test_bamf_app";
+
+  export_matcher_on_bus (matcher);
+  bamf_matcher_load_desktop_file (matcher, TEST_BAMF_APP_DESKTOP);
+
+  for (xid = G_MAXUINT; xid > G_MAXUINT-window_count; xid--)
+    {
+      gchar *name = g_strdup_printf ("Test Window %u", xid);
+
+      test_win = bamf_legacy_window_test_new (xid, name, class, exec);
+      window = BAMF_LEGACY_WINDOW (test_win);
+      test_windows = g_list_prepend (test_windows, window);
+
+      _bamf_legacy_screen_open_test_window (screen, test_win);
+      g_free (name);
+    }
+
+  app = bamf_matcher_get_application_by_desktop_file (matcher, TEST_BAMF_APP_DESKTOP);
+  g_assert (app);
+
+  g_assert (bamf_matcher_get_application_by_xid (matcher, G_MAXUINT) == app);
+
+  app_children = bamf_view_get_children (BAMF_VIEW (app));
+  g_assert_cmpuint (g_list_length (app_children), ==, window_count);
+
+  for (l = test_windows; l; l = l->next)
+    {
+      g_assert (find_window_in_app (app, BAMF_LEGACY_WINDOW (l->data)));
+    }
+
+  g_list_free (test_windows);
+  g_object_unref (matcher);
+  g_object_unref (screen);
+}
+
+static void
+test_new_desktop_matches_unmatched_windows (void)
+{
+  BamfMatcher *matcher;
+  BamfLegacyScreen *screen;
+  BamfLegacyWindowTest *test_win;
+  BamfApplication *app;
+  GList *app_children;
+  guint xid = 0;
+  const int window_count = 5;
+
+  screen = bamf_legacy_screen_get_default();
+  matcher = bamf_matcher_get_default ();
+  char *exec = "testbamfapp";
+  char *class = "test_bamf_app";
+
+  export_matcher_on_bus (matcher);
+  g_assert (!bamf_matcher_get_application_by_desktop_file (matcher, TEST_BAMF_APP_DESKTOP));
+
+  for (xid = G_MAXUINT; xid > G_MAXUINT-window_count; xid--)
+    {
+      gchar *name = g_strdup_printf ("Test Window %u", xid);
+
+      test_win = bamf_legacy_window_test_new (xid, name, class, exec);
+      _bamf_legacy_screen_open_test_window (screen, test_win);
+
+      g_free (name);
+    }
+
+  bamf_matcher_load_desktop_file (matcher, TEST_BAMF_APP_DESKTOP);
+
+  app = bamf_matcher_get_application_by_desktop_file (matcher, TEST_BAMF_APP_DESKTOP);
+  g_assert (app);
+
+  app_children = bamf_view_get_children (BAMF_VIEW (app));
+  g_assert_cmpuint (g_list_length (app_children), ==, window_count);
+
+  for (xid = G_MAXUINT; xid > G_MAXUINT-window_count; xid--)
+    {
+      g_assert (bamf_matcher_get_application_by_xid (matcher, xid) == app);
+    }
+
+  g_object_unref (matcher);
+  g_object_unref (screen);
+}
+