Merge lp:~mvo/software-center/treeview-keep-state-on-db-cache-change into lp:software-center

[Michael Vogt]

This branch depends on the other branches:
lp:~mvo/software-center/lp964433
lp:~mvo/software-center/lp966879
lp:~mvo/software-center/lp846204

and adds the ability to save/restore the state of the installedpane treeview when the db or the cache changes.
This means that if you e.g. uninstal a application when the view refreshes the expanded nodes and scrollposition
is saved.

To test, go to the installed pane, expand some rows and run:
$ dbus-send --print-reply --dest=com.ubuntu.Softwarecenter /com/ubuntu/Softwarecenter com.ubuntu.SoftwarecenterIFace.triggerCacheReload

or simply remove a application there.

[Gary Lasker (gary-lasker)]

This is really nice! And definitely a welcome improvement over the complete collapse of the tree after a refresh of the treeview.

At some point we should look at removing the multiple refreshes (and the showing the spinner) that we get on an install/remove.

Thanks, Michael!

review: Approve

=== modified file 'softwarecenter/ui/gtk3/models/appstore2.py'
--- softwarecenter/ui/gtk3/models/appstore2.py	2012-03-16 10:44:05 +0000
+++ softwarecenter/ui/gtk3/models/appstore2.py	2012-03-29 14:01:25 +0000
@@ -61,6 +61,9 @@
         self.pkg_count = pkg_count
         self.vis_count = pkg_count
 
+    def __repr__(self):
+        return "[CategoryRowReference: name=%s]" % self.untranslated_name
+
 
 class UncategorisedRowRef(CategoryRowReference):
 
@@ -75,6 +78,9 @@
                                       display_name,
                                       None, pkg_count)
 
+    def __repr__(self):
+        return "[UncategorizedRowReference: name=%s]" % self.untranslated_name
+
 
 class AppPropertiesHelper(GObject.GObject):
     """ Baseclass that contains common functions for our
@@ -409,6 +415,7 @@
         """ set the content of the liststore based on a list of
             xapian.MSetItems
         """
+        LOG.debug("set_from_matches len(matches)='%s'" % len(matches))
         self.current_matches = matches
         n_matches = len(matches)
         if n_matches == 0:
@@ -432,6 +439,7 @@
         self.buffer_icons()
 
     def load_range(self, indices, step):
+        LOG.debug("load_range: %s %s" % (indices, step))
         db = self.db.xapiandb
         matches = self.current_matches
 
@@ -446,7 +454,8 @@
         for i in range(start, end):
             try:
                 row_content = self[(i,)][0]
-            except IndexError:
+            except IndexError as e:
+                LOG.warn("failed to load rows: '%s'" % e)
                 break
 
             if row_content:
 
=== modified file 'softwarecenter/ui/gtk3/panes/installedpane.py'
--- softwarecenter/ui/gtk3/panes/installedpane.py	2012-03-27 22:55:49 +0000
+++ softwarecenter/ui/gtk3/panes/installedpane.py	2012-03-29 14:01:25 +0000
@@ -325,8 +325,25 @@
         self.nonapps_visible = NonAppVisibility.NEVER_VISIBLE
         self.refresh_apps()
 
+    def _save_treeview_state(self):
+        # store the state
+        expanded_rows = []
+        self.app_view.tree_view.map_expanded_rows(
+            lambda view, path, data: expanded_rows.append(path.to_string()),
+            None)
+        vadj = self.app_view.tree_view_scroll.get_vadjustment().get_value()
+        return expanded_rows, vadj
+
+    def _restore_treeview_state(self, state):
+        expanded_rows, vadj = state
+        for ind in expanded_rows:
+            path = Gtk.TreePath.new_from_string(ind)
+            self.app_view.tree_view.expand_row(path, False)
+        self.app_view.tree_view_scroll.get_vadjustment().set_lower(vadj)
+        self.app_view.tree_view_scroll.get_vadjustment().set_value(vadj)
+
     #~ @interrupt_build_and_wait
-    def _build_categorised_installedview(self):
+    def _build_categorised_installedview(self, keep_state=False):
         LOG.debug('Rebuilding categorised installedview...')
 
         # display the busy cursor and a local spinner while we build the view
@@ -335,6 +352,13 @@
             window.set_cursor(self.busy_cursor)
         self.show_installed_view_spinner()
 
+        if keep_state:
+            treeview_state = self._save_treeview_state()
+
+        # disconnect the model to avoid e.g. updates of "cursor-changed"
+        #  AppTreeView.expand_path while the model is in rebuild-flux
+        self.app_view.set_model(None)
+
         model = self.base_model  # base model not treefilter
         model.clear()
 
@@ -410,6 +434,10 @@
             self.app_view._append_appcount(self.installed_count,
                 mode=AppView.INSTALLED_MODE)
 
+            self.app_view.set_model(self.treefilter)
+            if keep_state:
+                self._restore_treeview_state(treeview_state)
+
             # hide the local spinner
             self.hide_installed_view_spinner()
 
@@ -425,7 +453,7 @@
 
         GObject.idle_add(profiled_rebuild_categorised_view)
 
-    def _build_oneconfview(self):
+    def _build_oneconfview(self, keep_state=False):
         LOG.debug('Rebuilding oneconfview for %s...' % self.current_hostid)
 
         # display the busy cursor and the local spinner while we build the view
@@ -434,6 +462,13 @@
             window.set_cursor(self.busy_cursor)
         self.show_installed_view_spinner()
 
+        if keep_state:
+            treeview_state = self._save_treeview_state()
+
+        # disconnect the model to avoid e.g. updates of "cursor-changed"
+        #  AppTreeView.expand_path while the model is in rebuild-flux
+        self.app_view.set_model(None)
+
         model = self.base_model  # base model not treefilter
         model.clear()
 
@@ -518,6 +553,10 @@
             self.app_view._append_appcount(self.installed_count,
                 mode=AppView.DIFF_MODE)
 
+            self.app_view.set_model(self.treefilter)
+            if keep_state:
+                self._restore_treeview_state(treeview_state)
+
             # hide the local spinner
             self.hide_installed_view_spinner()
 
@@ -599,10 +638,11 @@
     def refresh_apps(self, *args, **kwargs):
         """refresh the applist and update the navigation bar """
         logging.debug("installedpane refresh_apps")
+        keep_state = kwargs.get("keep_state", False)
         if self.current_hostid:
-            self._build_oneconfview()
+            self._build_oneconfview(keep_state)
         else:
-            self._build_categorised_installedview()
+            self._build_categorised_installedview(keep_state)
 
     def _clear_search(self):
         # remove the details and clear the search
@@ -629,9 +669,17 @@
         self.app_view._append_appcount(appcount, mode=AppView.DIFF_MODE)
         return vis_cats
 
+    def _refresh_on_cache_or_db_change(self):
+        self.refresh_apps(keep_state=True)
+        self.app_details_view.refresh_app()
+
     def on_db_reopen(self, db):
-        self.refresh_apps(rebuild=True)
-        self.app_details_view.refresh_app()
+        LOG.debug("on_db_reopen")
+        self._refresh_on_cache_or_db_change()
+
+    def on_cache_ready(self, cache):
+        LOG.debug("on_cache_ready")
+        self._refresh_on_cache_or_db_change()
 
     def on_application_selected(self, appview, app):
         """callback when an app is selected"""
 
=== modified file 'softwarecenter/ui/gtk3/panes/softwarepane.py'
--- softwarecenter/ui/gtk3/panes/softwarepane.py	2012-03-20 08:23:24 +0000
+++ softwarecenter/ui/gtk3/panes/softwarepane.py	2012-03-29 14:01:25 +0000
@@ -254,15 +254,6 @@
     def on_cache_ready(self, cache):
         " refresh the application list when the cache is re-opened "
         LOG.debug("on_cache_ready")
-        # it only makes sense to refresh if there is something to
-        # refresh, otherwise we create a bunch of (not yet needed)
-        # AppStore objects on startup when the cache sends its
-        # initial "cache-ready" signal
-        model = self.app_view.tree_view.get_model()
-        if model is None:
-            return
-        # FIXME: preserve selection too
-        self.refresh_apps()
 
     @wait_for_apt_cache_ready
     def on_application_activated(self, appview, app):
 
=== modified file 'softwarecenter/ui/gtk3/views/appview.py'
--- softwarecenter/ui/gtk3/views/appview.py	2012-03-14 16:34:56 +0000
+++ softwarecenter/ui/gtk3/views/appview.py	2012-03-29 14:01:25 +0000
@@ -176,13 +176,16 @@
     def set_model(self, model):
         self.tree_view.set_model(model)
 
+    def get_model(self):
+        return self.tree_view.appmodel
+
     def display_matches(self, matches, is_search=False):
         # FIXME: installedpane handles display of the trees intimately,
         # so for the time being lets just return None in the case of our
         # TreeView displaying an AppTreeStore ...    ;(
         # ... also we dont currently support user sorting in the
         # installedview, so issue is somewhat moot for the time being...
-        if isinstance(self.tree_view.appmodel, AppTreeStore):
+        if isinstance(self.get_model(), AppTreeStore):
             LOG.debug("display_matches called on AppTreeStore, ignoring")
             return
 
@@ -197,9 +200,16 @@
                 not self.user_defined_sort_method):
                 self.set_sort_method_with_no_signal(self._SORT_BY_TOP_RATED)
 
-        model = self.tree_view.appmodel
+        model = self.get_model()
+        # disconnect the model from the view before running
+        # set_from_matches to ensure that the _cell_data_func_cb is not
+        # run when the placeholder items are set, otherwise the purpose
+        # of the "load-on-demand" is gone and it leads to bugs like
+        # LP: #964433
+        self.set_model(None)
         if model:
             model.set_from_matches(matches)
+        self.set_model(model)
         self.user_defined_sort_method = False
 
         self.tree_view_scroll.get_vadjustment().set_lower(self.vadj)
 
=== modified file 'softwarecenter/ui/gtk3/widgets/apptreeview.py'
--- softwarecenter/ui/gtk3/widgets/apptreeview.py	2012-03-22 23:32:06 +0000
+++ softwarecenter/ui/gtk3/widgets/apptreeview.py	2012-03-29 14:01:25 +0000
@@ -20,6 +20,9 @@
     AppGenericStore, CategoryRowReference)
 
 
+LOG = logging.getLogger(__name__)
+
+
 class AppTreeView(Gtk.TreeView):
 
     """Treeview based view component that takes a AppStore and displays it"""
@@ -144,16 +147,16 @@
 
         if old is not None:
             start, end = self.get_visible_range() or (None, None)
-            if (start and start.compare(old) != -1) or \
-               (end and end.compare(old) != 1):
+            if ((start and start.compare(old) != -1) or
+                (end and end.compare(old) != 1)):
                 self._needs_collapse.append(old)
             else:
                 try:  # try... a lazy solution to Bug #846204
                     model.row_changed(old, model.get_iter(old))
                 except:
-                    msg = ("apptreeview.expand_path: Supplied 'old' "
-                           "path is an invalid tree path: '%s'" % old)
-                    logging.debug(msg)
+                    LOG.exception(
+                        "apptreeview.expand_path: Supplied 'old' "
+                        "path is an invalid tree path: '%s'" % old)
 
         if path == None:
             return
@@ -485,13 +488,8 @@
                             path)
 
     def _cell_data_func_cb(self, col, cell, model, it, user_data):
-
         path = model.get_path(it)
 
-        # this will give us the right underlying model regardless if its
-        # a TreeModelFilter, a AppTreeStore or a AppListStore
-        model = self.appmodel
-
         # this will pre-load data *only* on a AppListStore, it has
         # no effect with a AppTreeStore
         if model[path][0] is None: