Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web activity: quit the GTK main loop when activity is destroyed - SL #46... #29

Closed
wants to merge 1 commit into from

Conversation

manuq
Copy link
Contributor

@manuq manuq commented Aug 21, 2013

...03

In GTK activities, this is done by _ActivitySession when the activity
unregisters itself, in _complete_close method.

…#4603

In GTK activities, this is done by _ActivitySession when the activity
unregisters itself, in _complete_close method.
@erikos
Copy link
Contributor

erikos commented Aug 24, 2013

Is it actually desired to add the web activities as well to the session? If we want to storage before a shutdown that would be necessary.

diff --git a/src/sugar3/activity/activity.py b/src/sugar3/activity/activity.py
index b0339ec..4a6ba79 100644
--- a/src/sugar3/activity/activity.py
+++ b/src/sugar3/activity/activity.py
@@ -308,7 +308,7 @@ class Activity(Window, Gtk.Container):
         self._jobject = None
         self._read_file_called = False

-        self._session = _get_session()
+        self._session = get_session()
         self._session.register(self)
         self._session.connect('quit-requested',
                               self.__session_quit_requested_cb)
@@ -1033,7 +1033,7 @@ class _ClientHandler(dbus.service.Object, DBusProperties):
 _session = None


-def _get_session():
+def get_session():
     global _session

     if _session is None:
diff --git a/src/sugar3/activity/webactivity.py b/src/sugar3/activity/webactivity.py
index 4ea31d0..273ddd7 100644
--- a/src/sugar3/activity/webactivity.py
+++ b/src/sugar3/activity/webactivity.py
@@ -42,6 +42,12 @@ class WebActivity(Gtk.Window):
         self.set_decorated(False)
         self.maximize()

+        self._session = activity.get_session()
+        self._session.register(self)
+        self._session.connect('quit-requested',
+                              self.__session_quit_requested_cb)
+        self._session.connect('quit', self.__session_quit_cb)
+
         self.connect("key-press-event", self._key_press_event_cb)
         self.connect('realize', self._realize_cb)

@@ -62,6 +68,13 @@ class WebActivity(Gtk.Window):
     def run_main_loop(self):
         Gtk.main()

+    def __session_quit_requested_cb(self, session):
+        session.will_quit(self, True)
+
+    def __session_quit_cb(self, client):
+        self.destroy()
+        self._session.unregister(self)
+
     def _realize_cb(self, window):
         xid = window.get_window().get_xid()
         SugarExt.wm_set_bundle_id(xid, self._bundle_id)

@manuq
Copy link
Contributor Author

manuq commented Aug 26, 2013

Oh, thanks Simon, I wasn't aware of the session.

So we need to find a way to send "activity.stop" notification to the web activity when the session emits "quit-requested". This is how we implemented it:

sugarlabs/sugar@87e98af#L0R73

The problem is that we have apisocket in the shell side.

@dnarvaez
Copy link
Contributor

I think long term (perhaps not so long), session management will be handled by the shell. The X session protocol is pretty much obsolete, if I'm not mistake GNOME is not using it these days... And anyway it will go away with wayland.

So I think what we should do is to add signals to SessionManager and have apisocket listen.

@manuq
Copy link
Contributor Author

manuq commented Aug 29, 2013

Ok, did the session as we agreed yesterday, in shell SessionManager. Found a little surprise: the Journal is not being removed. See this commit:

https://github.com/manuq/sugar-1/commit/a1e96c8661b103ec90b8afb4658aadc207667e7a

I left debugging logs which show the Journal issue.

TestCase:

  • start sugar
  • start Gears web activity
  • make changes
  • logout sugar (without manually closing Gears before)
  • restart sugar
  • restart the previous instance of the Gears web activity

Expected: last changes in Gears instance should be there.

Note that what we are trying to solve with the commit above is another issue, different than the one depicted in SL #4603 . If we are going to fix the session like we agreed, this pull request should be pushed. It is very hard to test web activities and further develop sugar-shell in the current state.

@manuq
Copy link
Contributor Author

manuq commented Aug 30, 2013

Pushed after Daniel agreed, with an amended message that explains session and adds "Fixes #.."

@manuq manuq closed this Aug 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants