Created attachment 136480 [details] [review]
patch

Created attachment 138489 [details] [review]
updated patch against 2.27.4

Matthias: has there been any movement on the better ConsoleKit API for shutdown? I quickly looked at the gdm and ConsoleKit list archives and I don't see anything :/ I'm a bit reluctant to commit the current patch in its current form.

(btw, you can remove try try_system_restart() and try_system_stop() too in the patch)

No, I haven't found anybody to push this on, and didn't have the cycles to do it myself...

So we're stuck here for 2.28? :-/

yeah.

I blame session saving.

Err, I committed the patch yesterday before rolling the tarball :-) Just forgot to comment there that we keep the bug open to track the remaining issues because of CK limitations.

Basic stuff in --> removing gnome-target

https://bugs.freedesktop.org/show_bug.cgi?id=24493 has ConsoleKit patches

*** Bug 603783 has been marked as a duplicate of this bug. ***

Created attachment 151899 [details] [review]
First pass at using the CK API described in fdo bug

Here's a first pass at the patch.

I'm pretty sure it doesn't work since I didn't even test it (I didn't compile ConsoleKit with the patches), but at least it compiles. And hopefully, it shouldn't be too bad.

A few comments:

 + the patch ignores the inhibits that CK gives us when it returns from QueryStop, but we have the number of inhibits, so we could easily do something (look for TODO in the code). So assuming the code works fine, it might look like there's no change compared to before. Except that if you cancel the PK authentication for shutdown/reboot, gnome-session should go back to the running phase.

 + many FIXME and a few TODO here and there. It's mostly some notes to myself to make sure it's as solid as possible.

 + I'm not sure how to handle a CK daemon that doesn't have the new API. We could just ignore it, I guess. Actually, I'm not even sure how to detect the case.

 + should we display something if there are other sessions, but no inhibitor at all?

 + if the restart/shutdown is blocked because of a CK inhibitor and only because of this, we might want to monitor CK and continue the restart/shutdown once the CK inhibitor is gone. I didn't look if there's API in CK for this in the CK patches. (Or well, on a more general note, we'd want to update the inhibit dialog anyway)

Created attachment 153126 [details] [review]
first round of fixes

Hey Vincent, here is a update of your patch that fixes some of the issues I found while trying it against the rebased CK patches that I put in the CK bug.

The most important fix was that some of the new api is actually on the Session interface, not on Manager.

But the patch still has some fundamental issues:

It doesn't follow the outline in the CK docs:

      <orderedlist>
        <listitem><para>User initiates Shutdown operation</para></listitem>
        <listitem><para>Session leader calls QueryStop() on the ConsoleKit manager object</para></listitem>
        <listitem><para>ConsoleKit emits the QueryStop signal on all session objects</para></listitem>
        <listitem><para>All sessions enter their QueryEnd phase</para></listitem>
        <listitem><para>All session leaders respond back with StopResponse()</para></listitem>
        <listitem><para>The initial QueryStop() call returns</para></listitem>
        <listitem><para>Session leader uses GetSessions() and GetInhibitors() to assemble information to present to the user, in addition to its own inhibitors. Depending on the can_proceed value returned by QueryStop(), the dialog will offer the user to proceed with the shutdown operation</para></listitem>
        <listitem><para>Assuming the user chooses to proceed, the session leader calls Stop() or Restart()</para></listitem>
        <listitem><para>ConsoleKit sets the Stopping property</para></listitem>
        <listitem><para>ConsoleKit verifies that the user has the necessary privileges</para></listitem>
        <listitem><para>ConsoleKit emits the Stop signal on all session obj        <listitem><para>Session leader uses GetSessions() and GetInhibitors() to assemble information to present to the user, in addition to its own inhibitors. Depending on the can_proceed value returned by QueryStop(), the dialog will offer the user to proceed with the shutdown operation</para></listitem>ects</para></listitem>
        <listitem><para>All sessions enter their End phase</para></listitem>
        <listitem><para>All session leaders respond back with StopResponse()</para></listitem>
        <listitem><para>ConsoleKit shuts down the machine</para></listitem>

What your patch does is roughly the following:

- User initiates Shutdown
- Session leader enters QueryEnd phase
- QueryEnd phase ends
- Session leader calls QueryStop on ConsoleKit
- ConsoleKit emits QueryStop signal
- Session leader responds right away, other sessions enter their QueryEnd phase
...

One consequence of this order is that you don't get an inhibit dialog anymore when the QueryEnd phase is entered for another reason (eg the user initiates a logout instead of a shutdown), since you moved the showing of the inhibit dialog from query_end_session_complete to on_consolekit_query_stop_completed (where we never go in the logout case).

Created attachment 153265 [details] [review]
basically working now

*** Bug 608367 has been marked as a duplicate of this bug. ***

I consider this work obsolete now, considering ConsoleKit is on the way out.