[gnome-software/fix-flatpak-related-progress: 9/11] flatpak: Update progress for some skipped ops

[Matthew Leeds <mwleeds src gnome org>]

commit aa53f08f3609d6ae124596435daf7641eb72453b
Author: Matthew Leeds <matthew leeds endlessm com>
Date:   Tue Jul 7 18:05:16 2020 -0700

    flatpak: Update progress for some skipped ops
    
    Currently we don't do any progress updates (gs_app_set_progress()) for
    operations in a FlatpakTransaction that are skipped. This is problematic
    in some situations such as when an app or runtime is up to date but its
    locale extension needs an update to add a language (because the user
    changed the system language since installing it). In these situations,
    the app's progress is stuck at "Preparing..." forever and never gets to
    1%. Fix the issue by updating progress for even skipped operations (in
    which case the progress calculation excludes the size of the app itself
    and only takes into account related operations' size).
    
    Ideally we would test this situation in gs-self-test.c, which already
    has a test for an extension update and connects to
    notify::progress on the main app, but in practice adding
    "g_assert_cmpint (progress_cnt, >, 0);" after "g_assert
    (got_progress_installing);" leads to that assertion failing. It seems
    the progress is only ever updated to GS_APP_PROGRESS_UNKNOWN even though
    in actual use this works. I also tried adding 100 MB to extension update
    in case the update was too small for progress to work, but still no
    luck. gs_plugins_flatpak_app_update_func() has its assertion on
    progress_cnt commented out, so I guess something is wrong even in the
    case where there are no extensions.

 plugins/flatpak/gs-flatpak-transaction.c | 43 +++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 4 deletions(-)
---
diff --git a/plugins/flatpak/gs-flatpak-transaction.c b/plugins/flatpak/gs-flatpak-transaction.c
index 2fe3f927..00e35b85 100644
--- a/plugins/flatpak/gs-flatpak-transaction.c
+++ b/plugins/flatpak/gs-flatpak-transaction.c
@@ -270,13 +270,41 @@ update_progress_for_op (GsFlatpakTransaction        *self,
                         FlatpakTransactionOperation *current_op,
                         FlatpakTransactionOperation *root_op)
 {
-       GsApp *root_app = _transaction_operation_get_app (root_op);
+       g_autoptr(GsApp) root_app = NULL;
        guint64 related_prior_download_bytes = 0;
        guint64 related_download_bytes = 0;
        guint64 current_bytes_transferred = flatpak_transaction_progress_get_bytes_transferred 
(current_progress);
        gboolean seen_current_op = FALSE, seen_root_op = FALSE;
+       gboolean root_op_skipped = flatpak_transaction_operation_get_is_skipped (root_op);
        guint percent;
 
+       /* If @root_op is being skipped and its GsApp isn't being
+        * installed/removed, don't update the progress on it. It may be that
+        * @root_op is the runtime of an app and the app is the thing the
+        * transaction was created for.
+        */
+       if (root_op_skipped) {
+               /* _transaction_operation_set_app() is only called on non-skipped ops */
+               const gchar *ref = flatpak_transaction_operation_get_ref (root_op);
+               root_app = _ref_to_app (self, ref);
+               if (root_app == NULL) {
+                       g_warning ("Couldn't find GsApp for transaction operation %s",
+                                  flatpak_transaction_operation_get_ref (root_op));
+                       return;
+               }
+               if (gs_app_get_state (root_app) != AS_APP_STATE_INSTALLING &&
+                   gs_app_get_state (root_app) != AS_APP_STATE_REMOVING)
+                       return;
+       } else {
+               GsApp *unskipped_root_app = _transaction_operation_get_app (root_op);
+               if (unskipped_root_app == NULL) {
+                       g_warning ("Couldn't find GsApp for transaction operation %s",
+                                  flatpak_transaction_operation_get_ref (root_op));
+                       return;
+               }
+               root_app = g_object_ref (unskipped_root_app);
+       }
+
        /* This relies on ops in a #FlatpakTransaction being run in the order
         * they’re returned by flatpak_transaction_get_operations(), which is true. */
        for (GList *l = ops; l != NULL; l = l->next) {
@@ -288,6 +316,12 @@ update_progress_for_op (GsFlatpakTransaction        *self,
                if (op == root_op)
                        seen_root_op = TRUE;
 
+               /* Currently libflatpak doesn't return skipped ops in
+                * flatpak_transaction_get_operations(), but check just in case.
+                */
+               if (op == root_op && root_op_skipped)
+                       continue;
+
                if (op_is_related_to_op (op, root_op)) {
                        /* Saturate instead of overflowing */
                        related_download_bytes = saturated_uint64_add (related_download_bytes, 
op_download_size);
@@ -297,7 +331,7 @@ update_progress_for_op (GsFlatpakTransaction        *self,
        }
 
        g_assert (related_prior_download_bytes <= related_download_bytes);
-       g_assert (seen_root_op);
+       g_assert (seen_root_op || root_op_skipped);
 
        /* Avoid overflows when converting to percent, at the cost of losing
         * some precision in the least significant digits. */
@@ -337,9 +371,10 @@ update_progress_for_op_recurse_up (GsFlatpakTransaction        *self,
 {
        GPtrArray *related_to_ops = flatpak_transaction_operation_get_related_to_ops (root_op);
 
-       if (!flatpak_transaction_operation_get_is_skipped (root_op))
-               update_progress_for_op (self, progress, ops, current_op, root_op);
+       /* Update progress for @root_op */
+       update_progress_for_op (self, progress, ops, current_op, root_op);
 
+       /* Update progress for ops related to @root_op, e.g. apps whose runtime is @root_op */
        for (gsize i = 0; related_to_ops != NULL && i < related_to_ops->len; i++) {
                FlatpakTransactionOperation *related_to_op = g_ptr_array_index (related_to_ops, i);
                update_progress_for_op_recurse_up (self, progress, ops, current_op, related_to_op);