Re: [Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
Pushed as dc8f6ed7852f919fe7123d458706fb82430257e9 Sascha gave his formal bless, too. Regards, Simon ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
On 02/17/2011 01:21 PM, Martin Langhoff wrote:
On Wed, Feb 16, 2011 at 12:54 PM, Simon Schampijer wrote:
the general approach - just a few small comments inline.
Thanks! There's a patch in reply, and a few comments from me here...
+def remove_window_by_xid(self, xid):
+"""Remove a window from the windows stack."""
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+# must break after changing array
I think this comment can go away. The code itself should be clear enough.
Sugar developers are smarter than I expect then :-) My concern is
about someone "expanding" the code inside the loop, and getting bitten
by my doing something that is not quite kosher.
I think this code is ok without the comment :) Actually there is another
occasion of the same code that has no comment - if we think it is
necessary we should have it in both.
@@ -510,11 +529,12 @@ class ShellModel(gobject.GObject):
home_activity = Activity(activity_info, activity_id,
window)
self._add_activity(home_activity)
else:
-home_activity.set_window(window)
+logging.debug('setting new window for existing activity')
+home_activity.add_window(window)
This code gets called for each window including the launcher. To make that
clear maybe you can come up with another message.
I tried to make things clearer with a comment block for the function,
and more explicit debug lines. HTH.
Yes, looks good now.
logging.debug('%s launched in %f seconds.',
home_activity.get_type(), startup_time)
With the current logic the debug log does not log what we expect,
Here is where we were crashing in race conditions :-/ Anyway, I had
that as a separate patch (posted together w this one). Now I've merged
this change in the patch too.
Thanks - looks good now.
Regards,
Simon
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
On Wed, Feb 16, 2011 at 12:54 PM, Simon Schampijer wrote:
> the general approach - just a few small comments inline.
Thanks! There's a patch in reply, and a few comments from me here...
>> + def remove_window_by_xid(self, xid):
>> + """Remove a window from the windows stack."""
>> + for wnd in self._windows:
>> + if wnd.get_xid() == xid:
>> + # must break after changing array
>
> I think this comment can go away. The code itself should be clear enough.
Sugar developers are smarter than I expect then :-) My concern is
about someone "expanding" the code inside the loop, and getting bitten
by my doing something that is not quite kosher.
>> @@ -510,11 +529,12 @@ class ShellModel(gobject.GObject):
>> home_activity = Activity(activity_info, activity_id,
>> window)
>> self._add_activity(home_activity)
>> else:
>> - home_activity.set_window(window)
>> + logging.debug('setting new window for existing activity')
>> + home_activity.add_window(window)
>
> This code gets called for each window including the launcher. To make that
> clear maybe you can come up with another message.
I tried to make things clearer with a comment block for the function,
and more explicit debug lines. HTH.
>> logging.debug('%s launched in %f seconds.',
>> home_activity.get_type(), startup_time)
>
> With the current logic the debug log does not log what we expect,
Here is where we were crashing in race conditions :-/ Anyway, I had
that as a separate patch (posted together w this one). Now I've merged
this change in the patch too.
cheers,
m
--
martin.langh...@gmail.com
mar...@laptop.org -- Software Architect - OLPC
- ask interesting questions
- don't get distracted with shiny stuff - working code first
- http://wiki.laptop.org/go/User:Martinlanghoff
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel
Re: [Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
On 02/12/2011 05:27 PM, martin.langh...@gmail.com wrote:
From: Martin Langhoff
Now activities have a stack of windows (Activity._windows). The
lowest still-valid window in the stack is considered the main window.
When a window is closed, the shell finds what activity had that window,
tells it to remove it from its stack. If that was the last window
in the activities' stack, the activity is marked as closed.
With this patch, activities can switch from one window to another
safely, without sugar shell losing track of them. Activities based
on AdobeAIR have been seen to switch windows (perhaps when they navigate
from one SWF to another).
This replaces the one-window = one-activity model, where the moment
an activity closed one window (perhaps one of many) it was considered
closed by the shell.
---
Amended version, with a fixup in the call to wm.get_sugar_window_type().
Hi Martin,
thanks for your patch and the detailed description. The case you
describe is valid and I know that you have been testing your patch
thoroughly. I like the general approach - just a few small comments inline.
Regards,
Simon
src/jarabe/model/shell.py | 81 +---
1 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/src/jarabe/model/shell.py b/src/jarabe/model/shell.py
index 661e370..135d1a1 100644
--- a/src/jarabe/model/shell.py
+++ b/src/jarabe/model/shell.py
@@ -62,11 +62,12 @@ class Activity(gobject.GObject):
the "type" of activity being created.
activity_id -- unique identifier for this instance
of the activity type
-window -- Main WnckWindow of the activity
+_windows -- WnckWindows registered for the activity. The lowest
+one in the stack is the main window.
"""
gobject.GObject.__init__(self)
-self._window = None
+self._windows = []
self._service = None
self._activity_id = activity_id
self._activity_info = activity_info
@@ -74,7 +75,7 @@ class Activity(gobject.GObject):
self._launch_status = Activity.LAUNCHING
if window is not None:
-self.set_window(window)
+self.add_window(window)
self._retrieve_service()
@@ -96,15 +97,20 @@ class Activity(gobject.GObject):
launch_status = gobject.property(getter=get_launch_status)
-def set_window(self, window):
-"""Set the window for the activity
-
-We allow resetting the window for an activity so that we
-can replace the launcher once we get its real window.
-"""
+def add_window(self, window):
+"""Add a window to the windows stack."""
if not window:
raise ValueError('window must be valid')
-self._window = window
+self._windows.append(window)
+
+def remove_window_by_xid(self, xid):
+"""Remove a window from the windows stack."""
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+# must break after changing array
I think this comment can go away. The code itself should be clear enough.
+self._windows.remove(wnd)
+return True
+return False
def get_service(self):
"""Get the activity service
@@ -118,8 +124,8 @@ class Activity(gobject.GObject):
def get_title(self):
"""Retrieve the application's root window's suggested title"""
-if self._window:
-return self._window.get_name()
+if self._windows:
+return self._windows[0].get_name()
else:
return ''
@@ -171,31 +177,44 @@ class Activity(gobject.GObject):
def get_xid(self):
"""Retrieve the X-windows ID of our root window"""
-if self._window is not None:
-return self._window.get_xid()
+if self._windows:
+return self._windows[0].get_xid()
else:
return None
+def has_xid(self, xid):
+"""Retrieve the X-windows ID of our root window"""
+if self._windows:
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+return True
+return None
We should return False here if we do not find a window and for the
docstring I would recommend something like "Check if an X-window with
the given xid is in the windows stack".
def get_window(self):
"""Retrieve the X-windows root window of this application
-This was stored by the set_window method, which was
-called by HomeModel._add_activity, which was called
+This was stored by the add_window method, which was
+called by HomeModel._add_activity, which was called
via a callback that looks for all 'window-opened'
events.
+We keep a stack of the windows. The lowest window in the
+stack that is still valid we consider the main one.
+
[Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
From: Martin Langhoff
Now activities have a stack of windows (Activity._windows). The
lowest still-valid window in the stack is considered the main window.
When a window is closed, the shell finds what activity had that window,
tells it to remove it from its stack. If that was the last window
in the activities' stack, the activity is marked as closed.
With this patch, activities can switch from one window to another
safely, without sugar shell losing track of them. Activities based
on AdobeAIR have been seen to switch windows (perhaps when they navigate
from one SWF to another).
This replaces the one-window = one-activity model, where the moment
an activity closed one window (perhaps one of many) it was considered
closed by the shell.
---
Amended version, with a fixup in the call to wm.get_sugar_window_type().
---
src/jarabe/model/shell.py | 81 +---
1 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/src/jarabe/model/shell.py b/src/jarabe/model/shell.py
index 661e370..135d1a1 100644
--- a/src/jarabe/model/shell.py
+++ b/src/jarabe/model/shell.py
@@ -62,11 +62,12 @@ class Activity(gobject.GObject):
the "type" of activity being created.
activity_id -- unique identifier for this instance
of the activity type
-window -- Main WnckWindow of the activity
+_windows -- WnckWindows registered for the activity. The lowest
+one in the stack is the main window.
"""
gobject.GObject.__init__(self)
-self._window = None
+self._windows = []
self._service = None
self._activity_id = activity_id
self._activity_info = activity_info
@@ -74,7 +75,7 @@ class Activity(gobject.GObject):
self._launch_status = Activity.LAUNCHING
if window is not None:
-self.set_window(window)
+self.add_window(window)
self._retrieve_service()
@@ -96,15 +97,20 @@ class Activity(gobject.GObject):
launch_status = gobject.property(getter=get_launch_status)
-def set_window(self, window):
-"""Set the window for the activity
-
-We allow resetting the window for an activity so that we
-can replace the launcher once we get its real window.
-"""
+def add_window(self, window):
+"""Add a window to the windows stack."""
if not window:
raise ValueError('window must be valid')
-self._window = window
+self._windows.append(window)
+
+def remove_window_by_xid(self, xid):
+"""Remove a window from the windows stack."""
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+# must break after changing array
+self._windows.remove(wnd)
+return True
+return False
def get_service(self):
"""Get the activity service
@@ -118,8 +124,8 @@ class Activity(gobject.GObject):
def get_title(self):
"""Retrieve the application's root window's suggested title"""
-if self._window:
-return self._window.get_name()
+if self._windows:
+return self._windows[0].get_name()
else:
return ''
@@ -171,31 +177,44 @@ class Activity(gobject.GObject):
def get_xid(self):
"""Retrieve the X-windows ID of our root window"""
-if self._window is not None:
-return self._window.get_xid()
+if self._windows:
+return self._windows[0].get_xid()
else:
return None
+def has_xid(self, xid):
+"""Retrieve the X-windows ID of our root window"""
+if self._windows:
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+return True
+return None
+
def get_window(self):
"""Retrieve the X-windows root window of this application
-This was stored by the set_window method, which was
-called by HomeModel._add_activity, which was called
+This was stored by the add_window method, which was
+called by HomeModel._add_activity, which was called
via a callback that looks for all 'window-opened'
events.
+We keep a stack of the windows. The lowest window in the
+stack that is still valid we consider the main one.
+
HomeModel currently uses a dbus service query on the
activity to determine to which HomeActivity the newly
launched window belongs.
"""
-return self._window
+ if self._windows:
+return self._windows[0]
+return None
def get_type(self):
"""Retrieve the activity bundle id for future reference"""
-if self._window is None:
+if not self._windows:
return None
else:
-return wm.get_bundle_id(self._window)
+return wm.get_bundle_id(s
Re: [Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
On Sat, Feb 12, 2011 at 2:09 PM, wrote: > diff --git a/src/jarabe/model/shell.py b/src/jarabe/model/shell.py > index 661e370..bd7e367 100644 I'd like to recall this particular patch. Apologies, posted the wrong version. cheers, m -- martin.langh...@gmail.com mar...@laptop.org -- Software Architect - OLPC - ask interesting questions - don't get distracted with shiny stuff - working code first - http://wiki.laptop.org/go/User:Martinlanghoff ___ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel
[Sugar-devel] [PATCH 1/2] shell: handle activities that cycle through windows dlo#10695
From: Martin Langhoff
Now activities have a stack of windows (Activity._windows). The
lowest still-valid window in the stack is considered the main window.
When a window is closed, the shell finds what activity had that window,
tells it to remove it from its stack. If that was the last window
in the activities' stack, the activity is marked as closed.
With this patch, activities can switch from one window to another
safely, without sugar shell losing track of them. Activities based
on AdobeAIR have been seen to switch windows (perhaps when they navigate
from one SWF to another).
This replaces the one-window = one-activity model, where the moment
an activity closed one window (perhaps one of many) it was considered
closed by the shell.
---
src/jarabe/model/shell.py | 81 +---
1 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/src/jarabe/model/shell.py b/src/jarabe/model/shell.py
index 661e370..bd7e367 100644
--- a/src/jarabe/model/shell.py
+++ b/src/jarabe/model/shell.py
@@ -62,11 +62,12 @@ class Activity(gobject.GObject):
the "type" of activity being created.
activity_id -- unique identifier for this instance
of the activity type
-window -- Main WnckWindow of the activity
+_windows -- WnckWindows registered for the activity. The lowest
+one in the stack is the main window.
"""
gobject.GObject.__init__(self)
-self._window = None
+self._windows = []
self._service = None
self._activity_id = activity_id
self._activity_info = activity_info
@@ -74,7 +75,7 @@ class Activity(gobject.GObject):
self._launch_status = Activity.LAUNCHING
if window is not None:
-self.set_window(window)
+self.add_window(window)
self._retrieve_service()
@@ -96,15 +97,20 @@ class Activity(gobject.GObject):
launch_status = gobject.property(getter=get_launch_status)
-def set_window(self, window):
-"""Set the window for the activity
-
-We allow resetting the window for an activity so that we
-can replace the launcher once we get its real window.
-"""
+def add_window(self, window):
+"""Add a window to the windows stack."""
if not window:
raise ValueError('window must be valid')
-self._window = window
+self._windows.append(window)
+
+def remove_window_by_xid(self, xid):
+"""Remove a window from the windows stack."""
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+# must break after changing array
+self._windows.remove(wnd)
+return True
+return False
def get_service(self):
"""Get the activity service
@@ -118,8 +124,8 @@ class Activity(gobject.GObject):
def get_title(self):
"""Retrieve the application's root window's suggested title"""
-if self._window:
-return self._window.get_name()
+if self._windows:
+return self._windows[0].get_name()
else:
return ''
@@ -171,31 +177,44 @@ class Activity(gobject.GObject):
def get_xid(self):
"""Retrieve the X-windows ID of our root window"""
-if self._window is not None:
-return self._window.get_xid()
+if self._windows:
+return self._windows[0].get_xid()
else:
return None
+def has_xid(self, xid):
+"""Retrieve the X-windows ID of our root window"""
+if self._windows:
+for wnd in self._windows:
+if wnd.get_xid() == xid:
+return True
+return None
+
def get_window(self):
"""Retrieve the X-windows root window of this application
-This was stored by the set_window method, which was
-called by HomeModel._add_activity, which was called
+This was stored by the add_window method, which was
+called by HomeModel._add_activity, which was called
via a callback that looks for all 'window-opened'
events.
+We keep a stack of the windows. The lowest window in the
+stack that is still valid we consider the main one.
+
HomeModel currently uses a dbus service query on the
activity to determine to which HomeActivity the newly
launched window belongs.
"""
-return self._window
+ if self._windows:
+return self._windows[0]
+return None
def get_type(self):
"""Retrieve the activity bundle id for future reference"""
-if self._window is None:
+if not self._windows:
return None
else:
-return wm.get_bundle_id(self._window)
+return wm.get_bundle_id(self._windows[0])
def is_journal(self):
"""Returns boolean if
