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

[Bert Freudenberg]

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.

- Bert -