Fix account preferences widget leaks

3 files changed, 123 insertions, 103 deletions

diff --git a/main/src/windows/preferences_window/account_preferences_subpage.vala b/main/src/windows/preferences_window/account_preferences_subpage.vala
index 660a9aeb..cc1e7b5e 100644
--- a/main/src/windows/preferences_window/account_preferences_subpage.vala
+++ b/main/src/windows/preferences_window/account_preferences_subpage.vala
@@ -67,64 +67,74 @@ public class Dino.Ui.AccountPreferencesSubpage : Adw.NavigationPage {
             dialog.present();
         });
 
-        this.notify["model"].connect(() => {
-            model.notify["selected-account"].connect(() => {
-                foreach (var binding in bindings) {
-                    binding.unbind();
-                }
+        this.notify["model"].connect(on_model_changed);
+    }
 
-                avatar.model = model.selected_account.avatar_model;
-                xmpp_address.subtitle = account.bare_jid.to_string();
+    private void on_model_changed() {
+        model.notify["selected-account"].connect(refresh);
+        refresh();
+    }
 
-                if (alias_entry_changed != 0) local_alias.disconnect(alias_entry_changed);
-                local_alias.text = account.alias ?? "";
-                alias_entry_changed = local_alias.changed.connect(() => {
-                    account.alias = local_alias.text;
-                });
+    private static bool account_enabled_to_disable_button_label(Binding binding, Value from_value, ref Value to_value) {
+        bool enabled_bool = (bool) from_value;
+        to_value = enabled_bool ? _("Disable account") : _("Enable account");
+        return true;
+    }
 
-                bindings += account.bind_property("enabled", disable_account_button, "label", BindingFlags.SYNC_CREATE, (binding, from, ref to) => {
-                    bool enabled_bool = (bool) from;
-                    to = enabled_bool ? _("Disable account") : _("Enable account");
-                    return true;
-                });
-                bindings += account.bind_property("enabled", avatar_menu_box, "visible", BindingFlags.SYNC_CREATE);
-                bindings += account.bind_property("enabled", password_change, "visible", BindingFlags.SYNC_CREATE);
-                bindings += account.bind_property("enabled", connection_status, "visible", BindingFlags.SYNC_CREATE);
-                bindings += model.selected_account.bind_property("connection-state", connection_status, "subtitle", BindingFlags.SYNC_CREATE, (binding, from, ref to) => {
-                    to = get_status_label();
-                    return true;
-                });
-                bindings += model.selected_account.bind_property("connection-error", connection_status, "subtitle", BindingFlags.SYNC_CREATE, (binding, from, ref to) => {
-                    to = get_status_label();
-                    return true;
-                });
-                bindings += model.selected_account.bind_property("connection-error", enter_password_button, "visible", BindingFlags.SYNC_CREATE, (binding, from, ref to) => {
-                    var error = (ConnectionManager.ConnectionError) from;
-                    to = error != null && error.source == ConnectionManager.ConnectionError.Source.SASL;
-                    return true;
-                });
+    private static bool connection_status_to_label(Binding binding, Value from_value, ref Value to_value) {
+        var account = binding.source as ViewModel.AccountDetails;
+        to_value = get_status_label(account);
+        return true;
+    }
 
-                // Only show avatar removal button if an avatar is set
-                var avatar_model = model.selected_account.avatar_model.tiles.get_item(0) as ViewModel.AvatarPictureTileModel;
-                avatar_model.notify["image-file"].connect(() => {
-                    remove_avatar_button.visible = avatar_model.image_file != null;
-                });
-                remove_avatar_button.visible = avatar_model.image_file != null;
+    private static bool connection_error_to_enter_password_visible(Binding binding, Value from_value, ref Value to_value) {
+        var error = (ConnectionManager.ConnectionError) from_value;
+        to_value = error != null && error.source == ConnectionManager.ConnectionError.Source.SASL;
+        return true;
+    }
 
-                model.selected_account.notify["connection-error"].connect(() => {
-                    if (model.selected_account.connection_error != null) {
-                        connection_status.add_css_class("error");
-                    } else {
-                        connection_status.remove_css_class("error");
-                    }
-                });
-                if (model.selected_account.connection_error != null) {
-                    connection_status.add_css_class("error");
-                } else {
-                    connection_status.remove_css_class("error");
-                }
-            });
+    private static bool image_file_present_to_remove_button_visible(Binding binding, Value from_value, ref Value to_value) {
+        File? image_file = (File) from_value;
+        to_value = image_file != null;
+        return true;
+    }
+
+    private void on_connection_error_updated() {
+        if (model.selected_account.connection_error != null) {
+            connection_status.add_css_class("error");
+        } else {
+            connection_status.remove_css_class("error");
+        }
+    }
+
+    private void refresh() {
+        foreach (var binding in bindings) {
+            binding.unbind();
+        }
+
+        avatar.model = model.selected_account.avatar_model;
+        xmpp_address.subtitle = account.bare_jid.to_string();
+
+        if (alias_entry_changed != 0) local_alias.disconnect(alias_entry_changed);
+        local_alias.text = account.alias ?? "";
+        alias_entry_changed = local_alias.changed.connect(() => {
+            account.alias = local_alias.text;
         });
+
+        bindings += account.bind_property("enabled", disable_account_button, "label", BindingFlags.SYNC_CREATE, account_enabled_to_disable_button_label);
+        bindings += account.bind_property("enabled", avatar_menu_box, "visible", BindingFlags.SYNC_CREATE);
+        bindings += account.bind_property("enabled", password_change, "visible", BindingFlags.SYNC_CREATE);
+        bindings += account.bind_property("enabled", connection_status, "visible", BindingFlags.SYNC_CREATE);
+        bindings += model.selected_account.bind_property("connection-state", connection_status, "subtitle", BindingFlags.SYNC_CREATE, connection_status_to_label);
+        bindings += model.selected_account.bind_property("connection-error", connection_status, "subtitle", BindingFlags.SYNC_CREATE, connection_status_to_label);
+        bindings += model.selected_account.bind_property("connection-error", enter_password_button, "visible", BindingFlags.SYNC_CREATE, connection_error_to_enter_password_visible);
+
+        // Only show avatar removal button if an avatar is set
+        var avatar_model = model.selected_account.avatar_model.tiles.get_item(0) as ViewModel.AvatarPictureTileModel;
+        bindings += avatar_model.bind_property("image-file", remove_avatar_button, "visible", BindingFlags.SYNC_CREATE, image_file_present_to_remove_button_visible);
+
+        model.selected_account.notify["connection-error"].connect(on_connection_error_updated);
+        on_connection_error_updated();
     }
 
     private void show_select_avatar() {
@@ -177,36 +187,31 @@ public class Dino.Ui.AccountPreferencesSubpage : Adw.NavigationPage {
         dialog.present((Window)this.get_root());
     }
 
-    private string get_status_label() {
-        string? error_label = get_connection_error_description();
-        if (error_label != null) return error_label;
-
-        ConnectionManager.ConnectionState state = model.selected_account.connection_state;
-        switch (state) {
-            case ConnectionManager.ConnectionState.CONNECTING:
-                return _("Connecting…");
-            case ConnectionManager.ConnectionState.CONNECTED:
-                return _("Connected");
-            case ConnectionManager.ConnectionState.DISCONNECTED:
-                return _("Disconnected");
-        }
-        assert_not_reached();
-    }
-
-    private string? get_connection_error_description() {
-        ConnectionManager.ConnectionError? error = model.selected_account.connection_error;
-        if (error == null) return null;
-
-        switch (error.source) {
-            case ConnectionManager.ConnectionError.Source.SASL:
-                return _("Wrong password");
-            case ConnectionManager.ConnectionError.Source.TLS:
-                return _("Invalid TLS certificate");
-        }
-        if (error.identifier != null) {
-            return _("Error") + ": " + error.identifier;
+    private static string get_status_label(ViewModel.AccountDetails account) {
+        ConnectionManager.ConnectionError? error = account.connection_error;
+        if (error != null) {
+            switch (error.source) {
+                case ConnectionManager.ConnectionError.Source.SASL:
+                    return _("Wrong password");
+                case ConnectionManager.ConnectionError.Source.TLS:
+                    return _("Invalid TLS certificate");
+            }
+            if (error.identifier != null) {
+                return _("Error") + ": " + error.identifier;
+            } else {
+                return _("Error");
+            }
         } else {
-            return _("Error");
+            ConnectionManager.ConnectionState state = account.connection_state;
+            switch (state) {
+                case ConnectionManager.ConnectionState.CONNECTING:
+                    return _("Connecting…");
+                case ConnectionManager.ConnectionState.CONNECTED:
+                    return _("Connected");
+                case ConnectionManager.ConnectionState.DISCONNECTED:
+                    return _("Disconnected");
+            }
+            assert_not_reached();
         }
     }
 }
diff --git a/main/src/windows/preferences_window/accounts_preferences_page.vala b/main/src/windows/preferences_window/accounts_preferences_page.vala
index 7280c25e..5703d430 100644
--- a/main/src/windows/preferences_window/accounts_preferences_page.vala
+++ b/main/src/windows/preferences_window/accounts_preferences_page.vala
@@ -4,10 +4,10 @@ using Gtk;
 
 public class Dino.Ui.PreferencesWindowAccounts : Adw.PreferencesPage {
 
-    public signal void account_chosen(Account account);
+    private const string ACCOUNT_ROW_ACCOUNT = "DINO_ACCOUNT_ROW_ACCOUNT";
 
-    public Adw.PreferencesGroup active_accounts;
-    public Adw.PreferencesGroup disabled_accounts;
+    public signal void account_chosen(Account account);
+    private bool refreshed_once = false;
 
     public ViewModel.PreferencesDialog model { get; set; }
 
@@ -15,17 +15,39 @@ public class Dino.Ui.PreferencesWindowAccounts : Adw.PreferencesPage {
         this.title = _("Accounts");
         this.icon_name = "dino-system-users-symbolic";
 
-        this.notify["model"].connect(() => {
-            model.update.connect(refresh);
-        });
+        this.notify["model"].connect(on_model_changed);
+
+        check_widget_leak(this);
+    }
+
+    private void on_model_changed() {
+        model.update.connect(refresh);
+    }
+
+    private void on_add_account_clicked() {
+        Ui.ManageAccounts.AddAccountDialog add_account_dialog = new Ui.ManageAccounts.AddAccountDialog(model.stream_interactor, model.db);
+        add_account_dialog.added.connect(refresh);
+        add_account_dialog.present((Window)this.get_root());
+    }
+
+    private static void on_account_row_activated(Adw.ActionRow row) {
+        Widget widget = row;
+        while (widget != null && !(widget is PreferencesWindowAccounts)) {
+            widget = widget.parent;
+        }
+        if (widget is PreferencesWindowAccounts) {
+            ((PreferencesWindowAccounts) widget).account_chosen(row.get_data(ACCOUNT_ROW_ACCOUNT));
+        }
     }
 
     private void refresh() {
-        if (active_accounts != null) this.remove(active_accounts);
-        if (disabled_accounts != null) this.remove(disabled_accounts);
+        if (refreshed_once) {
+            this.remove(this.get_group(1));
+            this.remove(this.get_group(0));
+        }
 
-        active_accounts = new Adw.PreferencesGroup() { title=_("Accounts")};
-        disabled_accounts = new Adw.PreferencesGroup() { title=_("Disabled accounts")};
+        Adw.PreferencesGroup active_accounts = new Adw.PreferencesGroup() { title=_("Accounts")};
+        Adw.PreferencesGroup disabled_accounts = new Adw.PreferencesGroup() { title=_("Disabled accounts")};
         Button add_account_button = new Button.from_icon_name("dino-list-add-symbolic");
         add_account_button.add_css_class("flat");
         add_account_button.tooltip_text = _("Add Account");
@@ -34,13 +56,7 @@ public class Dino.Ui.PreferencesWindowAccounts : Adw.PreferencesPage {
         this.add(active_accounts);
         this.add(disabled_accounts);
 
-        add_account_button.clicked.connect(() => {
-            Ui.ManageAccounts.AddAccountDialog add_account_dialog = new Ui.ManageAccounts.AddAccountDialog(model.stream_interactor, model.db);
-            add_account_dialog.added.connect((account) => {
-                refresh();
-            });
-            add_account_dialog.present((Window)this.get_root());
-        });
+        add_account_button.clicked.connect(on_add_account_clicked);
 
         disabled_accounts.visible = false; // Only display disabled section if it contains accounts
         var enabled_account_added = false;
@@ -61,9 +77,8 @@ public class Dino.Ui.PreferencesWindowAccounts : Adw.PreferencesPage {
                 disabled_accounts.visible = true;
             }
 
-            row.activated.connect(() => {
-                account_chosen(account_details.account);
-            });
+            row.set_data(ACCOUNT_ROW_ACCOUNT, account_details.account);
+            row.activated.connect(on_account_row_activated);
         }
 
         // We always have to show the active accounts group for the add new button. Display placeholder if there are no active accounts
diff --git a/main/src/windows/preferences_window/preferences_dialog.vala b/main/src/windows/preferences_window/preferences_dialog.vala
index 7a70ca00..4baced96 100644
--- a/main/src/windows/preferences_window/preferences_dialog.vala
+++ b/main/src/windows/preferences_window/preferences_dialog.vala
@@ -10,17 +10,17 @@ public class Dino.Ui.PreferencesDialog : Adw.PreferencesDialog {
     [GtkChild] public unowned Dino.Ui.PreferencesWindowAccounts accounts_page;
     [GtkChild] public unowned Dino.Ui.PreferencesWindowEncryption encryption_page;
     [GtkChild] public unowned Dino.Ui.GeneralPreferencesPage general_page;
-    public Dino.Ui.AccountPreferencesSubpage account_page = new Dino.Ui.AccountPreferencesSubpage();
 
     [GtkChild] public unowned ViewModel.PreferencesDialog model { get; }
 
     construct {
         this.bind_property("model", accounts_page, "model", BindingFlags.SYNC_CREATE);
-        this.bind_property("model", account_page, "model", BindingFlags.SYNC_CREATE);
         this.bind_property("model", encryption_page, "model", BindingFlags.SYNC_CREATE);
 
         accounts_page.account_chosen.connect((account) => {
             model.selected_account = model.account_details[account];
+            Dino.Ui.AccountPreferencesSubpage account_page = new Dino.Ui.AccountPreferencesSubpage();
+            this.bind_property("model", account_page, "model", BindingFlags.SYNC_CREATE);
             this.push_subpage(account_page);
         });
     }