[geary/wip/713432-startup: 44/45] Bug fixes, a little cleanup, add documentation

[Jim Nelson <jnelson src gnome org>]

commit bf95d20990269400885bd36fb66d3508eeda30b6
Author: Jim Nelson <jim yorba org>
Date:   Wed Jan 15 14:59:11 2014 -0800

    Bug fixes, a little cleanup, add documentation

 src/client/application/geary-controller.vala       |   42 +++++++------
 src/engine/api/geary-folder.vala                   |    8 +++
 src/engine/imap-db/imap-db-account.vala            |    7 ++-
 .../imap-engine/imap-engine-generic-account.vala   |   64 ++++++++++---------
 .../imap-engine/imap-engine-generic-folder.vala    |   15 +++--
 5 files changed, 79 insertions(+), 57 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 2225ef2..898f67a 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,7 +96,7 @@ public class GearyController : Geary.BaseObject {
     private UnityLauncher? unity_launcher = null;
     private Libnotify? libnotify = null;
     private uint select_folder_timeout_id = 0;
-    private time_t next_folder_select_allowed_time = 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;
@@ -948,31 +948,33 @@ public class GearyController : Geary.BaseObject {
         
         folder_to_select = folder;
         
-        // 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.
-        time_t now = time_t();
-        long diff = now - next_folder_select_allowed_time;
-        if (diff < SELECT_FOLDER_TIMEOUT_MSEC) {
+        // 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, on_select_folder_timeout);
+                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_time = now + SELECT_FOLDER_TIMEOUT_MSEC;
+            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;
     }
     
@@ -980,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
@@ -1002,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;
@@ -1051,10 +1052,15 @@ public class GearyController : Geary.BaseObject {
         current_conversations.conversations_added.connect(on_conversation_count_changed);
         current_conversations.conversation_removed.connect(on_conversation_count_changed);
         
-        if (!current_conversations.is_monitoring)
+        if (!current_conversations.is_monitoring) {
+            debug("Starting monitoring of %s...", folder.to_string());
             yield current_conversations.start_monitoring_async(conversation_cancellable);
+            debug("Monitoring %s", folder.to_string());
+        }
         
         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 e733636..8561b94 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -70,6 +70,8 @@ public interface Geary.Folder : BaseObject {
         FAST_OPEN,
         /**
          * Do not delay opening a connection to the server.
+         *
+         * @see open_async
          */
         NO_DELAY;
         
@@ -325,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 8f9c9f3..166d531 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,9 @@ private class Geary.ImapDB.Account : BaseObject {
         
         background_cancellable = new Cancellable();
         
-        // Kick off a background update of the search table.
-        Timeout.add_seconds(30, () => {
+        // 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;
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 28e0996..24cdeb0 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -7,6 +7,38 @@
 private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount {
     private const int REFRESH_FOLDER_LIST_SEC = 4 * 60;
     
+    // A background batch operation for enumerating folders in the local store (database)
+    private class EnumerateLocalOperation : Nonblocking.BatchOperation {
+        public weak GenericAccount owner;
+        public Gee.HashMap<FolderPath, ImapDB.Folder>? local_folders = null;
+        
+        public EnumerateLocalOperation(GenericAccount owner) {
+            this.owner = owner;
+        }
+        
+        public override async Object? execute_async(Cancellable? cancellable) throws Error {
+            local_folders = yield owner.enumerate_local_folders_async(null, cancellable);
+            
+            return null;
+        }
+    }
+    
+    // A background batch operation for enumerating folders in the remote store (server)
+    private class EnumerateRemoteOperation : Nonblocking.BatchOperation {
+        public weak GenericAccount owner;
+        public Gee.HashMap<FolderPath, Imap.Folder>? remote_folders = null;
+        
+        public EnumerateRemoteOperation(GenericAccount owner) {
+            this.owner = owner;
+        }
+        
+        public override async Object? execute_async(Cancellable? cancellable) throws Error {
+            remote_folders = yield owner.enumerate_remote_folders_async(null, cancellable);
+            
+            return null;
+        }
+    }
+    
     private static Geary.FolderPath? outbox_path = null;
     private static Geary.FolderPath? search_path = null;
     
@@ -301,36 +333,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount {
         reschedule_folder_refresh(false);
     }
     
-    private class EnumerateLocalOperation : Nonblocking.BatchOperation {
-        public weak GenericAccount owner;
-        public Gee.HashMap<FolderPath, ImapDB.Folder>? local_folders = null;
-        
-        public EnumerateLocalOperation(GenericAccount owner) {
-            this.owner = owner;
-        }
-        
-        public override async Object? execute_async(Cancellable? cancellable) throws Error {
-            local_folders = yield owner.enumerate_local_folders_async(null, cancellable);
-            
-            return null;
-        }
-    }
-    
-    private class EnumerateRemoteOperation : Nonblocking.BatchOperation {
-        public weak GenericAccount owner;
-        public Gee.HashMap<FolderPath, Imap.Folder>? remote_folders = null;
-        
-        public EnumerateRemoteOperation(GenericAccount owner) {
-            this.owner = owner;
-        }
-        
-        public override async Object? execute_async(Cancellable? cancellable) throws Error {
-            remote_folders = yield owner.enumerate_remote_folders_async(null, cancellable);
-            
-            return null;
-        }
-    }
-    
     private async void enumerate_folders_async(Cancellable? cancellable) throws Error {
         check_open();
         
@@ -357,6 +359,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount {
         yield update_folders_async(existing_folders, remote_op.remote_folders, cancellable);
     }
     
+    // Called by EnumerateLocalOperation
     private async Gee.HashMap<FolderPath, ImapDB.Folder> enumerate_local_folders_async(
         Geary.FolderPath? parent, Cancellable? cancellable) throws Error {
         check_open();
@@ -382,6 +385,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount {
         return result;
     }
     
+    // Called by EnumerateRemoteOperation
     private async Gee.HashMap<FolderPath, Imap.Folder> enumerate_remote_folders_async(
         Geary.FolderPath? parent, Cancellable? cancellable) throws Error {
         check_open();
diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala 
b/src/engine/imap-engine/imap-engine-generic-folder.vala
index 5bc600c..f0081a5 100644
--- a/src/engine/imap-engine/imap-engine-generic-folder.vala
+++ b/src/engine/imap-engine/imap-engine-generic-folder.vala
@@ -437,6 +437,7 @@ 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();
@@ -460,13 +461,13 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
         // 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