diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index 2953d5bc..6218f56a 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -66,7 +66,7 @@ public class GearyController : Geary.BaseObject { private const string MOVE_MESSAGE_TOOLTIP_SINGLE = _("Move conversation"); private const string MOVE_MESSAGE_TOOLTIP_MULTIPLE = _("Move conversations"); - private const int SELECT_FOLDER_TIMEOUT_MSEC = 100; + private const int SELECT_FOLDER_TIMEOUT_USEC = 100 * 1000; private const int SEARCH_TIMEOUT_MSEC = 100; private const string PROP_ATTEMPT_OPEN_ACCOUNT = "attempt-open-account"; @@ -96,6 +96,7 @@ public class GearyController : Geary.BaseObject { private UnityLauncher? unity_launcher = null; private Libnotify? libnotify = null; private uint select_folder_timeout_id = 0; + private int64 next_folder_select_allowed_usec = 0; private Geary.Folder? folder_to_select = null; private Geary.Nonblocking.Mutex select_folder_mutex = new Geary.Nonblocking.Mutex(); private Geary.Account? account_to_select = null; @@ -945,24 +946,35 @@ public class GearyController : Geary.BaseObject { return; } - // To prevent the user from selecting folders too quickly, we actually - // schedule the action to happen after a timeout instead of acting - // directly. If the user selects another folder during the timeout, - // we nix the original timeout and start a new one. - if (select_folder_timeout_id != 0) - Source.remove(select_folder_timeout_id); folder_to_select = folder; - select_folder_timeout_id = Timeout.add(SELECT_FOLDER_TIMEOUT_MSEC, on_select_folder_timeout); + + // To prevent the user from selecting folders too quickly, we prevent additional selection + // changes to occur until after a timeout has expired from the last one + int64 now = get_monotonic_time(); + int64 diff = now - next_folder_select_allowed_usec; + if (diff < SELECT_FOLDER_TIMEOUT_USEC) { + // only start timeout if another timeout is not running ... this means the user can + // click madly and will see the last clicked-on folder 100ms after the first one was + // clicked on + if (select_folder_timeout_id == 0) + select_folder_timeout_id = Timeout.add((uint) (diff / 1000), on_select_folder_timeout); + } else { + do_select_folder.begin(folder_to_select, on_select_folder_completed); + folder_to_select = null; + + next_folder_select_allowed_usec = now + SELECT_FOLDER_TIMEOUT_USEC; + } } private bool on_select_folder_timeout() { - assert(folder_to_select != null); - select_folder_timeout_id = 0; + next_folder_select_allowed_usec = 0; - do_select_folder.begin(folder_to_select, on_select_folder_completed); + if (folder_to_select != null) + do_select_folder.begin(folder_to_select, on_select_folder_completed); folder_to_select = null; + return false; } @@ -970,6 +982,8 @@ public class GearyController : Geary.BaseObject { if (folder == current_folder) return; + debug("Switching to %s...", folder.to_string()); + cancel_folder(); // This function is not reentrant. It should be, because it can be @@ -992,9 +1006,6 @@ public class GearyController : Geary.BaseObject { yield current_folder.close_async(); } - if (folder != null) - debug("switching to %s", folder.to_string()); - current_folder = folder; if (current_account != folder.account) { current_account = folder.account; @@ -1026,7 +1037,7 @@ public class GearyController : Geary.BaseObject { update_ui(); - current_conversations = new Geary.App.ConversationMonitor(current_folder, Geary.Folder.OpenFlags.NONE, + current_conversations = new Geary.App.ConversationMonitor(current_folder, Geary.Folder.OpenFlags.NO_DELAY, ConversationListStore.REQUIRED_FIELDS, MIN_CONVERSATION_COUNT); if (inboxes.values.contains(current_folder)) { @@ -1045,6 +1056,8 @@ public class GearyController : Geary.BaseObject { yield current_conversations.start_monitoring_async(conversation_cancellable); select_folder_mutex.release(ref mutex_token); + + debug("Switched to %s", folder.to_string()); } private void on_scan_error(Error err) { diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index b7ad434b..a8708961 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -67,7 +67,13 @@ public interface Geary.Folder : BaseObject { * the messages (such as their flags or other metadata) may not be up-to-date * when the folder opens. Not all folders will support this flag. */ - FAST_OPEN; + FAST_OPEN, + /** + * Do not delay opening a connection to the server. + * + * @see open_async + */ + NO_DELAY; public bool is_any_set(OpenFlags flags) { return (this & flags) != 0; @@ -321,6 +327,12 @@ public interface Geary.Folder : BaseObject { * while it may take time (if ever) for the remote state to open. Thus, it's possible for * the "opened" signal to fire some time *after* this method completes. * + * {@link OpenFlags.NO_DELAY} may be passed to force an immediate opening of the remote folder. + * This still will not occur in the context of the open_async call, but will initiate the + * connection immediately. Use this only when it's known that remote calls or remote + * notifications to the Folder are imminent or monitoring the Folder is vital (such as with the + * Inbox). + * * However, even if the method returns before the Folder's OpenState is BOTH, this Folder is * ready for operation if this method returns without error. The messages the folder returns * may not reflect the full state of the Folder, however, and returned emails may subsequently diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index e9f8dcb7..166d5312 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -5,6 +5,8 @@ */ private class Geary.ImapDB.Account : BaseObject { + private const int POPULATE_SEARCH_TABLE_DELAY_SEC = 30; + private class FolderReference : Geary.SmartReference { public Geary.FolderPath path; @@ -132,8 +134,13 @@ private class Geary.ImapDB.Account : BaseObject { background_cancellable = new Cancellable(); - // Kick off a background update of the search table. - populate_search_table_async.begin(background_cancellable); + // Kick off a background update of the search table, but since the database is getting + // hammered at startup, wait a bit before starting the update + Timeout.add_seconds(POPULATE_SEARCH_TABLE_DELAY_SEC, () => { + populate_search_table_async.begin(background_cancellable); + + return false; + }); initialize_contacts(cancellable); diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 2fce9e56..18e99af0 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -304,24 +304,26 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { private async void enumerate_folders_async(Cancellable? cancellable) throws Error { check_open(); - // get all local folders - Gee.HashMap local_children = yield enumerate_local_folders_async(null, - cancellable); + // enumerate local folders first + Gee.HashMap local_folders = yield enumerate_local_folders_async( + null, cancellable); // convert to a list of Geary.Folder ... build_folder() also reports new folders, so this - // gets the word out quickly + // gets the word out quickly (local_only folders have already been reported) Gee.Collection existing_list = new Gee.ArrayList(); - existing_list.add_all(build_folders(local_children.values)); + existing_list.add_all(build_folders(local_folders.values)); existing_list.add_all(local_only.values); + // build a map of all existing folders Gee.HashMap existing_folders = Geary.traverse(existing_list).to_hash_map(f => f.path); - // get all remote (server) folder paths - Gee.HashMap remote_folders = yield enumerate_remote_folders_async(null, - cancellable); + // now that all local have been enumerated and reported (this is important to assist + // startup of the UI), enumerate the remote folders + Gee.HashMap? remote_folders = yield enumerate_remote_folders_async( + null, cancellable); - // combine the two and make sure everything is up-to-date + // pair the local and remote folders and make sure everything is up-to-date yield update_folders_async(existing_folders, remote_folders, cancellable); } diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala b/src/engine/imap-engine/imap-engine-generic-folder.vala index e93c0659..f0081a51 100644 --- a/src/engine/imap-engine/imap-engine-generic-folder.vala +++ b/src/engine/imap-engine/imap-engine-generic-folder.vala @@ -437,6 +437,12 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde public override async bool open_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable = null) throws Error { if (open_count++ > 0) { + // even if opened or opening, respect the NO_DELAY flag + if (open_flags.is_all_set(OpenFlags.NO_DELAY)) { + cancel_remote_open_timer(); + wait_for_open_async.begin(); + } + debug("Not opening %s: already open (open_count=%d)", to_string(), open_count); return false; @@ -444,29 +450,32 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde this.open_flags = open_flags; - open_internal(cancellable); + open_internal(open_flags, cancellable); return true; } - private void open_internal(Cancellable? cancellable) { + private void open_internal(Folder.OpenFlags open_flags, Cancellable? cancellable) { remote_semaphore = new Geary.Nonblocking.ReportingSemaphore(false); // start the replay queue replay_queue = new ReplayQueue(this); - // do NOT open the remote side here; wait for the ReplayQueue to require a remote connection - // or wait_for_open_async() to be called ... this allows for fast local-only operations - // to occur, local-only either because (a) the folder has all the information required - // (for a list or fetch operation), or (b) the operation was de facto local-only. - // In particular, EmailStore will open and close lots of folders, causing a lot of - // connection setup and teardown - + // Unless NO_DELAY is set, do NOT open the remote side here; wait for the ReplayQueue to + // require a remote connection or wait_for_open_async() to be called ... this allows for + // fast local-only operations to occur, local-only either because (a) the folder has all + // the information required (for a list or fetch operation), or (b) the operation was de + // facto local-only. In particular, EmailStore will open and close lots of folders, + // causing a lot of connection setup and teardown + // // However, want to eventually open, otherwise if there's no user interaction (i.e. a // second account Inbox they don't manipulate), no remote connection will ever be made, // meaning that folder normalization never happens and unsolicited notifications never // arrive - start_remote_open_timer(); + if (open_flags.is_all_set(OpenFlags.NO_DELAY)) + wait_for_open_async.begin(); + else + start_remote_open_timer(); } private void start_remote_open_timer() { @@ -649,7 +658,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde if (remote_reason.is_error()) { debug("Reestablishing broken connect to %s", to_string()); - open_internal(null); + open_internal(OpenFlags.NO_DELAY, null); return; } diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala index 8d714052..0cd39c44 100644 --- a/src/engine/imap/api/imap-account.vala +++ b/src/engine/imap/api/imap-account.vala @@ -12,9 +12,10 @@ * that a Geary.Account implementation would need (in particular, {@link Geary.ImapEngine.Account} * and makes them into simple async calls. * - * Geary.Imap.Account does __no__ management of the {@link Imap.Folder} objects it returns. Thus, - * calling a fetch or list operation several times in a row will return separate Folder objects - * each time. It is up to the higher layers of the stack to manage these objects. + * Geary.Imap.Account manages the {@link Imap.Folder} objects it returns, but only in the sense + * that it will not create new instances repeatedly. Otherwise, it does not refresh or update the + * Imap.Folders themselves (such as update their {@link Imap.StatusData} periodically). + * That's the responsibility of the higher layers of the stack. */ private class Geary.Imap.Account : BaseObject { @@ -28,6 +29,7 @@ private class Geary.Imap.Account : BaseObject { private Nonblocking.Mutex cmd_mutex = new Nonblocking.Mutex(); private Gee.HashMap path_to_mailbox = new Gee.HashMap< FolderPath, MailboxInformation>(); + private Gee.HashMap folders = new Gee.HashMap(); private Gee.List? list_collector = null; private Gee.List? status_collector = null; @@ -143,28 +145,38 @@ private class Geary.Imap.Account : BaseObject { } public async bool folder_exists_async(FolderPath path, Cancellable? cancellable) throws Error { - return path_to_mailbox.contains(path); + return path_to_mailbox.has_key(path); } public async Imap.Folder fetch_folder_async(FolderPath path, Cancellable? cancellable) throws Error { check_open(); + if (folders.has_key(path)) + return folders.get(path); + // if not in map, use list_children_async to add it (if it exists) - if (!path_to_mailbox.contains(path)) + if (!path_to_mailbox.has_key(path)) { + debug("Listing children to find %s", path.to_string()); yield list_children_async(path.get_parent(), cancellable); + } MailboxInformation? mailbox_info = path_to_mailbox.get(path); if (mailbox_info == null) throw_not_found(path); + Imap.Folder folder; if (!mailbox_info.attrs.contains(MailboxAttribute.NO_SELECT)) { StatusData status = yield fetch_status_async(path, cancellable); - return new Imap.Folder(session_mgr, status, mailbox_info); + folder = new Imap.Folder(session_mgr, status, mailbox_info); } else { - return new Imap.Folder.unselectable(session_mgr, mailbox_info); + folder = new Imap.Folder.unselectable(session_mgr, mailbox_info); } + + folders.set(path, folder); + + return folder; } private async StatusData fetch_status_async(FolderPath path, Cancellable? cancellable) @@ -211,8 +223,18 @@ private class Geary.Imap.Account : BaseObject { Gee.Map cmd_map = new Gee.HashMap< StatusCommand, MailboxSpecifier>(); foreach (MailboxInformation mailbox_info in child_info) { + // if already have an Imap.Folder for this mailbox, use that + if (folders.has_key(mailbox_info.path)) { + child_folders.add(folders.get(mailbox_info.path)); + + continue; + } + + // if new mailbox is unselectable, don't bother doing a STATUS command if (mailbox_info.attrs.contains(MailboxAttribute.NO_SELECT)) { - child_folders.add(new Imap.Folder.unselectable(session_mgr, mailbox_info)); + Imap.Folder folder = new Imap.Folder.unselectable(session_mgr, mailbox_info); + folders.set(folder.path, folder); + child_folders.add(folder); continue; } @@ -255,7 +277,10 @@ private class Geary.Imap.Account : BaseObject { } status_results.remove(found_status); - child_folders.add(new Imap.Folder(session_mgr, found_status, mailbox_info)); + + Imap.Folder folder = new Imap.Folder(session_mgr, found_status, mailbox_info); + folders.set(folder.path, folder); + child_folders.add(folder); } if (status_results.size > 0) @@ -307,10 +332,8 @@ private class Geary.Imap.Account : BaseObject { // stash all MailboxInformation by path // TODO: remove any MailboxInformation for this parent that is not found (i.e. has been // removed on the server) - foreach (MailboxInformation mailbox_info in list_results) { - FolderPath path = mailbox_info.mailbox.to_folder_path(mailbox_info.delim); - path_to_mailbox.set(path, mailbox_info); - } + foreach (MailboxInformation mailbox_info in list_results) + path_to_mailbox.set(mailbox_info.path, mailbox_info); return (list_results.size > 0) ? list_results : null; } diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index 87eae1da..a38dc753 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -65,7 +65,7 @@ private class Geary.Imap.Folder : BaseObject { this.session_mgr = session_mgr; this.info = info; - path = info.mailbox.to_folder_path(info.delim); + path = info.path; properties = new Imap.FolderProperties.status(status, info.attrs); } @@ -73,7 +73,7 @@ private class Geary.Imap.Folder : BaseObject { internal Folder.unselectable(ClientSessionManager session_mgr, MailboxInformation info) { this.session_mgr = session_mgr; this.info = info; - path = info.mailbox.to_folder_path(info.delim); + path = info.path; properties = new Imap.FolderProperties(0, 0, 0, null, null, info.attrs); } diff --git a/src/engine/imap/response/imap-mailbox-information.vala b/src/engine/imap/response/imap-mailbox-information.vala index 1e6520bc..f29294e8 100644 --- a/src/engine/imap/response/imap-mailbox-information.vala +++ b/src/engine/imap/response/imap-mailbox-information.vala @@ -30,10 +30,19 @@ public class Geary.Imap.MailboxInformation : Object { */ public MailboxAttributes attrs { get; private set; } + /** + * The {@link Geary.FolderPath} for the mailbox. + * + * This is constructed from the supplied {@link mailbox} and {@link delim} returned from the + * server. + */ + public Geary.FolderPath path { get; private set; } + public MailboxInformation(MailboxSpecifier mailbox, string? delim, MailboxAttributes attrs) { this.mailbox = mailbox; this.delim = delim; this.attrs = attrs; + path = mailbox.to_folder_path(delim); } /** diff --git a/src/engine/imap/transport/imap-client-connection.vala b/src/engine/imap/transport/imap-client-connection.vala index 433406c6..6b913698 100644 --- a/src/engine/imap/transport/imap-client-connection.vala +++ b/src/engine/imap/transport/imap-client-connection.vala @@ -30,7 +30,7 @@ public class Geary.Imap.ClientConnection : BaseObject { */ public const uint DEFAULT_COMMAND_TIMEOUT_SEC = 15; - private const int FLUSH_TIMEOUT_MSEC = 100; + private const int FLUSH_TIMEOUT_MSEC = 10; private enum State { UNCONNECTED, diff --git a/src/engine/imap/transport/imap-client-session-manager.vala b/src/engine/imap/transport/imap-client-session-manager.vala index 4acc6b79..abfd436d 100644 --- a/src/engine/imap/transport/imap-client-session-manager.vala +++ b/src/engine/imap/transport/imap-client-session-manager.vala @@ -5,7 +5,7 @@ */ public class Geary.Imap.ClientSessionManager : BaseObject { - public const int DEFAULT_MIN_POOL_SIZE = 2; + public const int DEFAULT_MIN_POOL_SIZE = 1; private const int AUTHORIZED_SESSION_ERROR_RETRY_TIMEOUT_MSEC = 1000; public bool is_open { get; private set; default = false; }