[Sugar-devel] [PATCH sugar] Friendstray: make sure the tray is right on the sharer side OLPC #10817

[Simon Schampijer]

On 07/04/2011 05:08 PM, Simon Schampijer wrote:
> On 07/02/2011 06:50 PM, Sascha Silbe wrote:
>> Excerpts from Simon Schampijer's message of Fri Jun 24 11:57:56 +0200
>> 2011:
>>> The code path that listens for the 'activity-added' signal is used to
>>> track the following use case: 'A' starts an activity
>>> (he gets the 'active-activity-changed' signal from the shell), he then
>>> at a later point shares the activity. In order to track joining/leaving
>>> buddies he needs to get the shared activity. On other machines if we
>>> receive the 'activity-added' signal we can ignore it, therefore the
>>> check if the current active activity is the shared one.
>>
>> I understood neither the description nor the patch at first. Let's go
>> into detail a bit:
>>
>>
>> Sequence of events:
>>
>> A B
>> ---------------------------------------------------------------------------
>>
>> starts activity
>> shell:active-activity-changed fires
>> shares activity
>> neighborhood:activity-added fires? neighborhood:activity-added fires
>> joins activity
>> shell:active-activity-changed fires
>>
>>
>> According to the HIG, the People side of the Frame shows the individuals
>> that the user is collaborating with in the currently active
>> activity. [1]
>
> Correct.
>
>> On the "sharer" side shell:active-activity-changed will fire before a
>> shared activity object exists. So we need to wait for the corresponding
>> neighborhood:activity-added signal in order to access the shared
>> activity object and connect to its buddy-added and buddy-received
>> signals.
>>
>> On the "joiner" side the shared activity object is already available
>> when shell:active-activity-changed fires, so we can connect to the
>> signals right away. neighborhood:activity-added doesn't need to do
>> anything in this case.
>>
>> When switching between different activities, the shared activity object
>> also already exists (if the activity is shared at all), so again
>> neighborhood:activity-added should be a no-op.
>
> Yes, this all is true.
>
>>> +++ b/src/jarabe/frame/friendstray.py
>>> @@ -75,6 +75,10 @@ class FriendsTray(VTray):
>>> def __neighborhood_activity_added_cb(self, neighborhood_model,
>>> shared_activity):
>>> logging.debug('FriendsTray.__neighborhood_activity_added_cb')
>>> + active_activity = shell.get_model().get_active_activity()
>>> + if active_activity.get_activity_id() != shared_activity.activity_id:
>>> + return
>>> +
>>
>> With the above understanding, shouldn't we reset self._shared_activity
>> to None in __active_activity_changed_cb() and add the following check
>> before yours?
>>
>> if self._shared_activity is not None:
>> return
>
> So my check prevents on any other machine then the sharer machine the
> neighborhood:activity-added to have any effect. All the rest should work
> fine. AFAIK your addition is not needed, from testing and looking at the
> code. But maybe I do oversee something, can you explain which exact case
> it should fix?
>
> Regards,
> Simon

Can I land this? This is in the latest OLPC image and tested.

Regards,
    Simon