Fix close of VNC displays

When clicking the close button on a virt-viewer window with
a VNC session open, while the VNC session terminates, the
window does not go away.

The problem is that the virt_viewer_session_vnc_disconnected
method never gets invoked. The close button triggers a call
to virt_viewer_session_clear_displays which unrefs the
VirtViewerDisplayVnc instance. This in turn triggers a call
to gtk_container_destroy, which destroys all widgets it
contains, ie the VncDisplay * object.

With the VncDisplay object in its dispose phase, no signals
will ever be emitted, thus the 'vnc-disconnected' signal
never gets seen.

The design issue is that VirtViewerDisplayVnc is assuming
it owns the VncDisplay, whereas in fact the real owner is
the VirtViewerSessionVnc object.

The solution is to introduce a new virt_viewer_display_close
method which can be used to de-parent the widget before
VirtViewerDisplay is unref'd.

The VirtViewerSessionVnc object also needs to hold a full ref
on the VncDisplay object, not merely a floating reference

* virt-viewer-display-spice.c, virt-viewer-display.c,
  virt-viewer-display.h: Add virt_viewer_display_close
* virt-viewer-display-vnc.c: Deparent VNC widget in
  virt_viewer_display_close impl
* virt-viewer-session-vnc.c: Improve logging
* virt-viewer-session.c: Call virt_viewer_display_close
  before unrefing display
* virt-viewer-window.c: Improve logging

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

7 files changed, 49 insertions, 3 deletions

diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
index 981fb57..985e116 100644
--- a/src/virt-viewer-display-spice.c
+++ b/src/virt-viewer-display-spice.c
@@ -46,6 +46,7 @@ static void virt_viewer_display_spice_send_keys(VirtViewerDisplay *display,
                                                 int nkeyvals);
 static GdkPixbuf *virt_viewer_display_spice_get_pixbuf(VirtViewerDisplay *display);
 static void virt_viewer_display_spice_release_cursor(VirtViewerDisplay *display);
+static void virt_viewer_display_spice_close(VirtViewerDisplay *display G_GNUC_UNUSED);
 
 static void
 virt_viewer_display_spice_finalize(GObject *obj)
@@ -69,6 +70,7 @@ virt_viewer_display_spice_class_init(VirtViewerDisplaySpiceClass *klass)
     dclass->send_keys = virt_viewer_display_spice_send_keys;
     dclass->get_pixbuf = virt_viewer_display_spice_get_pixbuf;
     dclass->release_cursor = virt_viewer_display_spice_release_cursor;
+    dclass->close = virt_viewer_display_spice_close;
 
     g_type_class_add_private(klass, sizeof(VirtViewerDisplaySpicePrivate));
 }
@@ -257,6 +259,12 @@ virt_viewer_display_spice_release_cursor(VirtViewerDisplay *display)
 }
 
 
+static void
+virt_viewer_display_spice_close(VirtViewerDisplay *display G_GNUC_UNUSED)
+{
+}
+
+
 /*
  * Local variables:
  *  c-indent-level: 4
diff --git a/src/virt-viewer-display-vnc.c b/src/virt-viewer-display-vnc.c
index 878740f..c0bcf13 100644
--- a/src/virt-viewer-display-vnc.c
+++ b/src/virt-viewer-display-vnc.c
@@ -39,6 +39,7 @@ struct _VirtViewerDisplayVncPrivate {
 
 static void virt_viewer_display_vnc_send_keys(VirtViewerDisplay* display, const guint *keyvals, int nkeyvals);
 static GdkPixbuf *virt_viewer_display_vnc_get_pixbuf(VirtViewerDisplay* display);
+static void virt_viewer_display_vnc_close(VirtViewerDisplay *display);
 
 static void
 virt_viewer_display_vnc_finalize(GObject *obj)
@@ -61,6 +62,7 @@ virt_viewer_display_vnc_class_init(VirtViewerDisplayVncClass *klass)
 
     dclass->send_keys = virt_viewer_display_vnc_send_keys;
     dclass->get_pixbuf = virt_viewer_display_vnc_get_pixbuf;
+    dclass->close = virt_viewer_display_vnc_close;
 
     g_type_class_add_private(klass, sizeof(VirtViewerDisplayVncPrivate));
 }
@@ -153,7 +155,6 @@ virt_viewer_display_vnc_new(VncDisplay *vnc)
     display = g_object_new(VIRT_VIEWER_TYPE_DISPLAY_VNC, NULL);
 
     g_object_ref(vnc);
-    g_object_ref(vnc); /* Because gtk_container_add steals the first ref */
     display->priv->vnc = vnc;
 
     gtk_container_add(GTK_CONTAINER(display), GTK_WIDGET(display->priv->vnc));
@@ -188,6 +189,19 @@ virt_viewer_display_vnc_new(VncDisplay *vnc)
     return GTK_WIDGET(display);
 }
 
+
+static void
+virt_viewer_display_vnc_close(VirtViewerDisplay *display)
+{
+    VirtViewerDisplayVnc *vnc = VIRT_VIEWER_DISPLAY_VNC(display);
+
+    /* We're not the real owner, so we shouldn't be letting the container
+     * destroy the widget. There are still signals that need to be
+     * propagated to the VirtViewerSession
+     */
+    gtk_container_remove(GTK_CONTAINER(display), GTK_WIDGET(vnc->priv->vnc));
+}
+
 /*
  * Local variables:
  *  c-indent-level: 4
diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c
index 40d23ad..49abf1c 100644
--- a/src/virt-viewer-display.c
+++ b/src/virt-viewer-display.c
@@ -579,6 +579,19 @@ void virt_viewer_display_release_cursor(VirtViewerDisplay *self)
     klass->release_cursor(self);
 }
 
+
+void virt_viewer_display_close(VirtViewerDisplay *self)
+{
+    VirtViewerDisplayClass *klass;
+
+    g_return_if_fail(VIRT_VIEWER_IS_DISPLAY(self));
+
+    klass = VIRT_VIEWER_DISPLAY_GET_CLASS(self);
+    g_return_if_fail(klass->close != NULL);
+
+    klass->close(self);
+}
+
 /*
  * Local variables:
  *  c-indent-level: 4
diff --git a/src/virt-viewer-display.h b/src/virt-viewer-display.h
index 07db5fd..ffbaf0e 100644
--- a/src/virt-viewer-display.h
+++ b/src/virt-viewer-display.h
@@ -75,6 +75,8 @@ struct _VirtViewerDisplayClass {
     GdkPixbuf *(*get_pixbuf)(VirtViewerDisplay *display);
     void (*release_cursor)(VirtViewerDisplay *display);
 
+    void (*close)(VirtViewerDisplay *display);
+
     /* signals */
     void (*display_pointer_grab)(VirtViewerDisplay *display);
     void (*display_pointer_ungrab)(VirtViewerDisplay *display);
@@ -112,6 +114,8 @@ void virt_viewer_display_set_auto_resize(VirtViewerDisplay *display, gboolean au
 gboolean virt_viewer_display_get_auto_resize(VirtViewerDisplay *display);
 void virt_viewer_display_release_cursor(VirtViewerDisplay *display);
 
+void virt_viewer_display_close(VirtViewerDisplay *display);
+
 G_END_DECLS
 
 #endif /* _VIRT_VIEWER_DISPLAY_H */
diff --git a/src/virt-viewer-session-vnc.c b/src/virt-viewer-session-vnc.c
index 075b1ce..3e27566 100644
--- a/src/virt-viewer-session-vnc.c
+++ b/src/virt-viewer-session-vnc.c
@@ -104,6 +104,7 @@ static void
 virt_viewer_session_vnc_disconnected(VncDisplay *vnc G_GNUC_UNUSED,
                                      VirtViewerSessionVnc *session)
 {
+    DEBUG_LOG("Disconnected");
     g_signal_emit_by_name(session, "session-disconnected");
     /* TODO perhaps? */
     /* virt_viewer_display_set_show_hint(VIRT_VIEWER_DISPLAY(session->priv->vnc), */
@@ -234,6 +235,7 @@ virt_viewer_session_vnc_close(VirtViewerSession* session)
 
     g_return_if_fail(self != NULL);
 
+    DEBUG_LOG("close vnc=%p", self->priv->vnc);
     if (self->priv->vnc != NULL) {
         virt_viewer_session_clear_displays(session);
         vnc_display_close(self->priv->vnc);
@@ -241,6 +243,7 @@ virt_viewer_session_vnc_close(VirtViewerSession* session)
     }
 
     self->priv->vnc = VNC_DISPLAY(vnc_display_new());
+    g_object_ref_sink(self->priv->vnc);
 
     g_signal_connect(self->priv->vnc, "vnc-connected",
                      G_CALLBACK(virt_viewer_session_vnc_connected), session);
@@ -271,6 +274,7 @@ virt_viewer_session_vnc_new(GtkWindow *main_window)
     session = g_object_new(VIRT_VIEWER_TYPE_SESSION_VNC, NULL);
 
     session->priv->vnc = VNC_DISPLAY(vnc_display_new());
+    g_object_ref_sink(session->priv->vnc);
     session->priv->main_window = g_object_ref(main_window);
 
     g_signal_connect(session->priv->vnc, "vnc-connected",
diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
index 51a8a5a..18b6922 100644
--- a/src/virt-viewer-session.c
+++ b/src/virt-viewer-session.c
@@ -304,8 +304,10 @@ void virt_viewer_session_clear_displays(VirtViewerSession *session)
     GList *tmp = session->priv->displays;
 
     while (tmp) {
-        g_signal_emit_by_name(session, "session-display-removed", tmp->data);
-        g_object_unref(tmp->data);
+        VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(tmp->data);
+        g_signal_emit_by_name(session, "session-display-removed", display);
+        virt_viewer_display_close(display);
+        g_object_unref(display);
         tmp = tmp->next;
     }
     g_list_free(session->priv->displays);
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index b17499b..771a8b9 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -651,6 +651,7 @@ virt_viewer_window_delete(GtkWidget *src G_GNUC_UNUSED,
                           void *dummy G_GNUC_UNUSED,
                           VirtViewerWindow *self)
 {
+    DEBUG_LOG("Window closed");
     virt_viewer_app_window_set_visible(self->priv->app, self, FALSE);
     return TRUE;
 }