pidgin/pidgin

Create and add notifications for account errors.

22 months ago, Gary Kramlich
86beaec0fa35
Parents 05ac40f0e0c4
Children 3745e6e9d836
Create and add notifications for account errors.

Adjust the NotificationManager API a bit to be easier to use and map better to
GListStore which the manager is now using internally.

Testing Done:
Ran the unit tests and forced a connection error and verified no issues (used gdb to verify that item was added).

Bugs closed: PIDGIN-17639

Reviewed at https://reviews.imfreedom.org/r/1509/
6 files changed, 87 insertions(+), 46 deletions(-)
  • +19 -1
    libpurple/account.c
  • +23 -0
    libpurple/purplenotification.c
  • +17 -0
    libpurple/purplenotification.h
  • +23 -30
    libpurple/purplenotificationmanager.c
  • +2 -5
    libpurple/purplenotificationmanager.h
  • +3 -10
    libpurple/tests/test_notification_manager.c
    • --- a/libpurple/account.c Tue Jun 28 16:04:47 2022 -0500
    +++ b/libpurple/account.c Thu Jun 30 20:24:09 2022 -0500
    @@ -33,6 +33,8 @@
    #include "purpleaccountpresence.h"
    #include "purpleconversationmanager.h"
    #include "purplecredentialmanager.h"
    +#include "purplenotification.h"
    +#include "purplenotificationmanager.h"
    #include "purpleprivate.h"
    #include "purpleprotocolclient.h"
    #include "purpleprotocolmanager.h"
    @@ -100,6 +102,7 @@
    void *registration_cb_user_data;
    PurpleConnectionErrorInfo *current_error; /* Errors */
    + PurpleNotification *error_notification;
    } PurpleAccountPrivate;
    typedef struct
    @@ -520,6 +523,7 @@
    {
    PurpleConnectionErrorInfo *old_err;
    PurpleAccountPrivate *priv;
    + PurpleNotificationManager *manager = NULL;
    g_return_if_fail(PURPLE_IS_ACCOUNT(account));
    priv = purple_account_get_instance_private(account);
    @@ -529,7 +533,19 @@
    if(new_err == old_err)
    return;
    - priv->current_error = new_err;
    + manager = purple_notification_manager_get_default();
    +
    + if(PURPLE_IS_NOTIFICATION(priv->error_notification)) {
    + purple_notification_manager_remove(manager, priv->error_notification);
    + g_clear_object(&priv->error_notification);
    + }
    +
    + if(new_err != NULL) {
    + priv->error_notification =
    + purple_notification_new(PURPLE_NOTIFICATION_TYPE_CONNECTION_ERROR,
    + account, new_err, NULL);
    + purple_notification_manager_add(manager, priv->error_notification);
    + }
    purple_signal_emit(purple_accounts_get_handle(),
    "account-error-changed",
    @@ -943,6 +959,8 @@
    g_free(priv->current_error);
    }
    + g_clear_object(&priv->error_notification);
    +
    g_free(priv->id);
    g_free(priv->username);
    g_free(priv->alias);
    --- a/libpurple/purplenotification.c Tue Jun 28 16:04:47 2022 -0500
    +++ b/libpurple/purplenotification.c Thu Jun 30 20:24:09 2022 -0500
    @@ -542,3 +542,26 @@
    return notification->data;
    }
    +
    +gint
    +purple_notification_compare(gconstpointer a, gconstpointer b) {
    + PurpleNotification *notification_a = NULL, *notification_b = NULL;
    +
    + if(a == NULL && b == NULL) {
    + return 0;
    + }
    +
    + if(a == NULL) {
    + return -1;
    + }
    +
    + if(b == NULL) {
    + return 1;
    + }
    +
    + notification_a = (PurpleNotification *)a;
    + notification_b = (PurpleNotification *)b;
    +
    + return g_date_time_compare(notification_a->created_timestamp,
    + notification_b->created_timestamp);
    +}
    --- a/libpurple/purplenotification.h Tue Jun 28 16:04:47 2022 -0500
    +++ b/libpurple/purplenotification.h Thu Jun 30 20:24:09 2022 -0500
    @@ -135,6 +135,8 @@
    * Timestamp is internally converted to UTC so you don't need to do that ahead
    * of time.
    *
    + * If @timestamp is %NULL, the current time will be used.
    + *
    * Since: 3.0.0
    */
    void purple_notification_set_created_timestamp(PurpleNotification *notification, GDateTime *timestamp);
    @@ -243,6 +245,21 @@
    */
    gpointer purple_notification_get_data(PurpleNotification *notification);
    +/**
    + * purple_notification_compare:
    + * @a: The first notification to compare.
    + * @b: The second notification to compare.
    + *
    + * A comparison function for PurpleNotification that is suitable as a
    + * GCompareFunc.
    + *
    + * Returns: -1 if @a's created timestamp occurred before @b, 0 if they were
    + * created at the same time, or 1 if @b was created before @a.
    + *
    + * Since: 3.0.0
    + */
    +gint purple_notification_compare(gconstpointer a, gconstpointer b);
    +
    G_END_DECLS
    #endif /* PURPLE_NOTIFICATION */
    --- a/libpurple/purplenotificationmanager.c Tue Jun 28 16:04:47 2022 -0500
    +++ b/libpurple/purplenotificationmanager.c Thu Jun 30 20:24:09 2022 -0500
    @@ -41,7 +41,7 @@
    struct _PurpleNotificationManager {
    GObject parent;
    - GHashTable *notifications;
    + GListStore *notifications;
    guint unread_count;
    };
    @@ -139,15 +139,14 @@
    manager = PURPLE_NOTIFICATION_MANAGER(obj);
    - g_clear_pointer(&manager->notifications, g_hash_table_destroy);
    + g_clear_object(&manager->notifications);
    G_OBJECT_CLASS(purple_notification_manager_parent_class)->finalize(obj);
    }
    static void
    purple_notification_manager_init(PurpleNotificationManager *manager) {
    - manager->notifications = g_hash_table_new_full(g_str_hash, g_str_equal,
    - NULL, g_object_unref);
    + manager->notifications = g_list_store_new(PURPLE_TYPE_NOTIFICATION);
    }
    static void
    @@ -288,20 +287,20 @@
    purple_notification_manager_add(PurpleNotificationManager *manager,
    PurpleNotification *notification)
    {
    - const gchar *id = NULL;
    -
    g_return_if_fail(PURPLE_IS_NOTIFICATION_MANAGER(manager));
    g_return_if_fail(PURPLE_IS_NOTIFICATION(notification));
    - id = purple_notification_get_id(notification);
    + if(g_list_store_find(manager->notifications, notification, NULL)) {
    + const gchar *id = purple_notification_get_id(notification);
    - if(g_hash_table_lookup(manager->notifications, (gpointer)id) != NULL) {
    g_warning("double add detected for notification %s", id);
    return;
    }
    - g_hash_table_insert(manager->notifications, (gpointer)id, notification);
    + g_list_store_insert_sorted(manager->notifications, notification,
    + (GCompareDataFunc)purple_notification_compare,
    + NULL);
    /* Connect to the notify signal for the read property only so we can
    * propagate out changes for any notification.
    @@ -319,49 +318,43 @@
    g_signal_emit(G_OBJECT(manager), signals[SIG_ADDED], 0, notification);
    }
    -gboolean
    +void
    purple_notification_manager_remove(PurpleNotificationManager *manager,
    - const gchar *id)
    + PurpleNotification *notification)
    {
    - gpointer data = NULL;
    - gboolean ret = FALSE;
    + guint position;
    - g_return_val_if_fail(PURPLE_IS_NOTIFICATION_MANAGER(manager), FALSE);
    - g_return_val_if_fail(id != NULL, FALSE);
    + g_return_if_fail(PURPLE_IS_NOTIFICATION_MANAGER(manager));
    + g_return_if_fail(PURPLE_IS_NOTIFICATION(notification));
    - data = g_hash_table_lookup(manager->notifications, id);
    - if(PURPLE_IS_NOTIFICATION(data)) {
    + if(g_list_store_find(manager->notifications, notification, &position)) {
    /* Reference the notification so we can emit the signal after it's been
    * removed from the hash table.
    */
    - g_object_ref(G_OBJECT(data));
    -
    - if(g_hash_table_remove(manager->notifications, id)) {
    - g_signal_emit(G_OBJECT(manager), signals[SIG_REMOVED], 0,
    - data);
    -
    - ret = TRUE;
    - }
    + g_object_ref(notification);
    /* Remove the notify signal handler for the read state incase someone
    * else added a reference to the notification which would then mess
    * with our unread count accounting.
    */
    - g_signal_handlers_disconnect_by_func(data,
    + g_signal_handlers_disconnect_by_func(notification,
    G_CALLBACK(purple_notification_manager_notify_cb),
    manager);
    /* If the notification is not read, we need to decrement the unread
    * count.
    */
    - if(!purple_notification_get_read(PURPLE_NOTIFICATION(data))) {
    + if(!purple_notification_get_read(notification)) {
    purple_notification_manager_decrement_unread_count(manager);
    }
    - g_object_unref(G_OBJECT(data));
    + g_list_store_remove(manager->notifications, position);
    +
    + g_signal_emit(G_OBJECT(manager), signals[SIG_REMOVED], 0, notification);
    +
    + g_object_unref(notification);
    +
    }
    -
    - return ret;
    }
    guint
    --- a/libpurple/purplenotificationmanager.h Tue Jun 28 16:04:47 2022 -0500
    +++ b/libpurple/purplenotificationmanager.h Thu Jun 30 20:24:09 2022 -0500
    @@ -69,16 +69,13 @@
    /**
    * purple_notification_manager_remove:
    * @manager: The instance.
    - * @id: The identifier of the notification to remove.
    + * @notification: The notification to remove.
    *
    * Removes @notification from @manager.
    *
    - * Returns: %TRUE if @notification was successfully removed from @manager,
    - * %FALSE otherwise.
    - *
    * Since: 3.0.0
    */
    -gboolean purple_notification_manager_remove(PurpleNotificationManager *manager, const gchar *id);
    +void purple_notification_manager_remove(PurpleNotificationManager *manager, PurpleNotification *notification);
    /**
    * purple_notification_manager_get_unread_count:
    --- a/libpurple/tests/test_notification_manager.c Tue Jun 28 16:04:47 2022 -0500
    +++ b/libpurple/tests/test_notification_manager.c Thu Jun 30 20:24:09 2022 -0500
    @@ -66,8 +66,6 @@
    PurpleNotificationManager *manager = NULL;
    PurpleNotification *notification = NULL;
    gint added_called = 0, removed_called = 0;
    - gboolean removed = FALSE;
    - const gchar *id = NULL;
    guint unread_count = 0;
    manager = g_object_new(PURPLE_TYPE_NOTIFICATION_MANAGER, NULL);
    @@ -85,7 +83,6 @@
    /* Create the notification and store it's id. */
    notification = purple_notification_new(PURPLE_NOTIFICATION_TYPE_GENERIC,
    NULL, NULL, NULL);
    - id = purple_notification_get_id(notification);
    /* Add the notification to the manager. */
    purple_notification_manager_add(manager, notification);
    @@ -98,8 +95,7 @@
    g_assert_cmpint(unread_count, ==, 1);
    /* Remove the notification. */
    - removed = purple_notification_manager_remove(manager, id);
    - g_assert_true(removed);
    + purple_notification_manager_remove(manager, notification);
    g_assert_cmpint(removed_called, ==, 1);
    /* Verify that the unread count is now 0. */
    @@ -139,7 +135,6 @@
    test_purple_notification_manager_double_remove(void) {
    PurpleNotificationManager *manager = NULL;
    PurpleNotification *notification = NULL;
    - const gchar *id = NULL;
    gint removed_called = 0;
    manager = g_object_new(PURPLE_TYPE_NOTIFICATION_MANAGER, NULL);
    @@ -155,12 +150,10 @@
    */
    g_object_ref(notification);
    - id = purple_notification_get_id(notification);
    -
    purple_notification_manager_add(manager, notification);
    - g_assert_true(purple_notification_manager_remove(manager, id));
    - g_assert_false(purple_notification_manager_remove(manager, id));
    + purple_notification_manager_remove(manager, notification);
    + purple_notification_manager_remove(manager, notification);
    g_assert_cmpint(removed_called, ==, 1);