[Spice-devel] [PATCH spice-gtk 2/3] Add a SpiceGtkSession Class
Marc-André Lureau
marcandre.lureau at gmail.com
Tue Oct 4 15:04:19 PDT 2011
hi
On Tue, Oct 4, 2011 at 4:20 PM, Hans de Goede <hdegoede at redhat.com> wrote:
> This initial commit of the SpiceGtkSession Class only adds the empty
> class and the 1:1 linkage to SpiceSession through 2 new private methods
> added to SpiceSession: spice_session_{get|set}_gtk_session.
>
> The following commits will move things which are currently per SpiceDisplay,
> but which really should be global, such as the clipboard, over to
> SpiceGtkSession.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> doc/reference/spice-gtk-docs.xml | 1 +
> doc/reference/spice-gtk-sections.txt | 18 +++
> doc/reference/spice-gtk.types | 2 +
> gtk/Makefile.am | 3 +
> gtk/map-file | 4 +
> gtk/spice-gtk-session.c | 220 ++++++++++++++++++++++++++++++++++
> gtk/spice-gtk-session.h | 63 ++++++++++
> gtk/spice-session-priv.h | 8 ++
> gtk/spice-session.c | 25 ++++
> 9 files changed, 344 insertions(+), 0 deletions(-)
> create mode 100644 gtk/spice-gtk-session.c
> create mode 100644 gtk/spice-gtk-session.h
>
> diff --git a/doc/reference/spice-gtk-docs.xml b/doc/reference/spice-gtk-docs.xml
> index c7f205b..2b4336d 100644
> --- a/doc/reference/spice-gtk-docs.xml
> +++ b/doc/reference/spice-gtk-docs.xml
> @@ -40,6 +40,7 @@
>
> <chapter>
> <title>GTK Widget, from spice-client-gtk</title>
> + <xi:include href="xml/spice-gtk-session.xml"/>
> <xi:include href="xml/spice-widget.xml"/>
> </chapter>
>
> diff --git a/doc/reference/spice-gtk-sections.txt b/doc/reference/spice-gtk-sections.txt
> index d789d5a..d5e8e70 100644
> --- a/doc/reference/spice-gtk-sections.txt
> +++ b/doc/reference/spice-gtk-sections.txt
> @@ -287,6 +287,24 @@ SpiceUsbDeviceManagerPrivate
> </SECTION>
>
> <SECTION>
> +<FILE>spice-gtk-session</FILE>
> +<TITLE>SpiceGtkSession</TITLE>
> +SpiceGtkSession
> +SpiceGtkSessionClass
> +spice_gtk_session_get
> +<SUBSECTION Standard>
> +SPICE_GTK_SESSION
> +SPICE_IS_GTK_SESSION
> +SPICE_TYPE_GTK_SESSION
> +spice_gtk_session_get_type
> +SPICE_GTK_SESSION_CLASS
> +SPICE_IS_GTK_SESSION_CLASS
> +SPICE_GTK_SESSION_GET_CLASS
> +<SUBSECTION Private>
> +SpiceGtkSectionPrivate
> +</SECTION>
> +
> +<SECTION>
> <FILE>spice-widget</FILE>
> <TITLE>SpiceDisplay</TITLE>
> SpiceDisplay
> diff --git a/doc/reference/spice-gtk.types b/doc/reference/spice-gtk.types
> index d8e0f28..a88ece1 100644
> --- a/doc/reference/spice-gtk.types
> +++ b/doc/reference/spice-gtk.types
> @@ -13,6 +13,7 @@
> #include "channel-record.h"
> #include "channel-smartcard.h"
> #include "channel-usbredir.h"
> +#include "spice-gtk-session.h"
> #include "spice-widget.h"
> #include "spice-grabsequence.h"
> #include "smartcard-manager.h"
> @@ -25,6 +26,7 @@ spice_cursor_channel_get_type
> spice_display_channel_get_type
> spice_display_get_type
> spice_grab_sequence_get_type
> +spice_gtk_session_get_type
> spice_inputs_channel_get_type
> spice_inputs_lock_get_type
> spice_main_channel_get_type
> diff --git a/gtk/Makefile.am b/gtk/Makefile.am
> index 7d197f6..f4b595f 100644
> --- a/gtk/Makefile.am
> +++ b/gtk/Makefile.am
> @@ -97,6 +97,7 @@ SPICE_GTK_LIBADD_COMMON = \
> $(NULL)
>
> SPICE_GTK_SOURCES_COMMON = \
> + spice-gtk-session.c \
> spice-widget.c \
> spice-widget-priv.h \
> vncdisplaykeymap.c \
> @@ -134,6 +135,7 @@ endif
>
> libspice_client_gtkincludedir = $(includedir)/spice-client-gtk-$(SPICE_GTK_API_VERSION)
> libspice_client_gtkinclude_HEADERS = \
> + spice-gtk-session.h \
> spice-widget.h \
> spice-grabsequence.h \
> $(NULL)
> @@ -562,6 +564,7 @@ glib_introspection_files = \
> gtk_introspection_files = \
> $(libspice_client_gtkinclude_HEADERS) \
> $(nodist_libspice_client_gtkinclude_HEADERS) \
> + spice-gtk-session.c \
> spice-widget.c \
> spice-grabsequence.c \
> $(NULL)
> diff --git a/gtk/map-file b/gtk/map-file
> index b383edf..bedfdb9 100644
> --- a/gtk/map-file
> +++ b/gtk/map-file
> @@ -61,12 +61,16 @@ spice_record_send_data;
> spice_session_connect;
> spice_session_disconnect;
> spice_session_get_channels;
> +spice_session_get_gtk_session;
> spice_session_get_type;
> spice_session_has_channel_type;
> spice_session_migration_get_type;
> spice_session_new;
> spice_session_open_fd;
> +spice_session_set_gtk_session;
> spice_session_verify_get_type;
> +spice_gtk_session_get;
> +spice_gtk_session_get_type;
> spice_set_session_option;
> spice_smartcard_channel_get_type;
> spice_smartcard_manager_get;
> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
> new file mode 100644
> index 0000000..a4bc106
> --- /dev/null
> +++ b/gtk/spice-gtk-session.c
> @@ -0,0 +1,220 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2010-2011 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + This library 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
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#include "config.h"
> +#include "spice-session-priv.h"
> +#include "spice-gtk-session.h"
> +
> +struct _SpiceGtkSessionPrivate {
> + SpiceSession *session;
> +};
> +
> +/**
> + * SECTION:spice-gtk-session
> + * @short_description: handles GTK connection details
> + * @title: Spice GTK Session
> + * @section_id:
> + * @see_also: #SpiceSession, and the GTK widget #SpiceDisplay
> + * @stability: Stable
> + * @include: spice-gtk-session.h
> + *
> + * The #SpiceGtkSession class is the spice-client-gtk counter part of
> + * #SpiceSession. It contains functionality which should be handled per
> + * session rather then per #SpiceDisplay (one session can have multiple
> + * displays), but which cannot live in #SpiceSession as it depends on
> + * GTK. For example the clipboard functionality.
> + *
> + * There should always be a 1:1 relation between #SpiceGtkSession objects
> + * and #SpiceSession objects. Therefor there is no spice_gtk_session_new,
> + * instead there is spice_gtk_session_get() which ensures this 1:1 relation.
> + *
> + * #SpiceDisplay uses #SpiceGtkSession internally, some #SpiceDisplay
> + * properties map directly to #SpiceGtkSession properties, this means that
> + * changing them for one #SpiceDisplay changes them for all displays.
> + *
> + * Depending on your UI, you may want to not show these properties on a
> + * per display basis and instead show them in a global settings menu which
> + * directly uses SpiceGtkSession.
> + */
> +
> +/* ------------------------------------------------------------------ */
> +/* gobject glue */
> +
> +#define SPICE_GTK_SESSION_GET_PRIVATE(obj) \
> + (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_GTK_SESSION, SpiceGtkSessionPrivate))
> +
> +G_DEFINE_TYPE (SpiceGtkSession, spice_gtk_session, G_TYPE_OBJECT);
> +
> +/* Properties */
> +enum {
> + PROP_0,
> + PROP_SESSION,
> +};
> +
> +static void spice_gtk_session_init(SpiceGtkSession *gtk_session)
> +{
> + SpiceGtkSessionPrivate *s;
> +
> + SPICE_DEBUG("New gtk session (compiled from package " PACKAGE_STRING ")");
Well we already have version information somewhere, so you may drop
this line to avoid redundancy in the log. Not a big deal though ;)
> + s = gtk_session->priv = SPICE_GTK_SESSION_GET_PRIVATE(gtk_session);
> + memset(s, 0, sizeof(*s));
I know this is used all over the place, but now that I am acutally
reviewing it I actually can check it.
gtype actually instantiate with g_slice_alloc0 (ALIGN_STRUCT
(total_instance_size) + node->data->instance.private_size);
So there is no need for memset. Feel free to commit a seperate patch
after for cleanup in the spice-gtk codebase. Thanks :)
> +}
> +
> +static GObject *
> +spice_gtk_session_constructor(GType gtype,
> + guint n_properties,
> + GObjectConstructParam *properties)
> +{
> + GObject *obj;
> + SpiceGtkSession *gtk_session;
> +
> + {
> + /* Always chain up to the parent constructor */
> + GObjectClass *parent_class;
> + parent_class = G_OBJECT_CLASS(spice_gtk_session_parent_class);
> + obj = parent_class->constructor(gtype, n_properties, properties);
> + }
> +
> + gtk_session = SPICE_GTK_SESSION(obj);
> + if (!gtk_session->priv->session)
> + g_error("SpiceGtKSession constructed without a session");
> +
> + return obj;
> +}
> +
In term of style you could simply:
GObject *self =
G_OBJECT_CLASS(spice_gtk_session_parent_class)->constructor(gtype,
n_properties, properties);
g_warn_if_fail (SPICE_GTK_SESSION(self)->priv->session);
return self;
Still, you will need to deal with that situation later on. So perhaps
best just to have the check before calling g_object_new():
spice_gtk_session_get(SpiceSession *session) {
g_return_val_if_fail (SPICE_IS_SESSION (session), NULL);
...
}
> +static void spice_gtk_session_dispose(GObject *gobject)
> +{
> +#if 0 /* Unused for now */
> + SpiceGtkSession *gtk_session = SPICE_GTK_SESSION(gobject);
> + SpiceGtkSessionPrivate *s = SPICE_GTK_SESSION_GET_PRIVATE(gtk_session);
> +#endif
> + /* release stuff */
> +
> + /* Chain up to the parent class */
> + if (G_OBJECT_CLASS(spice_gtk_session_parent_class)->dispose)
> + G_OBJECT_CLASS(spice_gtk_session_parent_class)->dispose(gobject);
> +}
> +
> +static void spice_gtk_session_finalize(GObject *gobject)
> +{
> +#if 0 /* Unused for now */
> + SpiceGtkSession *gtk_session = SPICE_GTK_SESSION(gobject);
> + SpiceGtkSessionPrivate *s = SPICE_GTK_SESSION_GET_PRIVATE(gtk_session);
> +#endif
> +
> + /* release stuff */
> +
> + /* Chain up to the parent class */
> + if (G_OBJECT_CLASS(spice_gtk_session_parent_class)->finalize)
> + G_OBJECT_CLASS(spice_gtk_session_parent_class)->finalize(gobject);
> +}
> +
I am tempted to ask you to remove those methods.
> +static void spice_gtk_session_get_property(GObject *gobject,
> + guint prop_id,
> + GValue *value,
> + GParamSpec *pspec)
> +{
> + SpiceGtkSession *gtk_session = SPICE_GTK_SESSION(gobject);
> + SpiceGtkSessionPrivate *s = SPICE_GTK_SESSION_GET_PRIVATE(gtk_session);
> +
> + switch (prop_id) {
> + case PROP_SESSION:
> + g_value_set_object(value, s->session);
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> + break;
> + }
> +}
> +
> +static void spice_gtk_session_set_property(GObject *gobject,
> + guint prop_id,
> + const GValue *value,
> + GParamSpec *pspec)
> +{
> + SpiceGtkSession *gtk_session = SPICE_GTK_SESSION(gobject);
> + SpiceGtkSessionPrivate *s = SPICE_GTK_SESSION_GET_PRIVATE(gtk_session);
> +
> + switch (prop_id) {
> + case PROP_SESSION:
> + s->session = g_value_get_object(value);
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> + break;
> + }
> +}
> +
> +static void spice_gtk_session_class_init(SpiceGtkSessionClass *klass)
> +{
> + GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
> +
> + gobject_class->constructor = spice_gtk_session_constructor;
> + gobject_class->dispose = spice_gtk_session_dispose;
> + gobject_class->finalize = spice_gtk_session_finalize;
> + gobject_class->get_property = spice_gtk_session_get_property;
> + gobject_class->set_property = spice_gtk_session_set_property;
> +
> + /**
> + * SpiceGtkSession:session:
> + *
> + * #SpiceSession this #SpiceGtkSession is associated with
> + *
> + **/
> + g_object_class_install_property
> + (gobject_class, PROP_SESSION,
> + g_param_spec_object("session",
> + "Session",
> + "SpiceSession",
> + SPICE_TYPE_SESSION,
> + G_PARAM_READWRITE |
> + G_PARAM_CONSTRUCT_ONLY |
> + G_PARAM_STATIC_STRINGS));
> +
> + g_type_class_add_private(klass, sizeof(SpiceGtkSessionPrivate));
> +}
> +
> +/* ------------------------------------------------------------------ */
> +/* public functions */
> +
> +/**
> + * spice_gtk_session_get:
> + * @session: #SpiceSession for which to get the #SpiceGtkSession
> + *
> + * Gets the #SpiceGtkSession associated with the passed in #SpiceSession,
> + * creating a #SpiceGtkSession for the session if necessary. Note that this
> + * function returns a weak reference, which should not be used after the
> + * #SpiceSession itself has been unref-ed by the caller.
> + *
> + * Returns: (transfer none): a weak reference to the #SpiceGtkSession associated with the passed in #SpiceSession
> + **/
> +SpiceGtkSession *spice_gtk_session_get(SpiceSession *session)
> +{
> + GObject *gtk_session;
> +
> + gtk_session = spice_session_get_gtk_session(session);
> + if (gtk_session == NULL) {
> + gtk_session = g_object_new(SPICE_TYPE_GTK_SESSION,
> + "session", session,
> + NULL);
> + spice_session_set_gtk_session(session, gtk_session);
> + }
> +
> + return SPICE_GTK_SESSION(gtk_session);
> +}
> diff --git a/gtk/spice-gtk-session.h b/gtk/spice-gtk-session.h
> new file mode 100644
> index 0000000..9c59fa2
> --- /dev/null
> +++ b/gtk/spice-gtk-session.h
> @@ -0,0 +1,63 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2010-2011 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + This library 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
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, see <http://www.gnu.org/licenses/>.
> +*/
> +#ifndef __SPICE_CLIENT_GTK_SESSION_H__
> +#define __SPICE_CLIENT_GTK_SESSION_H__
> +
> +#include "spice-client.h"
> +
> +G_BEGIN_DECLS
> +
> +#define SPICE_TYPE_GTK_SESSION (spice_gtk_session_get_type ())
> +#define SPICE_GTK_SESSION(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), SPICE_TYPE_GTK_SESSION, SpiceGtkSession))
> +#define SPICE_GTK_SESSION_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), SPICE_TYPE_GTK_SESSION, SpiceGtkSessionClass))
> +#define SPICE_IS_GTK_SESSION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), SPICE_TYPE_GTK_SESSION))
> +#define SPICE_IS_GTK_SESSION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), SPICE_TYPE_GTK_SESSION))
> +#define SPICE_GTK_SESSION_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), SPICE_TYPE_GTK_SESSION, SpiceGtkSessionClass))
> +
> +typedef struct _SpiceGtkSession SpiceGtkSession;
> +typedef struct _SpiceGtkSessionClass SpiceGtkSessionClass;
> +typedef struct _SpiceGtkSessionPrivate SpiceGtkSessionPrivate;
> +
> +struct _SpiceGtkSession
> +{
> + GObject parent;
> + SpiceGtkSessionPrivate *priv;
> + /* Do not add fields to this struct */
> +};
> +
> +struct _SpiceGtkSessionClass
> +{
> + GObjectClass parent_class;
> +
> + /* signals */
> +
> + /*< private >*/
> + /*
> + * If adding fields to this struct, remove corresponding
> + * amount of padding to avoid changing overall struct size
> + */
> + gchar _spice_reserved[SPICE_RESERVED_PADDING];
> +};
> +
> +GType spice_gtk_session_get_type(void);
> +
> +SpiceGtkSession *spice_gtk_session_get(SpiceSession *session);
> +
> +G_END_DECLS
> +
> +#endif /* __SPICE_CLIENT_GTK_SESSION_H__ */
> diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
> index 2815ed2..30b3c17 100644
> --- a/gtk/spice-session-priv.h
> +++ b/gtk/spice-session-priv.h
> @@ -61,6 +61,14 @@ void spice_session_get_caches(SpiceSession *session,
> void spice_session_palettes_clear(SpiceSession *session);
> void spice_session_images_clear(SpiceSession *session);
>
> +/*
> + * For use by SpiceGtkSession to ensure a 1:1 relation between SpiceGtkSessions
> + * and SpiceSessions
> + */
> +GObject *spice_session_get_gtk_session(SpiceSession *session);
> +void spice_session_set_gtk_session(SpiceSession *session,
> + GObject *spice_gtk_session);
> +
> G_END_DECLS
>
> #endif /* __SPICE_CLIENT_SESSION_PRIV_H__ */
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index 3ce80d5..6e53826 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -81,6 +81,7 @@ struct _SpiceSessionPrivate {
> display_cache images;
> display_cache palettes;
> SpiceGlzDecoderWindow *glz_window;
> + GObject *spice_gtk_session;
> };
>
> /**
> @@ -194,6 +195,11 @@ spice_session_dispose(GObject *gobject)
> s->migration_left = NULL;
> }
>
> + if (s->spice_gtk_session) {
> + g_object_unref(s->spice_gtk_session);
> + s->spice_gtk_session = NULL;
> + }
> +
> /* Chain up to the parent class */
> if (G_OBJECT_CLASS(spice_session_parent_class)->dispose)
> G_OBJECT_CLASS(spice_session_parent_class)->dispose(gobject);
> @@ -1506,3 +1512,22 @@ void spice_session_get_caches(SpiceSession *session,
> if (glz_window)
> *glz_window = s->glz_window;
> }
> +
> +GObject *spice_session_get_gtk_session(SpiceSession *session)
> +{
> + SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
> +
> + g_return_val_if_fail(s != NULL, NULL);
> +
> + return s->spice_gtk_session;
> +}
> +
> +void spice_session_set_gtk_session(SpiceSession *session,
> + GObject *spice_gtk_session)
> +{
> + SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
> +
> + g_return_if_fail(s != NULL);
> +
> + s->spice_gtk_session = spice_gtk_session;
> +}
If we don't want to make this API public at all, we could also use
gobject_{set,get}_data_full(), and the spice_session_set_gtk_session()
functions as part of the gtk library. What do you think?
> 1.7.6.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
--
Marc-André Lureau
More information about the Spice-devel
mailing list