pidgin/pidgin

Stop automatically adding/remove conversations from the conversation manager

4 weeks ago, Gary Kramlich
8019945e1c21
Parents 584895b844d2
Children db971edd4b59
Stop automatically adding/remove conversations from the conversation manager

This was done to mimic the old conversations API and shouldn't be needed going
forward. However, since we can't create conversations right now, we're probably
missing some edge cases here.

Also make sure to keep a reference around on the conversation while removing to avoid use after frees.

Testing Done:
Trained with the turtles.

Reviewed at https://reviews.imfreedom.org/r/3049/
3 files changed, 5 insertions(+), 22 deletions(-)
  • +0 -12
    libpurple/purpleconversation.c
  • +5 -0
    libpurple/purpleconversationmanager.c
  • +0 -10
    libpurple/tests/test_conversation.c
    • --- a/libpurple/purpleconversation.c Mon Mar 25 22:25:49 2024 -0500
    +++ b/libpurple/purpleconversation.c Mon Mar 25 22:56:20 2024 -0500
    @@ -573,7 +573,6 @@
    PurpleConversation *conv = PURPLE_CONVERSATION(object);
    PurpleAccount *account;
    PurpleConnection *gc;
    - PurpleConversationManager *manager;
    PurpleConversationUiOps *ops;
    G_OBJECT_CLASS(purple_conversation_parent_class)->constructed(object);
    @@ -589,10 +588,6 @@
    purple_connection_get_flags(gc));
    }
    - /* add the conversation to the appropriate lists */
    - manager = purple_conversation_manager_get_default();
    - purple_conversation_manager_register(manager, conv);
    -
    /* Auto-set the title. */
    purple_conversation_autoset_title(conv);
    @@ -620,19 +615,12 @@
    static void
    purple_conversation_finalize(GObject *object) {
    PurpleConversation *conv = PURPLE_CONVERSATION(object);
    - PurpleConversationManager *manager;
    PurpleConversationPrivate *priv =
    purple_conversation_get_instance_private(conv);
    PurpleConversationUiOps *ops = purple_conversation_get_ui_ops(conv);
    purple_request_close_with_handle(conv);
    - /* remove from conversations and im/chats lists prior to emit */
    - manager = purple_conversation_manager_get_default();
    - if(PURPLE_IS_CONVERSATION_MANAGER(manager)) {
    - purple_conversation_manager_unregister(manager, conv);
    - }
    -
    purple_signal_emit(purple_conversations_get_handle(),
    "deleting-conversation", conv);
    --- a/libpurple/purpleconversationmanager.c Mon Mar 25 22:25:49 2024 -0500
    +++ b/libpurple/purpleconversationmanager.c Mon Mar 25 22:56:20 2024 -0500
    @@ -305,6 +305,9 @@
    g_return_val_if_fail(PURPLE_IS_CONVERSATION_MANAGER(manager), FALSE);
    g_return_val_if_fail(PURPLE_IS_CONVERSATION(conversation), FALSE);
    + /* Make sure we have a reference in case we need to emit signals. */
    + g_object_ref(conversation);
    +
    unregistered = g_hash_table_remove(manager->conversations, conversation);
    if(unregistered) {
    /* Disconnect all the signals we added for propagation. */
    @@ -316,6 +319,8 @@
    g_signal_emit(manager, signals[SIG_UNREGISTERED], 0, conversation);
    }
    + g_object_unref(conversation);
    +
    return unregistered;
    }
    --- a/libpurple/tests/test_conversation.c Mon Mar 25 22:25:49 2024 -0500
    +++ b/libpurple/tests/test_conversation.c Mon Mar 25 22:56:20 2024 -0500
    @@ -37,7 +37,6 @@
    PurpleContactInfo *topic_author = NULL;
    PurpleContactInfo *topic_author1 = NULL;
    PurpleConversation *conversation = NULL;
    - PurpleConversationManager *conversation_manager = NULL;
    PurpleConversationType type = PURPLE_CONVERSATION_TYPE_UNSET;
    PurpleTags *tags = NULL;
    GDateTime *created_on = NULL;
    @@ -52,7 +51,6 @@
    char *user_nickname = NULL;
    gboolean age_restricted = FALSE;
    gboolean favorite = FALSE;
    - gboolean unregistered = FALSE;
    account = purple_account_new("test", "test");
    avatar = g_object_new(PURPLE_TYPE_AVATAR, NULL);
    @@ -158,14 +156,6 @@
    g_assert_cmpstr(user_nickname, ==, "knick-knack");
    g_clear_pointer(&user_nickname, g_free);
    - /* TODO: Conversations are automatically registered on construction for
    - * legacy reasons, so we need to explicitly unregister to clean them up,
    - * but this can go away once that stops happening. */
    - conversation_manager = purple_conversation_manager_get_default();
    - unregistered = purple_conversation_manager_unregister(conversation_manager,
    - conversation);
    - g_assert_true(unregistered);
    -
    g_clear_object(&avatar);
    g_clear_object(&topic_author);
    g_clear_pointer(&topic_updated, g_date_time_unref);