[sugar] #1560 NORM Trial-2: Activity launch not detected

[Dan Williams]

On Mon, 2007-06-04 at 21:03 +0200, Bert Freudenberg wrote:
> On Jun 4, 2007, at 18:51 , Marco Pesenti Gritti wrote:
> 
> > Bert Freudenberg wrote:
> >> On Jun 4, 2007, at 16:41 , Marco Pesenti Gritti wrote:
> >>
> >>> Bert Freudenberg wrote:
> >>>> On Jun 3, 2007, at 18:01 , Zarro Boogs per Child wrote:
> >>>>
> >>>>> #1560: Activity launch not detected
> >>>>> --------------------- 
> >>>>> +------------------------------------------------------
> >>>>>   Reporter:  bert    |       Owner:  dcbw
> >>>>>       Type:  defect  |      Status:  reopened
> >>>>>   Priority:  normal  |   Milestone:  Trial-2
> >>>>>  Component:  sugar   |     Version:
> >>>>> Resolution:          |    Keywords:
> >>>>>   Verified:  0       |
> >>>>> --------------------- 
> >>>>> +------------------------------------------------------
> >>>>> Comment (by marco):
> >>>>>
> >>>>>  This should be fixed again in the latest git. Activity id and  
> >>>>> bundle id
> >>>>>  should now be provided by two properties of the toplevel X  
> >>>>> window.
> >>>>>
> >>>>>  _SUGAR_ACTIVITY_ID (utf8 string)
> >>>>>  _SUGAR_BUNDLE_ID (utf8 string)
> >>>>>
> >>>>>  sugar/lib/sugar-x11-util.c has the code which we use to set those
> >>>>>  properties for python activities.
> >>>>
> >>>> This only complicates matters. Not only for Etoys (which does  
> >>>> not have a way to set X properties currently), but I see you had  
> >>>> to resort to C code even for Python activities. I'd much prefer  
> >>>> if the only thing Sugar cares about an activity X-wise is its  
> >>>> window ID (and we're working on a way to make that accessible in  
> >>>> Squeak). Everything else should be handled by DBus, and this is  
> >>>> how it actually was before your refactoring. If anything I'd  
> >>>> shift communication *towards" DBus, not away from it towards X  
> >>>> by introducing arbitrary and unnecessary properties.
> >>>
> >>> [ CCing Dan, since he wrote the original startup notification  
> >>> and  I'm interested in his opinion ]
> >>>
> >>> We decided a while ago to use X  for window management and I  
> >>> don't see a reason to revisit that decision. Etoys should respect  
> >>> the multiple windows semantic (which I know you are working on,  
> >>> thanks).
> >>>
> >>> To reassure you, we are not going to use X as an IPC (as people  
> >>> have done in the past), since that gets unbelievably ugly: that's  
> >>> where dbus has his role in the shell -> activity instance  
> >>> communication.
> >>
> >> That's good to hear. However, the X properties required now are  
> >> more complex than before. In the case of Squeak, at window  
> >> creation time I have no way of knowing the dbus activity id yet.
> >>
> >>>> Besides, it does not address the original problem - that if an  
> >>>> activity X window is opened and detected by the Sugar shell, but  
> >>>> it has not created its DBus service yet, the shell will mistake  
> >>>> it for a raw X app (creating a
> >>>> "raw" icon in the donut next to the still flashing activity icon).
> >>>
> >>> If there is a _SUGAR_ACTIVITY_ID property the window is an  
> >>> activity, otherwise it's a raw X window. So I think it actually  
> >>> solves the problem.
> >>>
> >>> The shell must know the activity and the bundle id as soon as an  
> >>> activity window is displayed, otherwise it can't represent it  
> >>> properly in the home. Relying on the dbus service for this was  
> >>> very fragile and introduced races which was hard or impossible to  
> >>> solve properly.
> >>
> >> To show the icon it's sufficient to know the bundle id. Actually,  
> >> you don't even really need that, because the icon is already  
> >> showing (flashing). You just need to know if this is an activity  
> >> window or not. And if it is, then sooner or later the dbus service  
> >> will appear telling you if that window belongs to this activity.
> >>
> >
> > You are probably misunderstanding how startup notification works.
> >
> > 1 Before an activity is started from the panel it informs the shell  
> > that an activity with a certain $ID is going to startup -> The  
> > shell display a pulsing icon.
> > 2 When the window finally appear, sugar needs to get its activity  
> > id so that it can transform the notification icon in a normal one  
> > (stop the pulsing, map it to the window xid etc).
> 
> I understand that. And this is perfectly reasonable. However, you  
> *could* do step 2 also in response to a dbus message. In fact, a few  
> days ago the Sugar code handled both cases nicely. Now you replaced  
> that mechanism by an admittedly better one, but at the same time you  
> have removed all fallbacks - and that's what I'm arguing about.
> 
> >> It's practically the same whether you announce the dbus service  
> >> via X props, or you send the XID via dbus. Supporting both makes  
> >> activity developer's live easier.
> >
> > Using X props the operation is atomic. The window appears *and* you  
> > know his activity_id at the same time. And that obviously avoid the  
> > races.
> 
> Unless you have an activity which cannot set the window properties  
> before opening the window.
> 
> >>> Window management should work using X communication. Mixing dbus  
> >>> and X there just introduce races and confusion. If we was going  
> >>> down the "dbus for window management" route then I'd agree with  
> >>> you, but we aren't.
> >>
> >>>> This can easily be fixed up later by detecting the DBus service  
> >>>> creation and removing the raw icon - this is how my fix worked.
> >>>
> >>> Now, that's really a bad hack. I don't want to show a random icon  
> >>> on the screen and than replace it when the DBUS service appear...  
> >>> it just suck.
> >>
> >> That icon would not even be visible because it is hidden by the X  
> >> window that just appeared. It is noble to try preventing to create  
> >> that icon unnecessarily, but if I had a say in designing Sugar,  
> >> I'd opt for robustness, making it simpler for external developers,  
> >> not harder.
> >>
> >
> > Robustness is exactly the reason I chose to use the X properties.  
> > It makes it slightly harder to setup a non-python Sugar activity  
> > but it's a much cleaner and robust approach.
> 
> Agreed, but it would help activity developers like me if you  
> supported alternate strategies.
> 
> >> In fact, if I launch an activity from the Sugar frame, it's  
> >> reasonable to assume that the next X window opened actually  
> >> belongs to that activity. So you don't really need to show that  
> >> raw icon, at least until the activity launch is finished.
> >
> > Huh. You are really not opting for robustness here!
> 
> You misunderstood. This would only be the backup-strategy. What I  
> suggest is allowing multiple ways to detect activities. There is the  
> one preferred way of X window properties, but there could be a DBus- 
> based one, too.

Having multiple APIs for the same thing isn't a good solution.  Neither
is having some random icon show up in the donut when the user clearly
clicked on something that is _not_ that random icon.  In our machine,
that random icon is going to sit there pulsing for an indeterminate
amount of time while the activity launches because the hardware isn't
very speedy, even B3s.  Even if the activity's window comes up and
covers the donut, you can still switch around between the home screen
and any of the other views while the thing launches.

How about adding --sugar-activity-id and --sugar-bundle-id arguments to
the Squeak VM command line, or check the SUGAR_ACTIVITY_ID and
SUGAR_BUNDLE_ID environment variables?  Squeak uses those to set the X
properties when it first starts up.  Subsequent activites, if they use
the same VM, can obviously know what their activity ID and bundle ID are
before they show the window, because the D-Bus aware code will have been
loaded at that point.  It would be fairly simple to help out activity
authors who can't start D-Bus immediately to pass those arguments to the
activity or put those variables into the activity environment for the
activity to use.

Another solution would be to change sugar/matchbox to not even raise
toplevel windows until they advertise the correct activity ID and bundle
ID.  That's also ugly, because that means that some window management
gets moved into sugar, which is what we are trying to avoid here anyway.

Dan