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

[Marco Pesenti Gritti]

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).

> 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.

>> 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.

> 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!

>> The startup phase is completed when the window appear on the screen, 
>> not when the dbus service appear. That's also exactly how startup 
>> notification is implemented in GNOME btw.
>
> Which isn't really relevant to Sugar, is it?

It's relevant that the Sugar approach to startup notification has been 
already used successfully in another based on  the same platform.

Marco