[gnome-software] gs-app: Add GS_APP_PROGRESS_UNKNOWN and clarify unknown behaviour

[Richard Hughes <rhughes src gnome org>]

commit a5d2bbbb8847f189f7bb594d8c1917adb1db85aa
Author: Philip Withnall <withnall endlessm com>
Date:   Thu May 14 13:12:40 2020 +0100

    gs-app: Add GS_APP_PROGRESS_UNKNOWN and clarify unknown behaviour
    
    The code previously used a mixture of assuming that 0% progress meant
    the progress is unknown (which is not strictly true) and treating it as
    a normal progress value.
    
    Add a new `GS_APP_PROGRESS_UNKNOWN` constant which should be used to
    explicitly indicate that progress is unknown. Adjust the rest of the
    code in `GsApp` and `GsAppList` to consistently handle this separately
    from percentage progress values 0–100.
    
    In particular, adjust the progress calculation for `GsAppList`s to
    return `GS_APP_PROGRESS_UNKNOWN` for the whole `GsAppList` if any app
    has unknown progress, or if the app list is empty. Those cases were
    ignored before.
    
    I think a separate property for indicating unknown progress would make
    more sense than a constant, but those changes would be too invasive.
    
    Signed-off-by: Philip Withnall <withnall endlessm com>
    Helps: #276

 lib/gs-app-list.c | 32 ++++++++++++++++++++++++++------
 lib/gs-app.c      | 34 ++++++++++++++++++++++------------
 lib/gs-app.h      | 11 +++++++++++
 3 files changed, 59 insertions(+), 18 deletions(-)
---
diff --git a/lib/gs-app-list.c b/lib/gs-app-list.c
index 3030f296..4f578078 100644
--- a/lib/gs-app-list.c
+++ b/lib/gs-app-list.c
@@ -32,7 +32,7 @@ struct _GsAppList
        guint                    size_peak;
        GsAppListFlags           flags;
        AsAppState               state;
-       guint                    progress;
+       guint                    progress;  /* 0–100 inclusive, or %GS_APP_PROGRESS_UNKNOWN */
 };
 
 G_DEFINE_TYPE (GsAppList, gs_app_list, G_TYPE_OBJECT)
@@ -67,19 +67,21 @@ gs_app_list_get_state (GsAppList *list)
  * gs_app_list_get_progress:
  * @list: A #GsAppList
  *
- * Gets the average percentage completion of all apps in the list.
+ * Gets the average percentage completion of all apps in the list. If any of the
+ * apps in the list has progress %GS_APP_PROGRESS_UNKNOWN, or if the app list
+ * is empty, %GS_APP_PROGRESS_UNKNOWN will be returned.
  *
  * This method will only return a valid result if gs_app_list_add_flag() has
  * been called with %GS_APP_LIST_FLAG_WATCH_APPS.
  *
- * Returns: the percentage completion, or 0 for unknown
+ * Returns: the percentage completion (0–100 inclusive), or %GS_APP_PROGRESS_UNKNOWN for unknown
  *
  * Since: 3.30
  **/
 guint
 gs_app_list_get_progress (GsAppList *list)
 {
-       g_return_val_if_fail (GS_IS_APP_LIST (list), 0);
+       g_return_val_if_fail (GS_IS_APP_LIST (list), GS_APP_PROGRESS_UNKNOWN);
        return list->progress;
 }
 
@@ -132,12 +134,24 @@ gs_app_list_invalidate_progress (GsAppList *self)
        /* find the average percentage complete of the list */
        if (apps->len > 0) {
                guint64 pc_cnt = 0;
+               gboolean unknown_seen = FALSE;
+
                for (guint i = 0; i < apps->len; i++) {
                        GsApp *app_tmp = g_ptr_array_index (apps, i);
+                       guint app_progress = gs_app_get_progress (app_tmp);
+
+                       if (app_progress == GS_APP_PROGRESS_UNKNOWN) {
+                               unknown_seen = TRUE;
+                               break;
+                       }
                        pc_cnt += gs_app_get_progress (app_tmp);
                }
-               progress = pc_cnt / apps->len;
+
+               progress = (!unknown_seen) ? pc_cnt / apps->len : GS_APP_PROGRESS_UNKNOWN;
+       } else {
+               progress = GS_APP_PROGRESS_UNKNOWN;
        }
+
        if (self->progress != progress) {
                self->progress = progress;
                g_object_notify (G_OBJECT (self), "progress");
@@ -911,8 +925,14 @@ gs_app_list_class_init (GsAppListClass *klass)
 
        /**
         * GsAppList:progress:
+        *
+        * A percentage (0–100, inclusive) indicating the progress through the
+        * current task on this app list. The value may otherwise be
+        * %GS_APP_PROGRESS_UNKNOWN if the progress is unknown or has a wide
+        * confidence interval on any app, or if the app list is empty.
         */
-       pspec = g_param_spec_uint ("progress", NULL, NULL, 0, 100, 0,
+       pspec = g_param_spec_uint ("progress", NULL, NULL,
+                                  0, GS_APP_PROGRESS_UNKNOWN, GS_APP_PROGRESS_UNKNOWN,
                                   G_PARAM_READABLE);
        g_object_class_install_property (object_class, PROP_PROGRESS, pspec);
 }
diff --git a/lib/gs-app.c b/lib/gs-app.c
index 5bc93fd1..f888a622 100644
--- a/lib/gs-app.c
+++ b/lib/gs-app.c
@@ -97,7 +97,7 @@ typedef struct
        AsAppState               state_recover;
        AsAppScope               scope;
        AsBundleKind             bundle_kind;
-       guint                    progress;
+       guint                    progress;  /* integer 0–100 (inclusive), or %GS_APP_PROGRESS_UNKNOWN */
        gboolean                 allow_cancel;
        GHashTable              *metadata;
        GsAppList               *addons;
@@ -442,7 +442,9 @@ gs_app_to_string_append (GsApp *app, GString *str)
                g_autofree gchar *qstr = gs_app_quirk_to_string (priv->quirk);
                gs_app_kv_lpad (str, "quirk", qstr);
        }
-       if (priv->progress > 0)
+       if (priv->progress == GS_APP_PROGRESS_UNKNOWN)
+               gs_app_kv_printf (str, "progress", "unknown");
+       else
                gs_app_kv_printf (str, "progress", "%u%%", priv->progress);
        if (priv->id != NULL)
                gs_app_kv_lpad (str, "id", priv->id);
@@ -837,7 +839,7 @@ gs_app_get_state (GsApp *app)
  *
  * Gets the percentage completion.
  *
- * Returns: the percentage completion, or 0 for unknown
+ * Returns: the percentage completion (0–100 inclusive), or %GS_APP_PROGRESS_UNKNOWN for unknown
  *
  * Since: 3.22
  **/
@@ -845,7 +847,7 @@ guint
 gs_app_get_progress (GsApp *app)
 {
        GsAppPrivate *priv = gs_app_get_instance_private (app);
-       g_return_val_if_fail (GS_IS_APP (app), 0);
+       g_return_val_if_fail (GS_IS_APP (app), GS_APP_PROGRESS_UNKNOWN);
        return priv->progress;
 }
 
@@ -892,7 +894,7 @@ gs_app_set_state_recover (GsApp *app)
 
        /* make sure progress gets reset when recovering state, to prevent
         * confusing initial states when going through more than one attempt */
-       gs_app_set_progress (app, 0);
+       gs_app_set_progress (app, GS_APP_PROGRESS_UNKNOWN);
 
        priv->state = priv->state_recover;
        gs_app_queue_notify (app, obj_props[PROP_STATE]);
@@ -1026,9 +1028,12 @@ gs_app_set_state_internal (GsApp *app, AsAppState state)
 /**
  * gs_app_set_progress:
  * @app: a #GsApp
- * @percentage: a percentage progress
+ * @percentage: a percentage progress (0–100 inclusive), or %GS_APP_PROGRESS_UNKNOWN
+ *
+ * This sets the progress completion of the application. Use
+ * %GS_APP_PROGRESS_UNKNOWN if the progress is unknown or has a wide confidence
+ * interval.
  *
- * This sets the progress completion of the application.
  * If called more than once with the same value then subsequent calls
  * will be ignored.
  *
@@ -1043,9 +1048,9 @@ gs_app_set_progress (GsApp *app, guint percentage)
        locker = g_mutex_locker_new (&priv->mutex);
        if (priv->progress == percentage)
                return;
-       if (percentage > 100) {
-               g_debug ("cannot set %u%% for %s, setting instead: 100%%",
-                        percentage, gs_app_get_unique_id_unlocked (app));
+       if (percentage != GS_APP_PROGRESS_UNKNOWN && percentage > 100) {
+               g_warning ("cannot set %u%% for %s, setting instead: 100%%",
+                          percentage, gs_app_get_unique_id_unlocked (app));
                percentage = 100;
        }
        priv->progress = percentage;
@@ -4145,7 +4150,7 @@ gs_app_set_property (GObject *object, guint prop_id, const GValue *value, GParam
                gs_app_set_state_internal (app, g_value_get_uint (value));
                break;
        case PROP_PROGRESS:
-               priv->progress = g_value_get_uint (value);
+               gs_app_set_progress (app, g_value_get_uint (value));
                break;
        case PROP_CAN_CANCEL_INSTALLATION:
                priv->allow_cancel = g_value_get_boolean (value);
@@ -4305,9 +4310,14 @@ gs_app_class_init (GsAppClass *klass)
 
        /**
         * GsApp:progress:
+        *
+        * A percentage (0–100, inclusive) indicating the progress through the
+        * current task on this app. The value may otherwise be
+        * %GS_APP_PROGRESS_UNKNOWN if the progress is unknown or has a wide
+        * confidence interval.
         */
        obj_props[PROP_PROGRESS] = g_param_spec_uint ("progress", NULL, NULL,
-                                  0, 100, 0,
+                                  0, GS_APP_PROGRESS_UNKNOWN, GS_APP_PROGRESS_UNKNOWN,
                                   G_PARAM_READWRITE | G_PARAM_CONSTRUCT);
 
        /**
diff --git a/lib/gs-app.h b/lib/gs-app.h
index 5cf1fa05..6b9e72e7 100644
--- a/lib/gs-app.h
+++ b/lib/gs-app.h
@@ -162,6 +162,17 @@ typedef enum {
 #define MEDIUM_PERMISSIONS (LIMITED_PERMISSIONS | \
                        GS_APP_PERMISSIONS_X11)
 
+/**
+ * GS_APP_PROGRESS_UNKNOWN:
+ *
+ * A value returned by gs_app_get_progress() if the app’s progress is unknown
+ * or has a wide confidence interval. Typically this would be represented in the
+ * UI using a pulsing progress bar or spinner.
+ *
+ * Since: 3.38
+ */
+#define GS_APP_PROGRESS_UNKNOWN G_MAXUINT
+
 GsApp          *gs_app_new                     (const gchar    *id);
 G_DEPRECATED_FOR(gs_app_set_from_unique_id)
 GsApp          *gs_app_new_from_unique_id      (const gchar    *unique_id);