[sugar] Ticket #1289, patch review

[Marco Pesenti Gritti]

First of all this is really useful, thanks!

+        # XXX this creates an uncollectable object, should be handled
+        # using a weakref callback...
         gobject.source_remove(self._launch_timeout_id)
         self._launch_timeout_id = 0

I don't understand this. To which object are you referring?

     def _launch_timeout_cb(self, user_data=None):
+        """Callback for launch operation timeouts
+        
+        XXX who registers this callback?
+        XXX who sends the message?
+        """

See

self._launch_timeout_id = gobject.timeout_add(20000,
self._launch_timeout_cb)

This code should go away at some point I think, we should just use the
dbus timeout.

+        """Retrieve the "activity_id" passed in to our constructor
+        
+        XXX What is the activity id, really?

Each activity instance has a unique id, so that it can be resumed and
shared on the network.

     def get_window(self):
+        """Retrieve the X-windows root window of this application
+        
+        This was stored by the set_window method.
+        
+        XXX What called that?
+        """

Activities are added to HomeModel just after starting them, before the
window actually appears, so that we can do startup notification. So
HomeModel listen for new windows creation and when the one associated
with an HomeActivity appears, it set_window on it.

+    Note: there seem to be logic issues in this function
+    not sure what it's trying to do with the iterations,
+    but it seems that it is enforcing that we must be
+    able to create 10 unique IDs in a row?  That seems 
+    kind of strange.  It's also using a weird while loop
+    instead of a simple for loop and breaks a for loop 
+    just to do a check and raise on the flag set... looks
+    like C code written in Python...  I believe the intent
+    is to try 10 times to create a unique ID and only fail 
+    if all 10 attempts fail...

Haven't seen the function but if you want to refactor it and post a
patch that would be useful. Dan might have more insights on why it does
what it does.

+    This object uses a dbus method on the ActivityFactory
+    service to create the new activity.  It generates 
+    GTK events in response to the success/failure of
+    activity startup using callbacks to the service's 
+    create call.

s/GTK events/GObject events

+        XXX where is the ActivityFactoryService specific to this
+        instance created?  That is, what's launching the 
+        activity-factory scripts?  Is it the 
+        com.redhat.Sugar.ActivityFactory service?  If so, where
+        is that defined/registered/created?

DBus activation. When you do a call on a dbus service and the service is
not active DBus automatically launch the process. We generate the
service information DBus uses to do this dinamically in
sugar/activity/bundleregistry.py.

+        object_id -- XXX what is this?

The id of the journal object associated with the activity. It was used
by the journal prototype implementation, might change when we do the
real one. When you resume an activity from the journal the object_id
will be passed in. It's optional since new activities does not have an
associated object (yet).

I'm not clear on how this relate to the activity id yet, i.e. I'm not
sure if we really need both. TBF

+        uri -- XXX what is this?

A URI associated with the activity. Used when opening an external file
or resource in the activity, rather than a journal object (downloads
stored on the file system for example or web pages).

+"""Metadata description of a given application/activity
+
+The bundle's metadata is normally read from an activity.info 
+file in order to describe a given bundle (directory).
+"""

The spec should be referenced there:
http://wiki.laptop.org/go/Activity_bundles

+        At the moment this requires that something somewhere
+        have registered a dbus service with the XID of the 
+        new window that is used to bind the requested activity 
+        to the window.

In the future the dbus service will not be strictly required. I want to
be able to launch simple windows in the shell for debugging purposes.
Still the service(s) will be required by proper activities.
It would be probably possible to increase compatibility further but
that's not planned at the moment.

Do you have write access to the sugar git module? Otherwise can you send
mail to Ivan about it and cc me?

Thanks!
Marco