[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: activity sessions in ksmserver and kwin
From: "Lubos Lunak" <l.lunak () kde ! org>
Date: 2010-10-20 9:57:56
Message-ID: 20101020095756.10041.91234 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5638/#review8254
-----------------------------------------------------------
Ship it!
Looks ok.
- Lubos
On 2010-10-18 12:20:20, Chani Armitage wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5638/
> -----------------------------------------------------------
> =
> (Updated 2010-10-18 12:20:20)
> =
> =
> Review request for kwin, Plasma and Lubos Lunak.
> =
> =
> Summary
> -------
> =
> so, this is the activity session code.
> it's not 100% complete, because kwin has to fake a few things until the a=
ctivities service gets its new api - but ivan's working on that. :) there w=
on't be any UI for it until that api's in anyways.
> =
> kwin handles the mapping from activity id -> windows -> session ids, beca=
use I had to call it anyways, and it's much easier to calculate from inside=
kwin.
> =
> The good:
> it can save and restore processes and their windows' activity associations
> all the regular xsmp stuff seems to be working, except cancel.
> I've done it by merely adding a few dbus methods, no changes to xsmp itse=
lf
> ksmserver doesn't know or care what activities are; even the kwin code is=
separated out into "find the clients for this activity" and "save a sessio=
n for these clients"
> =
> The bad:
> since ksmserver doesn't tell the wm or clients that anything special is h=
appening, it can't tell kwin "save the subsession data", or tell clients "c=
lose only window foo". for kwin this is actually a nice excuse to have it d=
o the mapping work, but for clients it means we can't close anything until =
we're ready to close everything. Clients would have to be made context-awar=
e either way to react to such a request; for 4.7 I might see if I can do it=
by extending xsmp or augmenting it with more dbus (instead of having the c=
lient do all the work after the session's closed, as I originally planned).
> =
> kwrite's cancel button doesn't cancel the session-close. it works on logo=
ut, so there must be a bug in my code.
> =
> session data isn't deleted when an activity is deleted (I forgot). I'd li=
ke to get this patch in ASAP though, and add simple details like that later=
- the patch is already quite big enough ;)
> =
> there's some "legacy" session code, I haven't read it all and I haven't i=
mplemented support for it yet. are there any apps using it? does it matter?=
what *is* this legacy session thing?
> =
> The ugly:
> there are some sync issues in here.
> first of all, we have a conflict between save-on-close and save-regularly=
. all xsmp stuff is save-on-close, whereas the activity service and plasma =
(and other modern apps) save quite often. If you open an activity and then =
X crashes, you've lost all session data for that activity. :(
> =
> I'm planning to try fixing that by saving (but not quitting) the login se=
ssion when an activity is opened/closed. I'm not sure how well it'd work, b=
ut it should at least keep the correct list of processes to start.
> =
> the second sync issue comes from the fact that a window can be open on a =
closed activity. I'll probably prevent the user from adding a window to a c=
losed activity, but windows that are already shared across activities... we=
ll, they could behave in unexpected ways. I think I'd like to do more tests=
before deciding how to handle that - but one thing that could help, perhap=
s, is storing the kwin session info in one big pool for all closed activiti=
es instead of a separate group for each activity.
> =
> =
> Diffs
> -----
> =
> trunk/KDE/kdebase/workspace/ksmserver/server.h 1186160 =
> trunk/KDE/kdebase/workspace/ksmserver/legacy.cpp 1186160 =
> trunk/KDE/kdebase/workspace/ksmserver/org.kde.KSMServerInterface.xml 11=
86160 =
> trunk/KDE/kdebase/workspace/ksmserver/server.cpp 1186160 =
> trunk/KDE/kdebase/workspace/ksmserver/shutdown.cpp 1186160 =
> trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1186160 =
> trunk/KDE/kdebase/workspace/kwin/org.kde.KWin.xml 1186160 =
> trunk/KDE/kdebase/workspace/kwin/sm.cpp 1186160 =
> trunk/KDE/kdebase/workspace/kwin/workspace.h 1186160 =
> =
> Diff: http://svn.reviewboard.kde.org/r/5638/diff
> =
> =
> Testing
> -------
> =
> I can use dbus to save/restore an activity, like I showed in my screencas=
t ( http://chani.wordpress.com/2010/10/04/activities-fun-with-screencast ).=
I haven't done much testing of multi-window processes, though, or saving a=
nd restoring multiple activities.
> =
> =
> Thanks,
> =
> Chani
> =
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5638/">http://svn.reviewboard.kde.org/r/5638/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks ok.</pre> <br />
<p>- Lubos</p>
<br />
<p>On October 18th, 2010, 12:20 p.m., Chani Armitage wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin, Plasma and Lubos Lunak.</div>
<div>By Chani Armitage.</div>
<p style="color: grey;"><i>Updated 2010-10-18 12:20:20</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">so, this is the activity session code. it's not 100% complete, \
because kwin has to fake a few things until the activities service gets its new api - \
but ivan's working on that. :) there won't be any UI for it until that \
api's in anyways.
kwin handles the mapping from activity id -> windows -> session ids, because I \
had to call it anyways, and it's much easier to calculate from inside kwin.
The good:
it can save and restore processes and their windows' activity associations
all the regular xsmp stuff seems to be working, except cancel.
I've done it by merely adding a few dbus methods, no changes to xsmp itself
ksmserver doesn't know or care what activities are; even the kwin code is \
separated out into "find the clients for this activity" and "save a \
session for these clients"
The bad:
since ksmserver doesn't tell the wm or clients that anything special is \
happening, it can't tell kwin "save the subsession data", or tell \
clients "close only window foo". for kwin this is actually a nice excuse to \
have it do the mapping work, but for clients it means we can't close anything \
until we're ready to close everything. Clients would have to be made \
context-aware either way to react to such a request; for 4.7 I might see if I can do \
it by extending xsmp or augmenting it with more dbus (instead of having the client do \
all the work after the session's closed, as I originally planned).
kwrite's cancel button doesn't cancel the session-close. it works on logout, \
so there must be a bug in my code.
session data isn't deleted when an activity is deleted (I forgot). I'd like \
to get this patch in ASAP though, and add simple details like that later - the patch \
is already quite big enough ;)
there's some "legacy" session code, I haven't read it all and I \
haven't implemented support for it yet. are there any apps using it? does it \
matter? what *is* this legacy session thing?
The ugly:
there are some sync issues in here.
first of all, we have a conflict between save-on-close and save-regularly. all xsmp \
stuff is save-on-close, whereas the activity service and plasma (and other modern \
apps) save quite often. If you open an activity and then X crashes, you've lost \
all session data for that activity. :(
I'm planning to try fixing that by saving (but not quitting) the login session \
when an activity is opened/closed. I'm not sure how well it'd work, but it \
should at least keep the correct list of processes to start.
the second sync issue comes from the fact that a window can be open on a closed \
activity. I'll probably prevent the user from adding a window to a closed \
activity, but windows that are already shared across activities... well, they could \
behave in unexpected ways. I think I'd like to do more tests before deciding how \
to handle that - but one thing that could help, perhaps, is storing the kwin session \
info in one big pool for all closed activities instead of a separate group for each \
activity.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">I can use dbus to save/restore an activity, like I showed in my \
screencast ( http://chani.wordpress.com/2010/10/04/activities-fun-with-screencast ). \
I haven't done much testing of multi-window processes, though, or saving and \
restoring multiple activities.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/KDE/kdebase/workspace/ksmserver/server.h <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/ksmserver/legacy.cpp <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/ksmserver/org.kde.KSMServerInterface.xml <span \
style="color: grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/ksmserver/server.cpp <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/ksmserver/shutdown.cpp <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/ksmserver/startup.cpp <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/kwin/org.kde.KWin.xml <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/kwin/sm.cpp <span style="color: \
grey">(1186160)</span></li>
<li>trunk/KDE/kdebase/workspace/kwin/workspace.h <span style="color: \
grey">(1186160)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5638/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic