[virt-tools-list] [PATCH v2 virt-viewer] VirtViewerApp: store windows in a list

Jonathon Jongsma jjongsma at redhat.com
Thu Sep 4 20:36:11 UTC 2014


Use a list to store the application's windows.  This is another step
towards separating the window from the guest display ID.
---

Changes from Fabiano's review

 src/remote-viewer.c   |  14 ++--
 src/virt-viewer-app.c | 206 ++++++++++++++++++++++++--------------------------
 src/virt-viewer-app.h |   2 +-
 3 files changed, 104 insertions(+), 118 deletions(-)

diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 5f5fa3d..ad5f522 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -405,8 +405,7 @@ spice_menu_update(RemoteViewer *self, VirtViewerWindow *win)
 }
 
 static void
-spice_menu_update_each(gpointer key G_GNUC_UNUSED,
-                       gpointer value,
+spice_menu_update_each(gpointer value,
                        gpointer user_data)
 {
     spice_menu_update(REMOTE_VIEWER(user_data), VIRT_VIEWER_WINDOW(value));
@@ -415,11 +414,11 @@ spice_menu_update_each(gpointer key G_GNUC_UNUSED,
 static void
 spice_ctrl_menu_updated(RemoteViewer *self)
 {
-    GHashTable *windows = virt_viewer_app_get_windows(VIRT_VIEWER_APP(self));
+    GList *windows = virt_viewer_app_get_windows(VIRT_VIEWER_APP(self));
 
     g_debug("Spice controller menu updated");
 
-    g_hash_table_foreach(windows, spice_menu_update_each, self);
+    g_list_foreach(windows, spice_menu_update_each, self);
 }
 
 #ifdef HAVE_OVIRT
@@ -482,8 +481,7 @@ foreign_menu_update(RemoteViewer *self, VirtViewerWindow *win)
 }
 
 static void
-foreign_menu_update_each(gpointer key G_GNUC_UNUSED,
-                         gpointer value,
+foreign_menu_update_each(gpointer value,
                          gpointer user_data)
 {
     foreign_menu_update(REMOTE_VIEWER(user_data), VIRT_VIEWER_WINDOW(value));
@@ -496,11 +494,11 @@ foreign_menu_update_each(gpointer key G_GNUC_UNUSED,
 static void
 spice_foreign_menu_updated(RemoteViewer *self)
 {
-    GHashTable *windows = virt_viewer_app_get_windows(VIRT_VIEWER_APP(self));
+    GList *windows = virt_viewer_app_get_windows(VIRT_VIEWER_APP(self));
 
     g_debug("Spice foreign menu updated");
 
-    g_hash_table_foreach(windows, foreign_menu_update_each, self);
+    g_list_foreach(windows, foreign_menu_update_each, self);
 }
 
 static SpiceSession *
diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
index a0b1d76..0b6f24c 100644
--- a/src/virt-viewer-app.c
+++ b/src/virt-viewer-app.c
@@ -108,7 +108,7 @@ static void virt_viewer_update_smartcard_accels(VirtViewerApp *self);
 struct _VirtViewerAppPrivate {
     VirtViewerWindow *main_window;
     GtkWidget *main_notebook;
-    GHashTable *windows;
+    GList *windows;
     GHashTable *displays;
     GHashTable *initial_display_map;
     gchar *clipboard;
@@ -444,14 +444,12 @@ void virt_viewer_app_set_uuid_string(VirtViewerApp *self, const gchar *uuid_stri
     // if we're changing our initial display map, move any existing windows to
     // the appropriate monitors according to the per-vm configuration
     if (mapping && self->priv->fullscreen) {
-        GHashTableIter iter;
-        gpointer value;
+        GList *l;
         gint i = 0;
 
-        g_hash_table_iter_init(&iter, self->priv->windows);
-        while (g_hash_table_iter_next(&iter, NULL, &value)) {
+        for (l = self->priv->windows; l; l = l->next) {
             gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, i);
-            app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), monitor);
+            app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(l->data), monitor);
             i++;
         }
     }
@@ -508,8 +506,7 @@ virt_viewer_app_maybe_quit(VirtViewerApp *self, VirtViewerWindow *window)
 
 }
 
-static void count_window_visible(gpointer key G_GNUC_UNUSED,
-                                 gpointer value,
+static void count_window_visible(gpointer value,
                                  gpointer user_data)
 {
     GtkWindow *win = virt_viewer_window_get_window(VIRT_VIEWER_WINDOW(value));
@@ -523,7 +520,7 @@ static guint
 virt_viewer_app_get_n_windows_visible(VirtViewerApp *self)
 {
     guint n = 0;
-    g_hash_table_foreach(self->priv->windows, count_window_visible, &n);
+    g_list_foreach(self->priv->windows, count_window_visible, &n);
     return n;
 }
 
@@ -550,8 +547,7 @@ virt_viewer_app_window_set_visible(VirtViewerApp *self,
     return FALSE;
 }
 
-static void hide_one_window(gpointer key G_GNUC_UNUSED,
-                            gpointer value,
+static void hide_one_window(gpointer value,
                             gpointer user_data G_GNUC_UNUSED)
 {
     VirtViewerApp* self = VIRT_VIEWER_APP(user_data);
@@ -562,7 +558,7 @@ static void hide_one_window(gpointer key G_GNUC_UNUSED,
 static void
 virt_viewer_app_hide_all_windows(VirtViewerApp *app)
 {
-    g_hash_table_foreach(app->priv->windows, hide_one_window, app);
+    g_list_foreach(app->priv->windows, hide_one_window, app);
 }
 
 G_MODULE_EXPORT void
@@ -736,24 +732,28 @@ virt_viewer_app_set_window_subtitle(VirtViewerApp *app,
 }
 
 static void
-set_title(gpointer key,
-          gpointer value,
+set_title(gpointer value,
           gpointer user_data)
 {
-    gint *nth = key;
+    gint nth = -1;
     VirtViewerApp *app = user_data;
     VirtViewerWindow *window = value;
-    virt_viewer_app_set_window_subtitle(app, window, *nth);
+    VirtViewerDisplay *display = virt_viewer_window_get_display(window);
+
+    if (!display)
+        return;
+
+    nth = virt_viewer_display_get_nth(display);
+    virt_viewer_app_set_window_subtitle(app, window, nth);
 }
 
 static void
 virt_viewer_app_set_all_window_subtitles(VirtViewerApp *app)
 {
-    g_hash_table_foreach(app->priv->windows, set_title, app);
+    g_list_foreach(app->priv->windows, set_title, app);
 }
 
-static void update_title(gpointer key G_GNUC_UNUSED,
-                         gpointer value,
+static void update_title(gpointer value,
                          gpointer user_data G_GNUC_UNUSED)
 {
     virt_viewer_window_update_title(VIRT_VIEWER_WINDOW(value));
@@ -762,11 +762,10 @@ static void update_title(gpointer key G_GNUC_UNUSED,
 static void
 virt_viewer_app_update_title(VirtViewerApp *self)
 {
-    g_hash_table_foreach(self->priv->windows, update_title, NULL);
+    g_list_foreach(self->priv->windows, update_title, NULL);
 }
 
-static void set_usb_options_sensitive(gpointer key G_GNUC_UNUSED,
-                                      gpointer value,
+static void set_usb_options_sensitive(gpointer value,
                                       gpointer user_data)
 {
     virt_viewer_window_set_usb_options_sensitive(VIRT_VIEWER_WINDOW(value),
@@ -776,59 +775,22 @@ static void set_usb_options_sensitive(gpointer key G_GNUC_UNUSED,
 static void
 virt_viewer_app_set_usb_options_sensitive(VirtViewerApp *self, gboolean sensitive)
 {
-    g_hash_table_foreach(self->priv->windows, set_usb_options_sensitive,
-                         GINT_TO_POINTER(sensitive));
+    g_list_foreach(self->priv->windows, set_usb_options_sensitive,
+                   GINT_TO_POINTER(sensitive));
 }
 
 static VirtViewerWindow *
 virt_viewer_app_get_nth_window(VirtViewerApp *self, gint nth)
 {
-    return g_hash_table_lookup(self->priv->windows, &nth);
-}
-
-static gboolean
-virt_viewer_app_remove_nth_window(VirtViewerApp *self, gint nth)
-{
-    VirtViewerWindow *win;
-    gboolean removed;
-
-    g_return_val_if_fail(nth != 0, FALSE);
-
-    win = virt_viewer_app_get_nth_window(self, nth);
-    g_return_val_if_fail(win != NULL, FALSE);
-
-    g_debug("Remove window %d %p", nth, win);
-    g_object_ref(win);
-    removed = g_hash_table_remove(self->priv->windows, &nth);
-    g_warn_if_fail(removed);
-    virt_viewer_app_update_menu_displays(self);
-
-    if (removed)
-        g_signal_emit(self, signals[SIGNAL_WINDOW_REMOVED], 0, win);
-
-    g_object_unref(win);
-
-    return removed;
-}
-
-static void
-virt_viewer_app_set_nth_window(VirtViewerApp *self, gint nth, VirtViewerWindow *win)
-{
-    gint *key;
-
-    g_return_if_fail(virt_viewer_app_get_nth_window(self, nth) == NULL);
-    key = g_malloc(sizeof(gint));
-    *key = nth;
-    g_debug("Insert window %d %p", nth, win);
-    g_hash_table_insert(self->priv->windows, key, win);
-    virt_viewer_app_set_window_subtitle(self, win, nth);
-    virt_viewer_app_update_menu_displays(self);
-    if (self->priv->session) {
-        virt_viewer_window_set_usb_options_sensitive(win,
-                    virt_viewer_session_get_has_usbredir(self->priv->session));
+    GList *l;
+    for (l = self->priv->windows; l; l = l->next) {
+        VirtViewerDisplay *display = virt_viewer_window_get_display(l->data);
+        if (display
+            && (virt_viewer_display_get_nth(display) == nth)) {
+            return l->data;
+        }
     }
-
-    g_signal_emit(self, signals[SIGNAL_WINDOW_ADDED], 0, win);
+    return NULL;
 }
 
 static void
@@ -879,7 +841,17 @@ virt_viewer_app_window_new(VirtViewerApp *self, gint nth)
     virt_viewer_window_set_kiosk(window, self->priv->kiosk);
     if (self->priv->main_window)
         virt_viewer_window_set_zoom_level(window, virt_viewer_window_get_zoom_level(self->priv->main_window));
-    virt_viewer_app_set_nth_window(self, nth, window);
+
+    self->priv->windows = g_list_append(self->priv->windows, window);
+    virt_viewer_app_set_window_subtitle(self, window, nth);
+    virt_viewer_app_update_menu_displays(self);
+    if (self->priv->session) {
+        virt_viewer_window_set_usb_options_sensitive(window,
+                    virt_viewer_session_get_has_usbredir(self->priv->session));
+    }
+
+    g_signal_emit(self, signals[SIGNAL_WINDOW_ADDED], 0, window);
+
     if (self->priv->fullscreen)
         app_window_try_fullscreen(self, window,
                                   virt_viewer_app_get_initial_monitor_for_display(self, nth));
@@ -962,24 +934,35 @@ virt_viewer_app_display_added(VirtViewerSession *session G_GNUC_UNUSED,
 }
 
 
+static void virt_viewer_app_remove_nth_window(VirtViewerApp *self,
+                                              gint nth)
+{
+    VirtViewerWindow *win = virt_viewer_app_get_nth_window(self, nth);
+    if (!win)
+        return;
+
+    virt_viewer_window_set_display(win, NULL);
+    virt_viewer_window_hide(win);
+
+    g_debug("Remove window %d %p", nth, win);
+    self->priv->windows = g_list_remove(self->priv->windows, win);
+
+    g_signal_emit(self, signals[SIGNAL_WINDOW_REMOVED], 0, win);
+
+    g_object_unref(win);
+}
+
 static void
 virt_viewer_app_display_removed(VirtViewerSession *session G_GNUC_UNUSED,
                                 VirtViewerDisplay *display,
                                 VirtViewerApp *self)
 {
-    VirtViewerWindow *win = NULL;
     gint nth;
 
-    gtk_widget_hide(GTK_WIDGET(display));
     g_object_get(display, "nth-display", &nth, NULL);
+    virt_viewer_app_remove_nth_window(self, nth);
     g_hash_table_remove(self->priv->displays, GINT_TO_POINTER(nth));
-    win = virt_viewer_app_get_nth_window(self, nth);
-    if (!win)
-        return;
-
-    virt_viewer_window_set_display(win, NULL);
-    if (nth != 0)
-        virt_viewer_app_remove_nth_window(self, nth);
+    virt_viewer_app_update_menu_displays(self);
 }
 
 static void
@@ -1635,13 +1618,13 @@ virt_viewer_app_dispose (GObject *object)
     VirtViewerAppPrivate *priv = self->priv;
 
     if (priv->windows) {
-        GHashTable *tmp = priv->windows;
+        GList *tmp = priv->windows;
         /* null-ify before unrefing, because we need
          * to prevent callbacks using priv->windows
          * while it is being disposed off. */
         priv->windows = NULL;
         priv->main_window = NULL;
-        g_hash_table_unref(tmp);
+        g_list_free_full(tmp, g_object_unref);
     }
 
     if (priv->displays) {
@@ -1720,7 +1703,6 @@ virt_viewer_app_init (VirtViewerApp *self)
 
     self->priv = GET_PRIVATE(self);
     self->priv->displays = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_object_unref);
-    self->priv->windows = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_object_unref);
     self->priv->config = g_key_file_new();
     self->priv->config_file = g_build_filename(g_get_user_config_dir(),
                                                "virt-viewer", "settings", NULL);
@@ -2123,15 +2105,22 @@ typedef struct {
     gboolean fullscreen;
 } FullscreenOptions;
 
-static void fullscreen_cb(gpointer key,
-                          gpointer value,
+static void fullscreen_cb(gpointer value,
                           gpointer user_data)
 {
     FullscreenOptions *options = (FullscreenOptions *)user_data;
-    gint nth = virt_viewer_app_get_initial_monitor_for_display(options->app, *(gint*)key);
+    gint nth = 0;
     VirtViewerWindow *vwin = VIRT_VIEWER_WINDOW(value);
+    VirtViewerDisplay *display = virt_viewer_window_get_display(vwin);
 
+    /* At startup, the main window will not yet have an associated display, so
+     * assume that it's the first display */
+    if (display)
+        nth = virt_viewer_display_get_nth(display);
     g_debug("fullscreen display %d: %d", nth, options->fullscreen);
+
+    nth = virt_viewer_app_get_initial_monitor_for_display(options->app, nth);
+
     if (options->fullscreen)
         app_window_try_fullscreen(options->app, vwin, nth);
     else
@@ -2157,18 +2146,20 @@ virt_viewer_app_set_fullscreen(VirtViewerApp *self, gboolean fullscreen)
 
     /* we iterate unconditionnaly, even if it was set before to update new windows */
     priv->fullscreen = fullscreen;
-    g_hash_table_foreach(priv->windows, fullscreen_cb, &options);
+    g_list_foreach(priv->windows, fullscreen_cb, &options);
 
     g_object_notify(G_OBJECT(self), "fullscreen");
 }
 
 static void
 menu_display_visible_toggled_cb(GtkCheckMenuItem *checkmenuitem,
-                                VirtViewerWindow *vwin)
+                                VirtViewerDisplay *display)
 {
-    VirtViewerApp *self;
+    VirtViewerApp *self = virt_viewer_session_get_app(virt_viewer_display_get_session(display));
     gboolean visible;
     static gboolean reentering = FALSE;
+    VirtViewerWindow *vwin = virt_viewer_app_get_nth_window(self,
+                                                            virt_viewer_display_get_nth(display));
 
     if (reentering) /* do not reenter if I switch you back */
         return;
@@ -2185,12 +2176,12 @@ menu_display_visible_toggled_cb(GtkCheckMenuItem *checkmenuitem,
 static gint
 update_menu_displays_sort(gconstpointer a, gconstpointer b)
 {
-    const int *ai = a;
-    const int *bi = b;
+    const int ai = GPOINTER_TO_INT(a);
+    const int bi = GPOINTER_TO_INT(b);
 
-    if (*ai > *bi)
+    if (ai > bi)
         return 1;
-    else if (*ai < *bi)
+    else if (ai < bi)
         return -1;
     else
         return 0;
@@ -2227,13 +2218,12 @@ window_empty_display_submenu(VirtViewerWindow *window)
 }
 
 static void
-window_update_menu_displays_cb(gpointer key G_GNUC_UNUSED,
-                               gpointer value,
+window_update_menu_displays_cb(gpointer value,
                                gpointer user_data)
 {
     VirtViewerApp *self = VIRT_VIEWER_APP(user_data);
     GtkMenuShell *submenu;
-    GList *keys = g_hash_table_get_keys(self->priv->windows);
+    GList *keys = g_hash_table_get_keys(self->priv->displays);
     GList *tmp;
 
     keys = g_list_sort(keys, update_menu_displays_sort);
@@ -2241,18 +2231,18 @@ window_update_menu_displays_cb(gpointer key G_GNUC_UNUSED,
 
     tmp = keys;
     while (tmp) {
-        int *nth = tmp->data;
-        VirtViewerWindow *vwin = VIRT_VIEWER_WINDOW(g_hash_table_lookup(self->priv->windows, nth));
-        VirtViewerDisplay *display = virt_viewer_window_get_display(vwin);
+        int nth = GPOINTER_TO_INT(tmp->data);
+        VirtViewerWindow *vwin = virt_viewer_app_get_nth_window(self, nth);
+        VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(g_hash_table_lookup(self->priv->displays, tmp->data));
         GtkWidget *item;
         gboolean visible, sensitive;
         gchar *label;
 
-        label = g_strdup_printf(_("Display %d"), *nth + 1);
+        label = g_strdup_printf(_("Display %d"), nth + 1);
         item = gtk_check_menu_item_new_with_label(label);
         g_free(label);
 
-        visible = gtk_widget_get_visible(GTK_WIDGET(virt_viewer_window_get_window(vwin)));
+        visible = vwin && gtk_widget_get_visible(GTK_WIDGET(virt_viewer_window_get_window(vwin)));
         gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(item), visible);
 
         sensitive = visible;
@@ -2268,7 +2258,7 @@ window_update_menu_displays_cb(gpointer key G_GNUC_UNUSED,
         gtk_widget_set_sensitive(item, sensitive);
 
         g_signal_connect(G_OBJECT(item),
-                         "toggled", G_CALLBACK(menu_display_visible_toggled_cb), vwin);
+                         "toggled", G_CALLBACK(menu_display_visible_toggled_cb), display);
         gtk_menu_shell_append(submenu, item);
         tmp = tmp->next;
     }
@@ -2282,7 +2272,7 @@ virt_viewer_app_update_menu_displays(VirtViewerApp *self)
 {
     if (!self->priv->windows)
         return;
-    g_hash_table_foreach(self->priv->windows, window_update_menu_displays_cb, self);
+    g_list_foreach(self->priv->windows, window_update_menu_displays_cb, self);
 }
 
 void
@@ -2342,8 +2332,7 @@ virt_viewer_app_get_main_window(VirtViewerApp *self)
 }
 
 static void
-show_status_cb(gpointer key G_GNUC_UNUSED,
-               gpointer value,
+show_status_cb(gpointer value,
                gpointer user_data)
 {
     VirtViewerNotebook *nb = virt_viewer_window_get_notebook(VIRT_VIEWER_WINDOW(value));
@@ -2365,13 +2354,12 @@ virt_viewer_app_show_status(VirtViewerApp *self, const gchar *fmt, ...)
     text = g_strdup_vprintf(fmt, args);
     va_end(args);
 
-    g_hash_table_foreach(self->priv->windows, show_status_cb, text);
+    g_list_foreach(self->priv->windows, show_status_cb, text);
     g_free(text);
 }
 
 static void
-show_display_cb(gpointer key G_GNUC_UNUSED,
-                gpointer value,
+show_display_cb(gpointer value,
                 gpointer user_data G_GNUC_UNUSED)
 {
     VirtViewerNotebook *nb = virt_viewer_window_get_notebook(VIRT_VIEWER_WINDOW(value));
@@ -2383,7 +2371,7 @@ void
 virt_viewer_app_show_display(VirtViewerApp *self)
 {
     g_return_if_fail(VIRT_VIEWER_IS_APP(self));
-    g_hash_table_foreach(self->priv->windows, show_display_cb, self);
+    g_list_foreach(self->priv->windows, show_display_cb, self);
 }
 
 gboolean
@@ -2402,7 +2390,7 @@ virt_viewer_app_get_session(VirtViewerApp *self)
     return self->priv->session;
 }
 
-GHashTable*
+GList*
 virt_viewer_app_get_windows(VirtViewerApp *self)
 {
     g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), NULL);
diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
index 25cfc8b..b68e50c 100644
--- a/src/virt-viewer-app.h
+++ b/src/virt-viewer-app.h
@@ -92,7 +92,7 @@ void virt_viewer_app_set_connect_info(VirtViewerApp *self,
 gboolean virt_viewer_app_window_set_visible(VirtViewerApp *self, VirtViewerWindow *window, gboolean visible);
 void virt_viewer_app_show_status(VirtViewerApp *self, const gchar *fmt, ...);
 void virt_viewer_app_show_display(VirtViewerApp *self);
-GHashTable* virt_viewer_app_get_windows(VirtViewerApp *self);
+GList* virt_viewer_app_get_windows(VirtViewerApp *self);
 gboolean virt_viewer_app_get_enable_accel(VirtViewerApp *self);
 VirtViewerSession* virt_viewer_app_get_session(VirtViewerApp *self);
 gboolean virt_viewer_app_get_fullscreen(VirtViewerApp *app);
-- 
1.9.3




More information about the virt-tools-list mailing list