[virt-tools-list] [virt-viewer 3/7] ovirt: Use OvirtForeignMenu class

[Jonathon Jongsma]

----- Original Message -----
> From: "Christophe Fergeau" <cfergeau at redhat.com>
> To: virt-tools-list at redhat.com
> Sent: Wednesday, April 16, 2014 11:59:50 AM
> Subject: [virt-tools-list] [virt-viewer 3/7] ovirt: Use OvirtForeignMenu	class
> 
> After the previous commit which introduced the OvirtForeignMenu
> class, we can now make use of it in the remote-viewer UI code.
> ---
>  src/remote-viewer.c | 61
>  +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 4320369..fcd63fc 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -30,6 +30,7 @@
>  
>  #ifdef HAVE_OVIRT
>  #include <govirt/govirt.h>
> +#include "ovirt-foreign-menu.h"
>  #endif
>  
>  #ifdef HAVE_SPICE_GTK
> @@ -413,6 +414,50 @@ spice_ctrl_menu_updated(RemoteViewer *self)
>      g_hash_table_foreach(windows, spice_menu_update_each, self);
>  }
>  
> +#ifdef HAVE_OVIRT
> +static void
> +ovirt_foreign_menu_update(VirtViewerWindow *win,
> +                          OvirtForeignMenu *foreign_menu)

Can this be renamed?  It's nearly identical to spice_ovirt_foreign_menu_update() below. It'd be nice if the names gave an indication of what the difference was so that you didn't need to inspect the implementation. Maybe something like ovirt_foreign_menu_update_for_window()?

> +{
> +    GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
> +    GtkWidget *submenu;
> +    GtkMenuShell *shell =
> GTK_MENU_SHELL(gtk_builder_get_object(virt_viewer_window_get_builder(win),
> "top-menu"));
> +
> +    if (menu != NULL)
> +        gtk_widget_destroy(menu);
> +
> +    menu = gtk_menu_item_new_with_label(_("CDRoms"));

"CDRoms" looks a bit strange.  In the user portal, there's a menu item called "Change CD".  Should we use similar terminology?

> +    gtk_menu_shell_append(shell, menu);
> +    g_object_set_data(G_OBJECT(win), "foreign-menu", menu);
> +
> +    submenu = ovirt_foreign_menu_get_gtk_menu(foreign_menu);
> +    gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
> +
> +    gtk_widget_show_all(menu);
> +}
> +
> +static void
> +ovirt_foreign_menu_update_each(gpointer key G_GNUC_UNUSED,
> +                               gpointer value,
> +                               gpointer user_data)
> +{
> +    ovirt_foreign_menu_update(VIRT_VIEWER_WINDOW(value),
> +                              OVIRT_FOREIGN_MENU(user_data));
> +}
> +
> +static void
> +spice_ovirt_foreign_menu_update(RemoteViewer *self,
> +                                OvirtForeignMenu *foreign_menu)
> +{
> +    GHashTable *windows =
> virt_viewer_app_get_windows(VIRT_VIEWER_APP(self));
> +
> +    DEBUG_LOG("Spice foreign menu updated");
> +
> +    g_hash_table_foreach(windows, ovirt_foreign_menu_update_each,
> +                         foreign_menu);
> +}
> +#endif
> +
>  static void
>  foreign_menu_update(RemoteViewer *self, VirtViewerWindow *win)
>  {
> @@ -707,6 +752,14 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED
> RestProxyAuth *auth,
>  }
>  
>  
> +static void
> +ovirt_foreign_menu_changed(OvirtForeignMenu *foreign_menu,
> +                           GParamSpec *pspec G_GNUC_UNUSED,
> +                           VirtViewerApp *app)
> +{
> +    spice_ovirt_foreign_menu_update(REMOTE_VIEWER(app), foreign_menu);
> +}
> +
>  static gboolean
>  create_ovirt_session(VirtViewerApp *app, const char *uri)
>  {
> @@ -791,6 +844,14 @@ create_ovirt_session(VirtViewerApp *app, const char
> *uri)
>          goto error;
>      }
>  
> +    {
> +        OvirtForeignMenu *menu = ovirt_foreign_menu_new(proxy);
> +        g_object_set(G_OBJECT(menu), "api", api, "vm", vm, NULL);
> +        g_signal_connect(G_OBJECT(menu), "notify::files",
> +                         (GCallback)ovirt_foreign_menu_changed, app);

What about new windows that are created after the notify::files signal fires? As far as I can tell, they won't have a foreign menu unless the list of files changes.


> +        ovirt_foreign_menu_start(menu);
> +    }
> +
>      virt_viewer_app_set_connect_info(app, NULL, ghost, gport, gtlsport,
>                                       session_type, NULL, NULL, 0, NULL);
>  
> --
> 1.9.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>