[geary/wip/789924-network-transition-redux: 8/11] Refine process for opening a remote session in MinimalFolder.



commit e76c0ed9acf5b1f9e29bb29c07a3995efb4d926b
Author: Michael James Gratton <mike vee net>
Date:   Mon Feb 5 18:13:56 2018 +1100

    Refine process for opening a remote session in MinimalFolder.
    
    * src/engine/imap-engine/imap-engine-minimal-folder.vala
      (MinimalFolder.normalize_folders): Add additional documentation for
      normalize_folders to make it a bit more clear what is going on.
      (MinimalFolder.open_remote_session_locked): Improve handling of
      IOError.CANCELLED errors - don't signal or close the folder since they
      are expected when closing the folder. Hook up to remote session signals
      for untagged responses before commencing normalisation to avoid missing
      any, but explicitly enable IMAP IDLE only after it completes to ensure
      we don't get any untagged EXPUNGE messages during normalisation that
      would screw up positional addressing. Ensure that the remote session's
      folder properties are actually hooked up to the MinimalFolder's
      aggregate properties. Hook up to the disconnect signal only when the
      session has been successfully opened to avoid races closing it before a
      new session has been opened.
      (MinimalFolder.on_remote_appended, MinimalFolder.on_remote_updated,
      MinimalFolder.on_remote_removed): Use the signal's target object
      parameter for untagged signal handlers rather than the remote_session
      property to avoid a race between a new session receiving untagged
      responses and that property becoming non-null.
    
    * src/engine/imap/api/imap-folder-session.vala (FolderSession): Add
      enable_idle method to allow clients to explicitly request IMAP IDLE
      be enabled for the session.
    
    * src/engine/imap/transport/imap-client-session.vala (ClientSession):
      Make is_idle_supported a read-only GObject property, remove getter.
      Add enable_idle method to brow-beat ClientConnection into actually
      sending an IMAP IDLE command when requested. Ensure that IDLE is
      disabled when leaving SELECTED or AUTHORIZED states.
    
    * src/engine/imap/transport/imap-client-connection.vala
      (ClientConnection): Make idle_when_quiet an actual GObject property,
      remove getter & setter.

 .../imap-engine/imap-engine-minimal-folder.vala    |  173 +++++++++++++-------
 src/engine/imap/api/imap-folder-session.vala       |   21 +++-
 .../imap/transport/imap-client-connection.vala     |   42 +++---
 src/engine/imap/transport/imap-client-session.vala |  108 +++++++------
 4 files changed, 217 insertions(+), 127 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index b51c55b..735f467 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -337,16 +337,29 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         }
     }
 
+    /**
+     * Synchronises the remote and local folders on session established.
+     *
+     * See [[https://tools.ietf.org/html/rfc4549|RFC 4549]] for an
+     * overview of the process
+     */
     private async void normalize_folders(Geary.Imap.FolderSession session,
-                                         Cancellable? cancellable)
+                                         Cancellable cancellable)
         throws Error {
         debug("%s: Begin normalizing remote and local folders", to_string());
 
         Geary.Imap.FolderProperties local_properties = this.local_folder.get_properties();
         Geary.Imap.FolderProperties remote_properties = session.folder.properties;
 
-        // and both must have their next UID's (it's possible they don't if it's a non-selectable
-        // folder)
+        /*
+         * Step 1: Check UID validity. If there are any problems, we
+         * can't continue, so either bail out completely or clear all
+         * local messages and let the client start fetching them all
+         * again.
+         */
+
+        // Both must have their next UID's - it's possible they don't
+        // if it's a non-selectable folder.
         if (local_properties.uid_next == null || local_properties.uid_validity == null) {
             throw new ImapError.NOT_SUPPORTED(
                 "%s: Unable to verify UIDs: missing local UIDNEXT (%s) and/or UIDVALIDITY (%s)",
@@ -379,6 +392,13 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             return;
         }
 
+        /*
+         * Step 2: Check the local folder. It may be empty, in which
+         * case the client can just start fetching messages normally,
+         * or it may be corrupt in which also clear it out and do
+         * same.
+         */
+
         // fetch email from earliest email to last to (a) remove any deletions and (b) update
         // any flags that may have changed
         ImapDB.EmailIdentifier? local_earliest_id = yield local_folder.get_earliest_id_async(cancellable);
@@ -394,30 +414,25 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             return;
         }
 
-        assert(local_earliest_id.has_uid());
-        assert(local_latest_id.has_uid());
-        
-        // if any messages are still marked for removal from last time, that means the EXPUNGE
-        // never arrived from the server, in which case the folder is "dirty" and needs a full
-        // normalization
+        // If any messages are still marked for removal from last
+        // time, that means the EXPUNGE never arrived from the server,
+        // in which case the folder is "dirty" and needs a full
+        // normalization. However, there may be enqueued
+        // ReplayOperations waiting to remove messages on the server
+        // that marked some or all of those messages, Don't consider
+        // those already marked as "already marked" if they were not
+        // leftover from the last open of this folder
         Gee.Set<ImapDB.EmailIdentifier>? already_marked_ids = yield local_folder.get_marked_ids_async(
             cancellable);
-        
-        // however, there may be enqueue ReplayOperations waiting to remove messages on the server
-        // that marked some or all of those messages
         Gee.HashSet<ImapDB.EmailIdentifier> to_be_removed = new Gee.HashSet<ImapDB.EmailIdentifier>();
         replay_queue.get_ids_to_be_remote_removed(to_be_removed);
-        
-        // don't consider those already marked as "already marked" if they were not leftover from
-        // the last open of this folder
         if (already_marked_ids != null)
             already_marked_ids.remove_all(to_be_removed);
-        
+
         bool is_dirty = (already_marked_ids != null && already_marked_ids.size > 0);
-        
         if (is_dirty)
             debug("%s: %d remove markers found, folder is dirty", to_string(), already_marked_ids.size);
-        
+
         // a full normalize works from the highest possible UID on the remote and work down to the lowest 
UID on
         // the local; this covers all messages appended since last seen as well as any removed
         Imap.UID last_uid = remote_properties.uid_next.previous(true);
@@ -432,6 +447,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             return;
         }
 
+        /*
+         * Step 3: Check remote folder, work out what has changed.
+         */
+
         // if UIDNEXT has changed, that indicates messages have been appended (and possibly removed)
         int64 uidnext_diff = remote_properties.uid_next.value - local_properties.uid_next.value;
         
@@ -439,9 +458,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             ? local_properties.select_examine_messages : 0;
         int remote_message_count = (remote_properties.select_examine_messages >= 0)
             ? remote_properties.select_examine_messages : 0;
-        
-        // if UIDNEXT is the same as last time AND the total count of email is the same, then
-        // nothing has been added or removed
+
+        // if UIDNEXT is the same as last time AND the total count of
+        // email is the same, then nothing has been added or removed,
+        // and we're done.
         if (!is_dirty && uidnext_diff == 0 && local_message_count == remote_message_count) {
             debug("%s: No messages added/removed since last opened, normalization completed", to_string());
             return;
@@ -529,7 +549,11 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         
         debug("%s: changes since last seen: removed=%d appended=%d inserted=%d", to_string(),
             removed_uids.size, appended_uids.size, inserted_uids.size);
-        
+
+        /*
+         * Step 4: Synchronise local folder with remote
+         */
+
         // fetch from the server the local store's required flags for all appended/inserted messages
         // (which is simply equal to all remaining remote UIDs)
         Gee.List<Geary.Email> to_create = new Gee.ArrayList<Geary.Email>();
@@ -600,14 +624,16 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         // remove any extant remove markers, as everything is accounted for now, except for those
         // waiting to be removed in the queue
         yield local_folder.clear_remove_markers_async(to_be_removed, cancellable);
-        
-        check_open("normalize_folders (clear remove markers)");
-        
-        //
-        // now normalized
-        // notify subscribers of changes
-        //
-        
+
+        if (cancellable.is_cancelled()) {
+            return;
+        }
+
+
+        /*
+         * Step 5: Notify subscribers of what has happened.
+         */
+
         Folder.CountChangeReason count_change_reason = Folder.CountChangeReason.NONE;
         
         if (removed_ids != null && removed_ids.size > 0) {
@@ -846,6 +872,12 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
     private async void open_remote_session_locked(Cancellable? cancellable) {
         debug("%s: Opening remote session", to_string());
 
+        // Note that any IOError.CANCELLED errors caught below do not
+        // cause any error signals to be fired and do not force
+        // closing the folder, because the only time opening the
+        // session is cancelled is when the folder is already being
+        // closed, which is the desired result.
+
         // Don't try to re-open again
         this.remote_open_timer.reset();
 
@@ -855,16 +887,35 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         try {
             session = yield this._account.open_folder_session(this.path, cancellable);
         } catch (Error err) {
-            // Notify that there was a connection error, but don't
-            // force the folder closed, since it might come good again
-            // if the user fixes an auth problem or the network comes
-            // back or whatever.
-            notify_open_failed(Folder.OpenFailed.REMOTE_ERROR, err);
+            if (!(err is IOError.CANCELLED)) {
+                // Notify that there was a connection error, but don't
+                // force the folder closed, since it might come good again
+                // if the user fixes an auth problem or the network comes
+                // back or whatever.
+                notify_open_failed(Folder.OpenFailed.REMOTE_ERROR, err);
+            }
             return;
         }
 
         // Phase 2: Update local state based on the remote session
 
+        // Replay signals need to be hooked up before normalisation to
+        // avoid there being a race between that and new messages
+        // arriving, being removed, etc. This is safe since
+        // normalisation only issues FETCH commands for messages based
+        // on the state of the remote right after being selected, so
+        // any untagged EXIST and FETCH responses will be handled
+        // later by their replay ops, and no untagged EXPUNGE
+        // responses will be received since they are forbidden to be
+        // issued for FETCH commands.
+        //
+        // Note we don't need to unhook from these signals if an error
+        // occurs below since they won't be called once the session
+        // has been released.
+        session.appended.connect(on_remote_appended);
+        session.updated.connect(on_remote_updated);
+        session.removed.connect(on_remote_removed);
+
         try {
             yield normalize_folders(session, cancellable);
         } catch (Error err) {
@@ -872,9 +923,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             // so treat as in the error case above, after resolving if
             // the issue was local or remote.
             this._account.release_folder_session(session);
-            if (err is IOError.CANCELLED) {
-                notify_open_failed(OpenFailed.LOCAL_ERROR, err);
-            } else {
+            if (!(err is IOError.CANCELLED)) {
                 Folder.CloseReason local_reason = CloseReason.LOCAL_ERROR;
                 Folder.CloseReason remote_reason = CloseReason.REMOTE_CLOSE;
                 if (!is_remote_error(err)) {
@@ -895,17 +944,9 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             return;
         }
 
-        this._properties.add(session.folder.properties);
-        this.remote_session = session;
-
-        // Signals need to be hooked after the remote session is in
-        // place so they can access the remote session's folder
-        // properties.
-        session.appended.connect(on_remote_appended);
-        session.updated.connect(on_remote_updated);
-        session.removed.connect(on_remote_removed);
-        session.disconnected.connect(on_remote_disconnected);
-
+        // Update the local folder's totals and UID values after
+        // normalisation, so it does not mistake the remote's current
+        // state with our previous state
         try {
             yield local_folder.update_folder_select_examine(
                 session.folder.properties, cancellable
@@ -916,8 +957,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             // the open was simply cancelled. So clean up, and force
             // the folder closed if needed.
             this._account.release_folder_session(session);
-            notify_open_failed(Folder.OpenFailed.LOCAL_ERROR, err);
             if (!(err is IOError.CANCELLED)) {
+                notify_open_failed(Folder.OpenFailed.LOCAL_ERROR, err);
                 this.close_internal_async.begin(
                     CloseReason.LOCAL_ERROR,
                     CloseReason.REMOTE_CLOSE,
@@ -928,6 +969,17 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             return;
         }
 
+        // All done, can now hook up the session to the folder
+        this.remote_session = session;
+        this._properties.add(session.folder.properties);
+        session.disconnected.connect(on_remote_disconnected);
+
+        // Enable IDLE now that the local and remote folders are in
+        // sync. Can't do this earlier since we might get untagged
+        // EXPUNGE responses during normalisation, which would be
+        // Badâ„¢. Do it in the background to avoid delay notifying
+        session.enable_idle.begin(cancellable);
+
         // Phase 3: Notify tasks waiting for the connection
 
         // notify any subscribers with similar information
@@ -975,8 +1027,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         notify_email_locally_complete(email_ids);
     }
 
-    private void on_remote_appended(int appended) {
-        int remote_count = this.remote_session.folder.properties.email_total;
+    private void on_remote_appended(Imap.FolderSession session, int appended) {
+        // Use the session param rather than remote_session attr since
+        // it may not be available yet
+        int remote_count = session.folder.properties.email_total;
         debug("%s on_remote_appended: remote_count=%d appended=%d",
               to_string(), remote_count, appended);
 
@@ -997,8 +1051,12 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         }
     }
 
-    private void on_remote_updated(Imap.SequenceNumber position, Imap.FetchedData data) {
-        int remote_count = this.remote_session.folder.properties.email_total;
+    private void on_remote_updated(Imap.FolderSession session,
+                                   Imap.SequenceNumber position,
+                                   Imap.FetchedData data) {
+        // Use the session param rather than remote_session attr since
+        // it may not be available yet
+        int remote_count = session.folder.properties.email_total;
         debug("%s on_remote_updated: remote_count=%d position=%s", to_string(),
               remote_count, position.to_string());
 
@@ -1007,8 +1065,11 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         );
     }
 
-    private void on_remote_removed(Imap.SequenceNumber position) {
-        int remote_count = this.remote_session.folder.properties.email_total;
+    private void on_remote_removed(Imap.FolderSession session,
+                                   Imap.SequenceNumber position) {
+        // Use the session param rather than remote_session attr since
+        // it may not be available yet
+        int remote_count = session.folder.properties.email_total;
         debug("%s on_remote_removed: remote_count=%d position=%s",
               to_string(), remote_count, position.to_string());
 
diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala
index 06f7a13..926edd1 100644
--- a/src/engine/imap/api/imap-folder-session.vala
+++ b/src/engine/imap/api/imap-folder-session.vala
@@ -151,8 +151,27 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
     }
 
     /**
-     * {@inheritDoc}
+     * Enables IMAP IDLE for the session, if supported.
      */
+    public async void enable_idle(Cancellable? cancellable)
+        throws Error {
+        ClientSession session = claim_session();
+        int token = yield this.cmd_mutex.claim_async(cancellable);
+        Error? cmd_err = null;
+        try {
+            yield session.enable_idle(cancellable);
+        } catch (Error err) {
+            cmd_err = err;
+        }
+
+        this.cmd_mutex.release(ref token);
+
+        if (cmd_err != null) {
+            throw cmd_err;
+        }
+    }
+
+    /** {@inheritDoc} */
     public override ClientSession? close() {
         ClientSession? old_session = base.close();
         if (old_session != null) {
diff --git a/src/engine/imap/transport/imap-client-connection.vala 
b/src/engine/imap/transport/imap-client-connection.vala
index 2edec29..735fe74 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -111,7 +111,25 @@ public class Geary.Imap.ClientConnection : BaseObject {
      * in logs and debug output.
      */
     public int cx_id { get; private set; }
-    
+
+    /**
+     * Determines if the connection will use IMAP IDLE when idle.
+     *
+     * If //true//, when the connection is not sending commands
+     * ("quiet"), it will issue an IDLE command to enter a state where
+     * unsolicited server data may be sent from the server without
+     * resorting to NOOP keepalives.  (Note that keepalives are still
+     * required to hold the connection open, according to the IMAP
+     * specification.)
+     *
+     * Note that setting this false will *not* break a connection out
+     * of IDLE state alone; a command needs to be flushed down the
+     * pipe to do that.  (NOOP would be a good choice.)  Nor will this
+     * initiate an IDLE command either; it can only do that after
+     * sending a command (again, NOOP would be a good choice).
+     */
+    public bool idle_when_quiet = false;
+
     private Geary.Endpoint endpoint;
     private Geary.State.Machine fsm;
     private SocketConnection? cx = null;
@@ -125,7 +143,6 @@ public class Geary.Imap.ClientConnection : BaseObject {
     private int tag_counter = 0;
     private char tag_prefix = 'a';
     private uint flush_timeout_id = 0;
-    private bool idle_when_quiet = false;
     private Gee.HashSet<Tag> posted_idle_tags = new Gee.HashSet<Tag>();
     private int outstanding_idle_dones = 0;
     private Tag? posted_synchronization_tag = null;
@@ -297,26 +314,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
         // TODO This could be optimized, but we'll leave it for now.
         return new Tag("%c%03d".printf(tag_prefix, tag_counter));
     }
-    
-    /**
-     * If true, when the connection is not sending commands ("quiet"), it will issue an IDLE command
-     * to enter a state where unsolicited server data may be sent from the server without resorting
-     * to NOOP keepalives.  (Note that keepalives are still required to hold the connection open,
-     * according to the IMAP specification.)
-     *
-     * Note that this will *not* break a connection out of IDLE state alone; a command needs to be
-     * flushed down the pipe to do that.  (NOOP would be a good choice.)  Nor will this initiate
-     * an IDLE command either; it can only do that after sending a command (again, NOOP would be
-     * a good choice).
-     */
-    public void set_idle_when_quiet(bool idle_when_quiet) {
-        this.idle_when_quiet = idle_when_quiet;
-    }
-    
-    public bool get_idle_when_quiet() {
-        return idle_when_quiet;
-    }
-    
+
     public SocketAddress? get_remote_address() {
         if (cx == null)
             return null;
diff --git a/src/engine/imap/transport/imap-client-session.vala 
b/src/engine/imap/transport/imap-client-session.vala
index 4954cbb..ff3b036 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -229,6 +229,12 @@ public class Geary.Imap.ClientSession : BaseObject {
      */
     public Capabilities capabilities { get; private set; default = new Capabilities(0); }
 
+    /** Determines if this session supports the IMAP IDLE extension. */
+    public bool is_idle_supported {
+        get { return this.capabilities.has_capability(Capabilities.IDLE); }
+    }
+
+
     // While the following inbox and namespace data should be server
     // specific, there is a small chance they will differ between
     // connections if the connections connect to different servers in
@@ -261,7 +267,6 @@ public class Geary.Imap.ClientSession : BaseObject {
     private uint selected_keepalive_secs = 0;
     private uint unselected_keepalive_secs = 0;
     private uint selected_with_idle_keepalive_secs = 0;
-    private bool allow_idle = true;
 
     private Gee.HashMap<Tag, StatusResponse> seen_completion_responses = new Gee.HashMap<
         Tag, StatusResponse>();
@@ -403,6 +408,7 @@ public class Geary.Imap.ClientSession : BaseObject {
             new Geary.State.Mapping(State.SELECTING, Event.LOGOUT, on_logout),
             new Geary.State.Mapping(State.SELECTING, Event.DISCONNECT, on_disconnect),
             new Geary.State.Mapping(State.SELECTING, Event.RECV_STATUS, on_recv_status),
+
             new Geary.State.Mapping(State.SELECTING, Event.RECV_COMPLETION, on_selecting_recv_completion),
             new Geary.State.Mapping(State.SELECTING, Event.SEND_ERROR, on_send_error),
             new Geary.State.Mapping(State.SELECTING, Event.RECV_ERROR, on_recv_error),
@@ -732,18 +738,15 @@ public class Geary.Imap.ClientSession : BaseObject {
         cx.recv_closed.connect(on_received_closed);
         cx.receive_failure.connect(on_network_receive_failure);
         cx.deserialize_failure.connect(on_network_receive_failure);
-        
+
         assert(connect_waiter == null);
         connect_waiter = new Nonblocking.Semaphore();
-        
-        // only use IDLE when in SELECTED or EXAMINED state
-        cx.set_idle_when_quiet(false);
-        
+
         params.proceed = true;
-        
+
         return State.CONNECTING;
     }
-    
+
     // this is used internally to tear-down the ClientConnection object and unhook it from
     // ClientSession
     private void drop_connection() {
@@ -1097,22 +1100,36 @@ public class Geary.Imap.ClientSession : BaseObject {
         
         return true;
     }
-    
+
     /**
-     * If enabled, an IDLE command will be used for notification of unsolicited server data whenever
-     * a mailbox is selected or examined.  IDLE will only be used if ClientSession has seen a
-     * CAPABILITY server data response with IDLE listed as a supported extension.
+     * Enables IMAP IDLE for the client session, if supported.
      *
-     * This will *not* break a connection out of IDLE mode; a command must be sent as well to force
-     * the connection back to de-idled state.
-     *
-     * Note that this overrides other heuristics ClientSession uses about allowing idle, so use
-     * with caution.
+     * If enabled, an IDLE command will be used for notification of
+     * unsolicited server data whenever a mailbox is selected or
+     * examined.  IDLE will only be used if ClientSession has seen a
+     * CAPABILITY server data response with IDLE listed as a supported
+     * extension.
      */
-    public void allow_idle_when_selected(bool allow_idle) {
-        this.allow_idle = allow_idle;
+    public async void enable_idle(Cancellable? cancellable)
+        throws Error {
+        if (this.is_idle_supported) {
+            switch (get_protocol_state(null)) {
+            case ProtocolState.AUTHORIZING:
+            case ProtocolState.AUTHORIZED:
+            case ProtocolState.SELECTED:
+            case ProtocolState.SELECTING:
+                this.cx.idle_when_quiet = true;
+                yield send_command_async(new NoopCommand(), cancellable);
+                break;
+
+            default:
+                throw new ImapError.NOT_SUPPORTED(
+                    "IMAP IDLE only supported in AUTHORIZED or SELECTED states"
+                );
+            }
+        }
     }
-    
+
     private void schedule_keepalive() {
         // if old one was scheduled, unschedule and schedule anew
         unschedule_keepalive();
@@ -1122,13 +1139,14 @@ public class Geary.Imap.ClientSession : BaseObject {
             case ProtocolState.UNCONNECTED:
             case ProtocolState.CONNECTING:
                 return;
-            
+
             case ProtocolState.SELECTING:
             case ProtocolState.SELECTED:
-                seconds = (allow_idle && supports_idle()) ? selected_with_idle_keepalive_secs
+                seconds = (this.cx.idle_when_quiet && this.is_idle_supported)
+                    ? selected_with_idle_keepalive_secs
                     : selected_keepalive_secs;
             break;
-            
+
             case ProtocolState.UNAUTHORIZED:
             case ProtocolState.AUTHORIZING:
             case ProtocolState.AUTHORIZED:
@@ -1180,11 +1198,7 @@ public class Geary.Imap.ClientSession : BaseObject {
     public bool install_recv_converter(Converter converter) {
         return (cx != null) ? cx.install_recv_converter(converter) : false;
     }
-    
-    public bool supports_idle() {
-        return capabilities.has_capability(Capabilities.IDLE);
-    }
-    
+
     //
     // send commands
     //
@@ -1336,23 +1350,16 @@ public class Geary.Imap.ClientSession : BaseObject {
         
         return yield command_transaction_async(cmd, cancellable);
     }
-    
+
     private uint on_select(uint state, uint event, void *user, Object? object) {
         MachineParams params = (MachineParams) object;
-        
+
         if (!reserve_state_change_cmd(params, state, event))
             return state;
-        
-        // Allow IDLE *before* issuing SELECT/EXAMINE because there's no guarantee another command
-        // will be issued any time soon, which is necessary for the IDLE command to be tacked on
-        // to the end of it.  In other words, telling ClientConnection to go into IDLE after the
-        // SELECT/EXAMINE command is too late unless another command is sent (set_idle_when_quiet()
-        // performs no I/O).
-        cx.set_idle_when_quiet(allow_idle && supports_idle());
-        
+
         return State.SELECTING;
     }
-    
+
     private uint on_not_selected(uint state, uint event, void *user, Object? object) {
         MachineParams params = (MachineParams) object;
         
@@ -1394,10 +1401,12 @@ public class Geary.Imap.ClientSession : BaseObject {
             
             default:
                 debug("[%s]: Unable to SELECT/EXAMINE: %s", to_string(), completion_response.to_string());
-                
-                // turn off IDLE, not entering SELECTED/EXAMINED state
-                cx.set_idle_when_quiet(false);
-                
+
+                // turn off IDLE, client should request it again if desired.
+                if (cx.idle_when_quiet) {
+                    cx.idle_when_quiet = false;
+                }
+
                 return State.AUTHORIZED;
         }
     }
@@ -1428,10 +1437,10 @@ public class Geary.Imap.ClientSession : BaseObject {
         assert(params.cmd is CloseCommand);
         if (!reserve_state_change_cmd(params, state, event))
             return state;
-        
+
         // returning to AUTHORIZED state, turn off IDLE
-        cx.set_idle_when_quiet(false);
-        
+        cx.idle_when_quiet = false;
+
         return State.CLOSING_MAILBOX;
     }
     
@@ -1481,11 +1490,14 @@ public class Geary.Imap.ClientSession : BaseObject {
     
     private uint on_logout(uint state, uint event, void *user, Object? object) {
         MachineParams params = (MachineParams) object;
-        
+
         assert(params.cmd is LogoutCommand);
         if (!reserve_state_change_cmd(params, state, event))
             return state;
-        
+
+        // Leaving AUTHORIZED state, turn off IDLE
+        cx.idle_when_quiet = false;
+
         return State.LOGGING_OUT;
     }
     


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