[geary/mjog/account-command-stacks: 74/77] Improve application shutdown process



commit b34b6db14b5047ee674702ca9c63d9f91a8a1fad
Author: Michael Gratton <mike vee net>
Date:   Tue Nov 5 08:26:45 2019 +1100

    Improve application shutdown process
    
    Ensure accounts being removed are de-selected and the cursor moved off
    of their folders in the folder list so that as they are removed, their
    folders are not selected in quick succession. Don't redudantly close
    inboxes in Application.Controller::close_async, and merge the two parts
    to the handling of account closing.

 src/client/application/application-controller.vala | 130 ++++++++++-----------
 src/client/components/main-window.vala             |  39 +++++--
 src/client/folder-list/folder-list-tree.vala       |  36 +++---
 3 files changed, 109 insertions(+), 96 deletions(-)
---
diff --git a/src/client/application/application-controller.vala 
b/src/client/application/application-controller.vala
index d1accf8f..c17066e5 100644
--- a/src/client/application/application-controller.vala
+++ b/src/client/application/application-controller.vala
@@ -337,36 +337,16 @@ public class Application.Controller : Geary.BaseObject {
         // explode if accounts are removed while iterating.
         AccountContext[] accounts = this.accounts.values.to_array();
 
-        // Close all inboxes. Launch these in parallel first so we're
-        // not wasting time waiting for each one to close. The account
-        // will wait around for them to actually close.
-        foreach (AccountContext context in accounts) {
-            Geary.Folder? inbox = context.inbox;
-            if (inbox != null) {
-                debug("Closing inbox: %s...", inbox.to_string());
-                inbox.close_async.begin(null, (obj, ret) => {
-                        try {
-                            inbox.close_async.end(ret);
-                        } catch (Error err) {
-                            debug(
-                                "Error closing Inbox %s at shutdown: %s",
-                                inbox.to_string(), err.message
-                            );
-                        }
-                    });
-                context.inbox = null;
-            }
-        }
-
-        // Close all Accounts. Again, this is done in parallel to
-        // minimise time taken to close, but here use a barrier to
-        // wait for all to actually finish closing.
+        // Close all Accounts. Launch these in parallel to minimise
+        // time taken to close, but here use a barrier to wait for all
+        // to actually finish closing.
         Geary.Nonblocking.CountingSemaphore close_barrier =
             new Geary.Nonblocking.CountingSemaphore(null);
         foreach (AccountContext context in accounts) {
             close_barrier.acquire();
             this.close_account.begin(
                 context.account.information,
+                true,
                 (obj, ret) => {
                     this.close_account.end(ret);
                     close_barrier.blind_notify();
@@ -893,13 +873,15 @@ public class Application.Controller : Geary.BaseObject {
         connect_account_async.begin(account, cancellable_open_account);
     }
 
-    private async void close_account(Geary.AccountInformation config) {
+    private async void close_account(Geary.AccountInformation config,
+                                     bool is_shutdown) {
         AccountContext? context = this.accounts.get(config);
         if (context != null) {
+            debug("Closing account: %s", context.account.information.id);
             Geary.Account account = context.account;
-            if (this.main_window.selected_account == account) {
-                this.main_window.deselect_account();
-            }
+
+            // Guard against trying to close the account twice
+            this.accounts.unset(account.information);
 
             // Stop updating status and showing errors when closing
             // the account - the user doesn't care any more
@@ -912,7 +894,53 @@ public class Application.Controller : Geary.BaseObject {
                 on_account_status_notify
             );
 
-            yield disconnect_account_async(context);
+            account.email_sent.disconnect(on_sent);
+            account.email_removed.disconnect(on_account_email_removed);
+            account.folders_available_unavailable.disconnect(on_folders_available_unavailable);
+            account.sending_monitor.start.disconnect(on_sending_started);
+            account.sending_monitor.finish.disconnect(on_sending_finished);
+
+            // Now the account is not in the accounts map, reset any
+            // status notifications for it
+            update_account_status();
+
+            // If we're not shutting down, select the inbox of the
+            // first account so that we show something other than
+            // empty conversation list/viewer.
+            Geary.Folder? to_select = null;
+            if (!is_shutdown) {
+                Geary.AccountInformation? first_account = get_first_account();
+                if (first_account != null) {
+                    AccountContext? first_context = this.accounts[first_account];
+                    if (first_context != null) {
+                        to_select = first_context.inbox;
+                    }
+                }
+            }
+
+            yield this.main_window.remove_account(account, to_select);
+
+            context.cancellable.cancel();
+            context.contacts.close();
+
+            // Explicitly close the inbox since we explicitly open it
+            Geary.Folder? inbox = context.inbox;
+            if (inbox != null) {
+                try {
+                    yield inbox.close_async(null);
+                } catch (Error close_inbox_err) {
+                    debug("Unable to close monitored inbox: %s", close_inbox_err.message);
+                }
+                context.inbox = null;
+            }
+
+            try {
+                yield account.close_async(null);
+            } catch (Error close_err) {
+                debug("Unable to close account %s: %s", account.to_string(), close_err.message);
+            }
+
+            debug("Account closed: %s", account.to_string());
         }
     }
 
@@ -1203,48 +1231,6 @@ public class Application.Controller : Geary.BaseObject {
         return retry;
     }
 
-    private async void disconnect_account_async(AccountContext context, Cancellable? cancellable = null) {
-        debug("Disconnecting account: %s", context.account.information.id);
-
-        Geary.Account account = context.account;
-
-        // Guard against trying to disconnect the account twice
-        this.accounts.unset(account.information);
-
-        // Now the account is not in the accounts map, reset any
-        // status notifications for it
-        update_account_status();
-
-        account.email_sent.disconnect(on_sent);
-        account.email_removed.disconnect(on_account_email_removed);
-        account.folders_available_unavailable.disconnect(on_folders_available_unavailable);
-        account.sending_monitor.start.disconnect(on_sending_started);
-        account.sending_monitor.finish.disconnect(on_sending_finished);
-
-        main_window.folder_list.remove_account(account);
-
-        context.cancellable.cancel();
-        context.contacts.close();
-
-        Geary.Folder? inbox = context.inbox;
-        if (inbox != null) {
-            try {
-                yield inbox.close_async(cancellable);
-            } catch (Error close_inbox_err) {
-                debug("Unable to close monitored inbox: %s", close_inbox_err.message);
-            }
-            context.inbox = null;
-        }
-
-        try {
-            yield account.close_async(cancellable);
-        } catch (Error close_err) {
-            debug("Unable to close account %s: %s", account.to_string(), close_err.message);
-        }
-
-        debug("Account closed: %s", account.to_string());
-    }
-
     /**
      * Returns true if we've attempted to open all accounts at this point.
      */
@@ -1753,6 +1739,7 @@ public class Application.Controller : Geary.BaseObject {
             if (this.application.engine.has_account(changed.id)) {
                 this.close_account.begin(
                     changed,
+                    false,
                     (obj, res) => {
                         this.close_account.end(res);
                         try {
@@ -1773,6 +1760,7 @@ public class Application.Controller : Geary.BaseObject {
         debug("%s: Closing account for removal", removed.id);
         this.close_account.begin(
             removed,
+            false,
             (obj, res) => {
                 this.close_account.end(res);
                 debug("%s: Account closed", removed.id);
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index 6d1d3a24..51a963ca 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -492,14 +492,6 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
         this.info_bar_frame.show();
     }
 
-    /** Deselected the currently selected account, if any. */
-    public void deselect_account() {
-        // XXX do other things like select the first/next most highest
-        // account's inbox?
-        this.search_bar.set_search_text(""); // Reset search.
-        this.select_folder.begin(null, false);
-    }
-
     /** Displays a composer addressed to a specific email address. */
     public void open_composer_for_mailbox(Geary.RFC822.MailboxAddress to) {
         Application.Controller controller = this.application.controller;
@@ -595,6 +587,37 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface {
         this.folder_list.remove_folder(to_remove);
     }
 
+    /**
+     * Removes the given account from the main window.
+     *
+     * If `to_select` is not null, the given folder will be selected,
+     * otherwise no folder will be.
+     */
+    public async void remove_account(Geary.Account to_remove,
+                                     Geary.Folder? to_select) {
+        // Explicitly unset the selected folder if it belongs to the
+        // account so we block until it's gone. This also clears the
+        // previous search folder, so it won't try to re-load that
+        // that when the account is gone.
+        if (this.selected_folder != null &&
+            this.selected_folder.account == to_remove) {
+            Geary.SearchFolder? current_search = (
+                this.selected_folder as Geary.SearchFolder
+            );
+
+            yield select_folder(to_select, false);
+
+            // Clear the account's search folder if it existed
+            if (current_search != null) {
+                this.search_bar.set_search_text("");
+                this.search_bar.search_mode_enabled = false;
+            }
+        }
+
+        // Finally, remove the account and its folders
+        this.folder_list.remove_account(to_remove);
+    }
+
     private void load_config(Configuration config) {
         // This code both loads AND saves the pane positions with live updating. This is more
         // resilient against crashes because the value in dconf changes *immediately*, and
diff --git a/src/client/folder-list/folder-list-tree.vala b/src/client/folder-list/folder-list-tree.vala
index f919caf6..80b31c81 100644
--- a/src/client/folder-list/folder-list-tree.vala
+++ b/src/client/folder-list/folder-list-tree.vala
@@ -136,8 +136,7 @@ public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface {
 
         // if found and selected, report nothing is selected in preparation for its removal
         if (entry != null && is_selected(entry)) {
-            this.selected = null;
-            folder_selected(null);
+            deselect_folder();
         }
 
         // if Inbox, remove from inboxes branch, selected or not
@@ -149,28 +148,20 @@ public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface {
 
     public void remove_account(Geary.Account account) {
         account.information.notify["ordinal"].disconnect(on_ordinal_changed);
+
+        // If a folder on this account is selected, unselect it.
+        if (this.selected != null &&
+            this.selected.account == account) {
+            deselect_folder();
+        }
+
         AccountBranch? account_branch = account_branches.get(account);
         if (account_branch != null) {
-            // If a folder on this account is selected, unselect it.
-            foreach (FolderEntry entry in account_branch.folder_entries.values) {
-                if (is_selected(entry)) {
-                    this.selected = null;
-                    folder_selected(null);
-                    break;
-                }
-            }
-
             if (has_branch(account_branch))
                 prune(account_branch);
             account_branches.unset(account);
         }
 
-        Sidebar.Entry? entry = inboxes_branch.get_entry_for_account(account);
-        if (entry != null && is_selected(entry)) {
-            this.selected = null;
-            folder_selected(null);
-        }
-
         inboxes_branch.remove_inbox(account);
 
         if (account_branches.size <= 1 && has_branch(inboxes_branch))
@@ -206,7 +197,18 @@ public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface {
     }
 
     public void deselect_folder() {
+        Gtk.TreeModel model = get_model();
+        Gtk.TreeIter iter;
+        if (model.get_iter_first(out iter)) {
+            Gtk.TreePath? first = model.get_path(iter);
+            if (first != null) {
+                set_cursor(first, null, false);
+            }
+        }
+
         get_selection().unselect_all();
+        this.selected = null;
+        folder_selected(null);
     }
 
     public override bool drag_motion(Gdk.DragContext context, int x, int y, uint time) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]