Merge lp:~gary-lasker/software-center/fix-crash-lp969732 into lp:software-center

Proposed by Gary Lasker
Status: Merged
Merge reported by: Gary Lasker
Merged at revision: not available
Proposed branch: lp:~gary-lasker/software-center/fix-crash-lp969732
Merge into: lp:software-center
Diff against target: 18 lines (+8/-0)
1 file modified
softwarecenter/ui/gtk3/models/appstore2.py (+8/-0)
To merge this branch: bzr merge lp:~gary-lasker/software-center/fix-crash-lp969732
Reviewer Review Type Date Requested Status
software-store-developers Pending
Review via email: mp+104191@code.launchpad.net

Description of the change

This branch fixes the crash described in bug 969732 by explicitly declaring the "needs-refresh" signal in the AppTreeStore class. Please see the description for bug 969732 where I have added steps to reproduce.

Thanks for your review!

To post a comment you must log in.
Michael Vogt (mvo) wrote :

Thanks for this branch!

It looks good, but I'm a bit puzzled why its needed there as AppTreeStore does not itself defines
__gsignals__ so it should set this up itself. I also can't reproduce this issue because of a bug in
ubuntu-sso-client currently. But I wonder if its maybe a order issue? And would love to know if:

=== modified file 'softwarecenter/ui/gtk3/models/appstore2.py'
--- softwarecenter/ui/gtk3/models/appstore2.py 2012-03-30 09:11:08 +0000
+++ softwarecenter/ui/gtk3/models/appstore2.py 2012-05-01 21:50:46 +0000
@@ -478,9 +478,9 @@

     def __init__(self, db, cache, icons, icon_size=AppGenericStore.ICON_SIZE,
                  global_icon_cache=True):
- AppGenericStore.__init__(
- self, db, cache, icons, icon_size, global_icon_cache)
         Gtk.TreeStore.__init__(self)
+ AppGenericStore.__init__(
+ self, db, cache, icons, icon_size, global_icon_cache)
         self.set_column_types(self.COL_TYPES)

     def set_documents(self, parent, documents):

works, i.e. just shuffling the __init__ around.

Gary Lasker (gary-lasker) wrote :

Hi Michael, thanks for your review. I tried your suggestion (shuffling the inits per your diff), but it made no difference and the same crash occurred using the steps to reproduce in the bug.

Note that we have lots of other instances in our code where subclasses have to explicitly declare signals from superclasses. Note, in fact, that AppListStore does exactly the same thing for the AppPropertiesHelper superclass. I guess the need for this is just an unfortunate quirk of GTK Python?

What is the bug in ubuntu-sso-client?

Thanks again!

Michael Vogt (mvo) wrote :

On Wed, May 02, 2012 at 01:40:24AM -0000, Gary Lasker wrote:
> Hi Michael, thanks for your review. I tried your suggestion (shuffling the inits per your diff), but it made no difference and the same crash occurred using the steps to reproduce in the bug.

Thanks for testing this, I created a minimal test case now for this and
it turns out that the problem is the multiple inheritance that makes
the __gsignals__ stuff not work.

from gi.repository import GObject, Gtk

class Foo(GObject.GObject):
    __gsignals__ = {
        "foo": (GObject.SignalFlags.RUN_LAST,
                None,
                ( ),
                ),
        }

class Baz(Foo):
    pass

class Bar(Gtk.ListStore, Foo):
    pass

if __name__ == "__main__":
    foo = Foo()
    foo.emit("foo")
    baz = Baz()
    baz.emit("foo")
    bar = Bar()
    bar.emit("foo")

> Note that we have lots of other instances in our code where subclasses have to explicitly declare signals from superclasses. Note, in fact, that AppListStore does exactly the same thing for the AppPropertiesHelper superclass. I guess the need for this is just an unfortunate quirk of GTK Python?

Right, in the AppListStore case there is another __gsignals__ so it
"shadows" the one from the superclass (which is in itself bad that it
can not be auto-merged :/) but I wasn't aware of the limitation when
multiple inheritance is involved. So +1 for this fix, especially as it
seems there is simply no way to add a gsignal except for the
__gsignals__ way :/

> What is the bug in ubuntu-sso-client?

I didn't had the time to investigate this unfortuantely but its a
assert failure in FindCredentials that looks like its really a bug in
the sso-client.

Cheers,
 Michael

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/ui/gtk3/models/appstore2.py'
2--- softwarecenter/ui/gtk3/models/appstore2.py 2012-03-30 09:11:08 +0000
3+++ softwarecenter/ui/gtk3/models/appstore2.py 2012-05-01 00:25:24 +0000
4@@ -476,6 +476,14 @@
5 """ A treestore based application model
6 """
7
8+ __gsignals__ = {
9+ # meh, this is a signal from AppPropertiesHelper
10+ "needs-refresh": (GObject.SignalFlags.RUN_LAST,
11+ None,
12+ (str, ),
13+ ),
14+ }
15+
16 def __init__(self, db, cache, icons, icon_size=AppGenericStore.ICON_SIZE,
17 global_icon_cache=True):
18 AppGenericStore.__init__(

Subscribers

People subscribed via source and target branches