resume journal entry race may duplicate resumed activity id #1719

5 files changed, 96 insertions, 47 deletions

diff --git a/src/jarabe/frame/activitiestray.py b/src/jarabe/frame/activitiestray.py
index b5762ee..3a80ac0 100644
--- a/src/jarabe/frame/activitiestray.py
+++ b/src/jarabe/frame/activitiestray.py
@@ -59,6 +59,7 @@ class ActivityButton(RadioToolButton):
         self.set_palette_invoker(FrameWidgetInvoker(self))
 
         self._home_activity = home_activity
+        self._notify_launch_hid = None
 
         self._icon = PulsingIcon()
         self._icon.props.base_color = home_activity.get_icon_color()
@@ -72,13 +73,12 @@ class ActivityButton(RadioToolButton):
         self.set_icon_widget(self._icon)
         self._icon.show()
 
-        if home_activity.props.launching:
+        if home_activity.props.launch_status == shell.Activity.LAUNCHING:
             self._icon.props.pulsing = True
-            self._notify_launching_hid = home_activity.connect( \
-                    'notify::launching', self.__notify_launching_cb)
-        else:
-            self._notify_launching_hid = None
-            self._notif_icon = None
+            self._notify_launch_hid = home_activity.connect( \
+                    'notify::launch-status', self.__notify_launch_status_cb)
+        elif home_activity.props.launch_status == shell.Activity.LAUNCH_FAILED:
+            self._on_failed_launch()
 
     def create_palette(self):
         if self._home_activity.is_journal():
@@ -88,10 +88,18 @@ class ActivityButton(RadioToolButton):
         palette.set_group_id('frame')
         self.set_palette(palette)
 
-    def __notify_launching_cb(self, home_activity, pspec):
-        if not home_activity.props.launching:
+    def _on_failed_launch(self):
+        # TODO http://bugs.sugarlabs.org/ticket/2007
+        pass
+
+    def __notify_launch_status_cb(self, home_activity, pspec):
+        home_activity.disconnect(self._notify_launch_hid)
+        self._notify_launch_hid = None
+        if home_activity.props.launch_status == shell.Activity.LAUNCH_FAILED:
+            self._on_failed_launch()
+        else:
             self._icon.props.pulsing = False
-            home_activity.disconnect(self._notify_launching_hid)
+
 
 class BaseInviteButton(ToolButton):
     def __init__(self, invite):
diff --git a/src/jarabe/model/shell.py b/src/jarabe/model/shell.py
index e03e0f7..553e889 100644
--- a/src/jarabe/model/shell.py
+++ b/src/jarabe/model/shell.py
@@ -35,6 +35,7 @@ _SERVICE_NAME = "org.laptop.Activity"
 _SERVICE_PATH = "/org/laptop/Activity"
 _SERVICE_INTERFACE = "org.laptop.Activity"
 
+
 class Activity(gobject.GObject):
     """Activity which appears in the "Home View" of the Sugar shell
 
@@ -46,10 +47,9 @@ class Activity(gobject.GObject):
 
     __gtype_name__ = 'SugarHomeActivity'
 
-    __gproperties__ = {
-        'launching' : (bool, None, None, False,
-                       gobject.PARAM_READWRITE),
-    }
+    LAUNCHING = 0
+    LAUNCH_FAILED = 1
+    LAUNCHED = 2
 
     def __init__(self, activity_info, activity_id, window=None):
         """Initialise the HomeActivity
@@ -69,7 +69,7 @@ class Activity(gobject.GObject):
         self._activity_id = activity_id
         self._activity_info = activity_info
         self._launch_time = time.time()
-        self._launching = True
+        self._launch_status = Activity.LAUNCHING
 
         if window is not None:
             self.set_window(window)
@@ -84,6 +84,16 @@ class Activity(gobject.GObject):
                     signal_name="NameOwnerChanged",
                     dbus_interface="org.freedesktop.DBus")
 
+        self._launch_completed_hid = get_model().connect('launch-completed',
+                self.__launch_completed_cb)
+        self._launch_failed_hid = get_model().connect('launch-failed',
+                self.__launch_failed_cb)
+
+    def get_launch_status(self):
+        return self._launch_status
+
+    launch_status = gobject.property(getter=get_launch_status)
+
     def set_window(self, window):
         """Set the window for the activity
 
@@ -224,14 +234,6 @@ class Activity(gobject.GObject):
             return self._window.get_xid() == activity.get_xid()
         return False
 
-    def do_set_property(self, pspec, value):
-        if pspec.name == 'launching':
-            self._launching = value
-
-    def do_get_property(self, pspec):
-        if pspec.name == 'launching':
-            return self._launching
-
     def _get_service_name(self):
         if self._activity_id:
             return _SERVICE_NAME + self._activity_id
@@ -270,6 +272,23 @@ class Activity(gobject.GObject):
     def _set_active_error(self, err):
         logging.error("set_active() failed: %s", err)
 
+    def _set_launch_status(self, value):
+        get_model().disconnect(self._launch_completed_hid)
+        get_model().disconnect(self._launch_failed_hid)
+        self._launch_completed_hid = None
+        self._launch_failed_hid = None
+        self._launch_status = value
+        self.notify('launch_status')
+
+    def __launch_completed_cb(self, model, home_activity):
+        if home_activity is self:
+            self._set_launch_status(Activity.LAUNCHED)
+
+    def __launch_failed_cb(self, model, home_activity):
+        if home_activity is self:
+            self._set_launch_status(Activity.LAUNCH_FAILED)
+
+
 class ShellModel(gobject.GObject):
     """Model of the shell (activity management)
 
@@ -332,9 +351,20 @@ class ShellModel(gobject.GObject):
         self._active_activity = None
         self._tabbing_activity = None
         self._pservice = presenceservice.get_instance()
+        self._launchers = {}
 
         self._screen.toggle_showing_desktop(True)
 
+    def get_launcher(self, activity_id):
+        return self._launchers.get(str(activity_id))
+
+    def register_launcher(self, activity_id, launcher):
+        self._launchers[activity_id] = launcher
+
+    def unregister_launcher(self, activity_id):
+        if activity_id in self._launchers:
+            del self._launchers[activity_id]
+
     def _update_zoom_level(self, window):
         if window.get_window_type() == wnck.WINDOW_DIALOG:
             return
@@ -482,9 +512,7 @@ class ShellModel(gobject.GObject):
                 home_activity.set_window(window)
 
             if wm.get_sugar_window_type(window) != 'launcher':
-                home_activity.props.launching = False
-                if not home_activity.is_journal():
-                    self.emit('launch-completed', home_activity)
+                self.emit('launch-completed', home_activity)
 
                 startup_time = time.time() - home_activity.get_launch_time()
                 logging.debug('%s launched in %f seconds.',
@@ -560,7 +588,6 @@ class ShellModel(gobject.GObject):
                              " was not found in the bundle registry."
                              % service_name)
         home_activity = Activity(activity_info, activity_id)
-        home_activity.props.launching = True
         self._add_activity(home_activity)
 
         self._set_active_activity(home_activity)
@@ -577,9 +604,10 @@ class ShellModel(gobject.GObject):
         if home_activity:
             logging.debug("Activity %s (%s) launch failed", activity_id,
                 home_activity.get_type())
-            if home_activity.props.launching:
+            if self.get_launcher(activity_id) is not None:
                 self.emit('launch-failed', home_activity)
             else:
+                # activity sent failure notification after closing launcher
                 self._remove_activity(home_activity)
         else:
             logging.error('Model for activity id %s does not exist.',
@@ -592,7 +620,7 @@ class ShellModel(gobject.GObject):
             logging.debug('Activity %s has been closed already.', activity_id)
             return False
 
-        if home_activity.props.launching:
+        if self.get_launcher(activity_id) is not None:
             logging.debug('Activity %s still launching, assuming it failed.',
                 activity_id)
             self.notify_launch_failed(activity_id)
diff --git a/src/jarabe/view/launcher.py b/src/jarabe/view/launcher.py
index 422a49a..89251e5 100644
--- a/src/jarabe/view/launcher.py
+++ b/src/jarabe/view/launcher.py
@@ -156,9 +156,6 @@ class _Animation(animator.Animation):
         self._icon.props.size = int(self.start_size + d)
 
 
-_launchers = {}
-
-
 def setup():
     model = shell.get_model()
     model.connect('launch-started', __launch_started_cb)
@@ -167,14 +164,15 @@ def setup():
 
 
 def add_launcher(activity_id, icon_path, icon_color):
+    model = shell.get_model()
 
-    if activity_id in _launchers:
+    if model.get_launcher(activity_id) is not None:
         return
 
     launch_window = LaunchWindow(activity_id, icon_path, icon_color)
     launch_window.show()
 
-    _launchers[activity_id] = launch_window
+    model.register_launcher(activity_id, launch_window)
 
 
 def __launch_started_cb(home_model, home_activity):
@@ -184,7 +182,7 @@ def __launch_started_cb(home_model, home_activity):
 
 def __launch_failed_cb(home_model, home_activity):
     activity_id = home_activity.get_activity_id()
-    launcher = _launchers.get(activity_id)
+    launcher = shell.get_model().get_launcher(activity_id)
 
     if launcher is None:
         logging.error('Launcher for %s is missing', activity_id)
@@ -209,8 +207,11 @@ def __launch_completed_cb(home_model, home_activity):
 def _destroy_launcher(home_activity):
     activity_id = home_activity.get_activity_id()
 
-    if activity_id in _launchers:
-        _launchers[activity_id].destroy()
-        del _launchers[activity_id]
-    else:
-        logging.error('Launcher for %s is missing', activity_id)
+    launcher = shell.get_model().get_launcher(activity_id)
+    if launcher is None:
+        if not home_activity.is_journal():
+            logging.error('Launcher was not registered for %s', activity_id)
+        return
+
+    shell.get_model().unregister_launcher(activity_id)
+    launcher.destroy()
diff --git a/src/jarabe/view/palettes.py b/src/jarabe/view/palettes.py
index 2beceff..ad84f08 100644
--- a/src/jarabe/view/palettes.py
+++ b/src/jarabe/view/palettes.py
@@ -39,20 +39,32 @@ class BasePalette(Palette):
     def __init__(self, home_activity):
         Palette.__init__(self)
 
-        if home_activity.props.launching:
-            home_activity.connect('notify::launching',
-                                  self._launching_changed_cb)
+        self._notify_launch_hid = None
+
+        if home_activity.props.launch_status == shell.Activity.LAUNCHING:
+            self._notify_launch_hid = home_activity.connect( \
+                    'notify::launch-status', self.__notify_launch_status_cb)
             self.set_primary_text(_('Starting...'))
+        elif home_activity.props.launch_status == shell.Activity.LAUNCH_FAILED:
+            self._on_failed_launch()
         else:
             self.setup_palette()
 
-    def _launching_changed_cb(self, home_activity, pspec):
-        if not home_activity.props.launching:
-            self.setup_palette()
-
     def setup_palette(self):
         raise NotImplementedError
 
+    def _on_failed_launch(self):
+        self.set_primary_text(_('Activity failed to start'))
+
+    def __notify_launch_status_cb(self, home_activity, pspec):
+        home_activity.disconnect(self._notify_launch_hid)
+        self._notify_launch_hid = None
+        if home_activity.props.launch_status == shell.Activity.LAUNCH_FAILED:
+            self._on_failed_launch()
+        else:
+            self.setup_palette()
+
+
 class CurrentActivityPalette(BasePalette):
     def __init__(self, home_activity):
         self._home_activity = home_activity
diff --git a/src/jarabe/view/service.py b/src/jarabe/view/service.py
index fbcc961..bb71694 100644
--- a/src/jarabe/view/service.py
+++ b/src/jarabe/view/service.py
@@ -83,7 +83,7 @@ class UIService(dbus.service.Object):
 
         if activity is not None and activity.get_window() is not None:
             activity.get_window().activate(gtk.get_current_event_time())
-            return not activity.props.launching
+            return self._shell_model.get_launcher(activity_id) is None
 
         return False