Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Tue, Dec 07, 2010 at 12:39:19PM +, Michael Meeks wrote: > Hi David, > > On Tue, 2010-12-07 at 10:54 +0100, David Tardon wrote: > > > I think the simplest thing here is to run SessionManagerClient::open > > > when initializing the quickstarter (the function is exported, so there > > > should be no problem with that), but there might be a cleaner solution. > > > > What if we do this when SalSession is created? Does anyone see any > > problem with that approach ? > > There are some potential risks here; we don't want to do this if we are > just going to push our arguments off to another process or before we > start the splash (that is prolly the legacy explanation for deferring it > - since it is a set of synchronous calls AFAICS). > > Luckily the new standalone unix splash / quick-starter handles both of > these before we even start, and deprecates those concerns, so I suspect > there is no issue here. Having said that - this is another miracle of > UNO-isation - with the SessionManagerClient stuff being instantiated in > framework/ via a service, which itself is (presuambly) instantiated via > another service etc. etc. - making the code not only slower, but far > more difficult to follow :-) Yeah, that one is created as com.sun.star.frame.SessionListener service in desktop::Desktop::OpenClients :-) > > So - anyhow, I'd say whack the patch in, and hopefully it will solve > your abrt issues in an elegant way :-) > Okay, pushing. D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
Hi David, On Tue, 2010-12-07 at 10:54 +0100, David Tardon wrote: > > I think the simplest thing here is to run SessionManagerClient::open > > when initializing the quickstarter (the function is exported, so there > > should be no problem with that), but there might be a cleaner solution. > > What if we do this when SalSession is created? Does anyone see any > problem with that approach ? There are some potential risks here; we don't want to do this if we are just going to push our arguments off to another process or before we start the splash (that is prolly the legacy explanation for deferring it - since it is a set of synchronous calls AFAICS). Luckily the new standalone unix splash / quick-starter handles both of these before we even start, and deprecates those concerns, so I suspect there is no issue here. Having said that - this is another miracle of UNO-isation - with the SessionManagerClient stuff being instantiated in framework/ via a service, which itself is (presuambly) instantiated via another service etc. etc. - making the code not only slower, but far more difficult to follow :-) So - anyhow, I'd say whack the patch in, and hopefully it will solve your abrt issues in an elegant way :-) Thanks ! Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
> The problem is that SessionManagerClient is only initialized with the
> first sal frame (IOW, GtkSalFrame::Show calls
> SessionManagerClient::open). Obviously, if there has never been any
> frame opened, which, I suppose, happens in a lot of cases, this hadn't
> happened. So the app is not aware that the desktop session is ending and
> continues to run, till it's killed by an XIOError.
>
> I think the simplest thing here is to run SessionManagerClient::open
> when initializing the quickstarter (the function is exported, so there
> should be no problem with that), but there might be a cleaner solution.
>
What if we do this when SalSession is created? Does anyone see any
problem with that approach?
D.
commit d1227b3d8b43ced1ef834bb00aec26a03d76c648
Author: David Tardon
Date: Tue Dec 7 10:51:36 2010 +0100
initialize session client when SalSession is created
This avoids crash on end of desktop session if quickstarter is running
and there has not been any window opened.
diff --git a/vcl/unx/gtk/window/gtkframe.cxx b/vcl/unx/gtk/window/gtkframe.cxx
index 39f5f87..9ed4385 100644
--- a/vcl/unx/gtk/window/gtkframe.cxx
+++ b/vcl/unx/gtk/window/gtkframe.cxx
@@ -1282,7 +1282,6 @@ void GtkSalFrame::Show( BOOL bVisible, BOOL bNoActivate )
gtk_window_set_keep_above( GTK_WINDOW(m_pWindow), bVisible );
if( bVisible )
{
-SessionManagerClient::open(); // will simply return after the
first time
initClientId();
getDisplay()->startupNotificationCompleted();
diff --git a/vcl/unx/source/app/sm.cxx b/vcl/unx/source/app/sm.cxx
index 2b26694..5125ed2 100644
--- a/vcl/unx/source/app/sm.cxx
+++ b/vcl/unx/source/app/sm.cxx
@@ -82,6 +82,7 @@ SalSession* X11SalInstance::CreateSalSession()
{
if( ! pOneInstance )
pOneInstance = new IceSalSession();
+SessionManagerClient::open();
return pOneInstance;
}
diff --git a/vcl/unx/source/window/salframe.cxx
b/vcl/unx/source/window/salframe.cxx
index 4310f2d..839bd03 100644
--- a/vcl/unx/source/window/salframe.cxx
+++ b/vcl/unx/source/window/salframe.cxx
@@ -1152,8 +1152,6 @@ void X11SalFrame::Show( BOOL bVisible, BOOL bNoActivate )
setXEmbedInfo();
if( bVisible )
{
-SessionManagerClient::open(); // will simply return after the first
time
-
mbInShow = TRUE;
if( ! (nStyle_ & SAL_FRAME_STYLE_INTRO) )
{
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Tue, 2010-12-07 at 09:53 +0100, David Tardon wrote: > Bah, the header where it's declared is not public :( Make it public then :-) C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Tue, Dec 07, 2010 at 09:34:52AM +0100, David Tardon wrote: > The problem is that SessionManagerClient is only initialized with the > first sal frame (IOW, GtkSalFrame::Show calls > SessionManagerClient::open). Obviously, if there has never been any > frame opened, which, I suppose, happens in a lot of cases, this hadn't > happened. So the app is not aware that the desktop session is ending and > continues to run, till it's killed by an XIOError. > > I think the simplest thing here is to run SessionManagerClient::open > when initializing the quickstarter (the function is exported, so there > should be no problem with that), but there might be a cleaner solution. > Bah, the header where it's declared is not public :( D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
Hi, Michael, On Mon, Dec 06, 2010 at 03:59:17PM +, Michael Meeks wrote: > Hi David, > > On Mon, 2010-12-06 at 15:18 +0100, David Tardon wrote: > > On Mon, Dec 06, 2010 at 02:08:36PM +, Michael Meeks wrote: > > > Really ? as soon as the X server dies, we'll get all manner of XErrors > > > that will kill us nastily; I don't see a feature there, but perhaps I'm > > > missing it. > > > > And that dying on XIOError is something we (i.e., Caolan and me) wanted > > to avoid. > > Sure - but I don't believe you'll do that here; And you're absolutely right--I was so focused on the quickstarter I didn't see the forest for the trees... The real problem is elsewhere (see below). > as you destroy the > windows, you'll do a lot of X calls to free server resources that will > just fail (surely). > > vcl/unx/source/app/saldata.cxx-int X11SalData::XIOErrorHdl( Display * ) > > does (essentially) _exit(0); which is about all you can do at that > stage if you don't want to crash. > > > Because otherwise abrt (Fedora's crash catching tool) > > dutifully saves it and lets the user report it as a crash and we are > > flooded (again) by reports like > > https://bugzilla.redhat.com/show_bug.cgi?id=650170 . > > Hmm, obviously not good. I couldn't see a stack trace there that I > could read. I have attached one. > However - I don't see why the systray should be any > different to having a top-level window open, or indeed any normal gtk+ > app getting getting nailed by a zapped X server. [ do you really file > 10x bugs for different apps if someone does ctrl-alt-backspace to zap > the server ? ]. > > > I recognize this is not of general interest, though. > > Nah - we should fix it; but - ... getting a good trace for it would > really help. Is it from the g_error in the gtk+ X error handler ? do we > really get a good grace period to cleanup from the session manager ? > [ and just don't cleanup the systray applet there ? ] or ... > The problem is that SessionManagerClient is only initialized with the first sal frame (IOW, GtkSalFrame::Show calls SessionManagerClient::open). Obviously, if there has never been any frame opened, which, I suppose, happens in a lot of cases, this hadn't happened. So the app is not aware that the desktop session is ending and continues to run, till it's killed by an XIOError. I think the simplest thing here is to run SessionManagerClient::open when initializing the quickstarter (the function is exported, so there should be no problem with that), but there might be a cleaner solution. D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Mon, Dec 6, 2010 at 10:12 AM, Christopher Backhouse wrote: ... > > Chris > > PS - is there a git commit notification list I can subscribe to? The git web > interface is a royal pain to navigate. http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits > > ___ > LibreOffice mailing list > LibreOffice@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/libreoffice > ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On 06/12/2010 15:47, Michael Meeks wrote: Yes - it's a pain; hence doing it for you. I was thinking of more exciting fun / new things :-) Thanks. We rather need someone clueful to tackle the problem of the flat ODF / XML export - if you're interested ? http://wiki.documentfoundation.org/Development/Easy_Hacks#De-Java-ise_flat_XML_export Well, I would describe myself as a good C++ programmer, and I'm sure Java is easy enough to read. I don't really know anything about XSLT or XML in general though. Still, excuses to get rid of Java are always nice. Or are you interested in a particular area / component we can help you get stuck into ? :-) I'm reasonably handy with gtk, though via the python bindings which hide all the memory management, and a lot of the type conversion from you, so dunno how well I'll get on in C. The XML export thing sounds like a good idea to at least look into. Otherwise, maybe some more gtk/gnome integration stuff. Sadly all the bugs that really wind me up are most likely deep-seated and beyond my ability to fix (focus issues, not-quite-native-appearance, import bugs especially from powerpoint). Be warned: I'm a student, doing this for fun in the evenings, so progress might be slow. Though I was amazed to find a (semi)useful task that only took about an hour to understand and do. Chris PS - is there a git commit notification list I can subscribe to? The git web interface is a royal pain to navigate. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
Hi David, On Mon, 2010-12-06 at 15:18 +0100, David Tardon wrote: > On Mon, Dec 06, 2010 at 02:08:36PM +, Michael Meeks wrote: > > Really ? as soon as the X server dies, we'll get all manner of XErrors > > that will kill us nastily; I don't see a feature there, but perhaps I'm > > missing it. > > And that dying on XIOError is something we (i.e., Caolan and me) wanted > to avoid. Sure - but I don't believe you'll do that here; as you destroy the windows, you'll do a lot of X calls to free server resources that will just fail (surely). vcl/unx/source/app/saldata.cxx-int X11SalData::XIOErrorHdl( Display * ) does (essentially) _exit(0); which is about all you can do at that stage if you don't want to crash. > Because otherwise abrt (Fedora's crash catching tool) > dutifully saves it and lets the user report it as a crash and we are > flooded (again) by reports like > https://bugzilla.redhat.com/show_bug.cgi?id=650170 . Hmm, obviously not good. I couldn't see a stack trace there that I could read. However - I don't see why the systray should be any different to having a top-level window open, or indeed any normal gtk+ app getting getting nailed by a zapped X server. [ do you really file 10x bugs for different apps if someone does ctrl-alt-backspace to zap the server ? ]. > I recognize this is not of general interest, though. Nah - we should fix it; but - ... getting a good trace for it would really help. Is it from the g_error in the gtk+ X error handler ? do we really get a good grace period to cleanup from the session manager ? [ and just don't cleanup the systray applet there ? ] or ... Thanks, Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Mon, 2010-12-06 at 15:01 +, Christopher Backhouse wrote: > I have locally a check against the gtk version (the highest version Heh - sorry pardon; I just added that to configure - we disable the gtk + systray where we know it won't work there; and that also fixes the scp2 issues you fell into ;-) > I don't really understand the build system. I grepped for and removed > every mention of libegg I could find, and now I have a broken build... > "make dev-install" is still trying to copy it, and failing, This is even > after I reran autogen and did a make clean. I must have missed an > occurrence somewhere. Yes - it's a pain; hence doing it for you. I was thinking of more exciting fun / new things :-) We rather need someone clueful to tackle the problem of the flat ODF / XML export - if you're interested ? http://wiki.documentfoundation.org/Development/Easy_Hacks#De-Java-ise_flat_XML_export Or are you interested in a particular area / component we can help you get stuck into ? :-) Thanks ! Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg (Spam Message)
On Mon, 2010-12-06 at 15:24 +0100, Sebastian Spaeth wrote: > Perhaps removing the bundled libegg project and disable copying it in > our install package? :-) Already done :-) Regards, Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On 06/12/2010 14:08, Michael Meeks wrote: I gave it a reasonble test, of all the combinations I could think of [ though not session exit ], and I've pushed it. Cool, thanks. On 06/12/2010 14:08, Michael Meeks wrote: I guess, since SLED10 ships with gtk+ 2.8.11 or something crazy we should prolly dynamically detect and not enable the systray quickstarter for that version: fine with me. I have locally a check against the gtk version (the highest version needed by any feature I use (the tooltip and so on) is I think 2.18, but this could be done more fine grained). The option is still in the user interface, it just bails out of constructing the tray icon, with no feedback, which isn't great user experience, but probably better than undefined symbol errors. I don't have any means to test with an old gtk to see if this actually works though. On 06/12/2010 14:24, Sebastian Spaeth wrote: Perhaps removing the bundled libegg project and disable copying it in our install package? :-) I don't really understand the build system. I grepped for and removed every mention of libegg I could find, and now I have a broken build... "make dev-install" is still trying to copy it, and failing, This is even after I reran autogen and did a make clean. I must have missed an occurrence somewhere. Chris ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Mon, 06 Dec 2010 14:08:36 +, Michael Meeks wrote: > Thanks so much for this Christopher ! what is your next planned > feat ? :-) Perhaps removing the bundled libegg project and disable copying it in our install package? :-) Sebastian pgpfAQ227kXmt.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Mon, Dec 06, 2010 at 02:08:36PM +, Michael Meeks wrote: > > On Mon, 2010-12-06 at 08:30 +0100, David Tardon wrote: > > The current impl. shutdowns LibO when desktop session is ended. Unless > > I'm missing something obvious, the new impl. cannot do that... > > Really ? as soon as the X server dies, we'll get all manner of XErrors > that will kill us nastily; I don't see a feature there, but perhaps I'm > missing it. And that dying on XIOError is something we (i.e., Caolan and me) wanted to avoid. Because otherwise abrt (Fedora's crash catching tool) dutifully saves it and lets the user report it as a crash and we are flooded (again) by reports like https://bugzilla.redhat.com/show_bug.cgi?id=650170 . I recognize this is not of general interest, though. D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Mon, 2010-12-06 at 08:30 +0100, David Tardon wrote: > The current impl. shutdowns LibO when desktop session is ended. Unless > I'm missing something obvious, the new impl. cannot do that... Really ? as soon as the X server dies, we'll get all manner of XErrors that will kill us nastily; I don't see a feature there, but perhaps I'm missing it. I gave it a reasonble test, of all the combinations I could think of [ though not session exit ], and I've pushed it. I suppose we need to consider whether we can depend on gtk+ 2.10 - http://mail.gnome.org/archives/gtk-list/2006-July/msg00013.html from July 2006 - in order to get gtk+ support. I guess, since SLED10 ships with gtk+ 2.8.11 or something crazy we should prolly dynamically detect and not enable the systray quickstarter for that version: fine with me. Thanks so much for this Christopher ! what is your next planned feat ? :-) Thanks, Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Use GtkStatusIcon instead of libegg
On Sun, Dec 05, 2010 at 02:09:05PM +, Christopher Backhouse wrote: > Hi, > > This is item 3.17 in the easy hacks list. > Seems to work fine here. I can turn the quickstarter on and off and > launch things from it. > > The code is simplified a little from the libegg version (partially > because I removed a few what now seem to be be unnecessary hacks). > Hints on removing libegg from the tree entirely are welcome. I'm not > great with build stuff. > The current impl. shutdowns LibO when desktop session is ended. Unless I'm missing something obvious, the new impl. cannot do that... D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
