[Spice-devel] [PATCH spice-gtk 2/4] Add a desktop-integration helper class

[Marc-André Lureau]

Hi

----- Mensaje original -----
> > It would be more reliable to have a --with-desktop=<gnome/none> and
> > checking all the needed dependencies.
> 
> That is not going to work, as once we support multiple desktops we
> will want to
> compile in support for more then one. So that the same binary can run
> under
> gnome and under say kde, and do the right thing in both cases.

Yes, indeed.

> Also note that the code has been written so as to *not* complain
> (only g_debug)
> when things fail, to allow running under none gnome without polluting
> the console
> with error messages, this is by design.

ok

> #ifdef USE_DBUS
>      gnome_integration_inhibit_automount(self, toplevel_window_id);
>      kde_integration_inhibit_automount(self, toplevel_window_id);
> #endif
> }
> 
> Yes both will get called, and one of them will simply do nothing,
> just like
> gnome_integration_inhibit_automount now does nothing when not running
> under
> gnome-session.

ok

> The problem then becomes defining "nothing is done there", what if we
> are
> compiled with dbus but not running under a supported desktop
> environment?
> 
> I guess I could add a warning to spice_desktop_integration_init when
> we are
> missing desktop integration for the users environment, should this be
> a
> warning, or a debug ?

A warning or a debug, but not something silent.

> >> +if test "x$enable_dbus" != "xno"; then
> >> +  PKG_CHECK_MODULES([DBUS_GLIB], [dbus-glib-1],
> >
> > Since we are targetting GNOME3 with those changes, we shouldn't use
> > dbus-glib which has been deprecated for a while in favour of gdbus
> 
> We are targeting gnome >= 2.28 (and yes gnome-session in 2.28 does
> have
> the Inhibitor interface), so again this was a conscious choice, I
> would
> rather have 1 implementation then 2, until we drop support for gnome
> 2.28.

Argh, rhel6 is still glib 2.22, and gdbus is 2.26. ok :)

> > Finally, I think the GData list isn't very appropriate structure
> > for
> > mapping int -> pointer, why not use a hashtable?
> 
> Because:
> 1) GData is meant exactly for mapping uints (quarks) to pointers, iow
> it
> is being used as intended (well I need mapping (unique) uints to
> uints, but
> mapping uints to pointers does the job nicely).

quarks are "non-zero integer which uniquely identifies a particular string", not exactly uint.

> 2) Using a hashtable would needlessly complicate the code, since a
> hashtable
> is not intended for this.

Many people use it to do int -> pointer (and also int -> int G(U)INT_TO_POINTER)

> > My understanding of this logic could also be made with a refcount.
> No we need to cookie associated to the specific toplevel window id to
> uninhibit.

ah right, /blame me, I didn't spend enough time reviewing the code..

Could we simplify the API and the logic if we used a hidden window? have you thought about it? That way, we don't need to pass a window id around, we don't need to call (un)inhibit many times, we don't need to maintain int->int mapping, we only keep one cookie and a refcount.