Re: Screensaver Solution Discussion

[Brian Cameron <Brian Cameron Sun COM>]

Jeff:

On 02/09/10 03:28, Jeff Cai wrote:
> On Mon, 2010-02-08 at 19:00 -0600, Brian Cameron wrote:
> > gnome-session-worker:
> > > Currently gdm-session-worker supports several working states:
> > SETUP_COMPLETE->AUTHENTICATED->AUTHORIZED->ACCREDITED->SESSION_OPENED->SESSION_STARTED
> > > For gnome-screensaver, we need to add some new states in the state
> > diagram:
> > >
> > SCREENSAVER_SETUP_COMPLETE->SCREENSAVER_AUTHENTICATED->SCREENSAVER_AUTHORIZED.
> >
> > Is it really necessary to define new states?  Since GDM uses GObject
> > object-oriented techniques, I would think that it might make more
> > sense
> > to manage this via making two subclasses of a base class.  The
> > lockscreen subclass could just have no-ops for the states that are not
> > meaningful in the lockscreen case.
>
> Since gdm-session-worker handles the DBus signals of
> org.gnome.DisplayManager.Session in a state transition function, It
> looks natural to add a new state transition function with more states.
> It may look like monitoring the new signals come from Screensaver
> org.gnome.ScreenSaver.

Having a base "worker" class where common code is defined, and a
separate subclass for both the login and the screensaver worker may be
a more elegant way to design the code.  Since much of the code is common
(such as PAM interaction), a fair amount of code would probably remain
in the base class.  However, things like auditing are different, and
would make sense to implement in the subclass.

However, we are at the prototyping stage, so it probably makes the most
sense to use whichever approach is most expedient to get a working
prototype for review ready.  So, if you think that avoiding the use of
object oriented techniques will make for more rapid development, then
I think we should move forward with whatever approach is fastest and
refactor the code later if necessary.

Whether or not subclassing is a good idea would be easier to determine
once we know how intrusive the changes which are needed to make this
work.  If the changes needed to make this work make the worker overly
complicated in a way that could be resolved by making use of object-
oriented techniques, then this could be determined when a working patch
is ready for upstream review.

Another comment.  It would make sense for the GDM "screen locking"
feature to be configurable.  Some distros or users may wish to use
their own screen lock program.  Some might want to use xscreensaver
or something.  Therefore, I think it makes sense for this GDM "screen
locking" feature to be configurable.  It could be enabled by default,
but users who want to use other screen locking mechanisms should be able
to configure their system to do so.

Brian