[virt-tools-list] [PATCH virt-viewer 3/4] VirtViewerApp: store windows in a list

Fabiano Fidêncio fidencio at redhat.com
Wed Sep 3 21:35:46 UTC 2014


On Fri, 2014-08-22 at 09:59 -0500, Jonathon Jongsma wrote:
> Use a list to store the application's windows.  This is another step
> towards separating the window from the guest display ID.
> ---
>  src/remote-viewer.c   |  14 ++--
>  src/virt-viewer-app.c | 175 +++++++++++++++++++++++---------------------------
>  src/virt-viewer-app.h |   2 +-
>  3 files changed, 86 insertions(+), 105 deletions(-)
> 
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index cb43365..ccb775b 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 a3934f2..d143f94 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;

You don't have to initialize nth here, probably covscan would complain
about it.

>      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(VIRT_VIEWER_WINDOW(l->data));

You could use l->data directly here, no need to cast.

> +        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));
> @@ -978,8 +950,14 @@ virt_viewer_app_display_removed(VirtViewerSession *session G_GNUC_UNUSED,
>          return;
>  
>      virt_viewer_window_set_display(win, NULL);
> -    if (nth != 0)
> -        virt_viewer_app_remove_nth_window(self, nth);
> +
> +    g_debug("Remove window %d %p", nth, win);
> +    self->priv->windows = g_list_remove(self->priv->windows, win);
> +    virt_viewer_app_update_menu_displays(self);
> +
> +    g_signal_emit(self, signals[SIGNAL_WINDOW_REMOVED], 0, win);
> +
> +    g_object_unref(win);

Hmmm. I would prefer a function to remove the window from the windows
list and unref the removed window, instead of doing it in the way you're
doing it here.

>  }
>  
>  static void
> @@ -1635,13 +1613,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 +1698,6 @@ virt_viewer_app_init (VirtViewerApp *self)
>  
>      self->priv = GET_PRIVATE(self);
>      self->priv->displays = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, 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 +2100,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 +2141,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;
> @@ -2227,13 +2213,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);
> @@ -2242,8 +2227,8 @@ 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);
> +        VirtViewerWindow *vwin = virt_viewer_app_get_nth_window(self, *nth);
> +        VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(g_hash_table_lookup(self->priv->displays, nth));
>          GtkWidget *item;
>          gboolean visible, sensitive;
>          gchar *label;
> @@ -2252,7 +2237,7 @@ window_update_menu_displays_cb(gpointer key G_GNUC_UNUSED,
>          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 +2253,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 +2267,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 +2327,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 +2349,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 +2366,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 +2385,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);





More information about the virt-tools-list mailing list