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

[Marc-André Lureau]

----- Mensaje original -----
> Hi,
> 
> On 06/22/2012 02:37 PM, Marc-André Lureau wrote:
> 
> <snip?
> 
> >> 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.
> 
> Well, debug is silent by default, so I guess a warning?
> 
> <snip>
> 
> >>> 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.
> 
> And windows id's are non-zero integers which uniquely identify a
> particular window.
> >> 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)
> 
> Just because many people use hash-tables for something does not make
> them the
> right choice, I looked at using hash-tables first, before I choose to
> use
> GData and I found GData to be a better match.

Right, but if nobody does something this way, I am very suspicious :)

GData list seems to be a simple growing array of elements with GQuark key, and the API includes "gchar *key". Not really fond of using something meant for various string representations for windows xid.

I don't see why a GHashTable would be more complicated.

> >>> 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.
> 
> Erm, no, the whole purpose of the toplevel window id is so that if
> the window goes
> away gnome-session can automatically uninhibit if the app does not do
> so properly,
> keeping a hidden window around for this would break this.

By go away, you mean destroyed? I suppose the manager doesn't care if the window is shown/visible or not, but that it exists and is destroyed either intentionally, or unintentionally after abort etc.. I don't see the need to pass around each and every toplevel window to the manager (visible or not)