[geary/wip/778276-better-flag-updates: 9/17] Convert folder unseen update into an account op, only schedule if closed.

[Michael Gratton <mjog src gnome org>]

commit 0e61a425dde1542c5e7c3c32e4ba02252929f6d0
Author: Michael James Gratton <mike vee net>
Date:   Fri Nov 24 15:39:14 2017 +1100

    Convert folder unseen update into an account op, only schedule if closed.
    
    If the folder is open, we should be getting notifications of flag changes
    from the server, so double-checking is kind of pointless, and since it's
    implemented using an IMAP STATUS, also expensive for the server.
    
    * src/engine/imap-engine/imap-engine-generic-account.vala
      (GenericAccount): Convert refresh_unseen_async method into a new
      RefreshFolderUnseen account operation, add a guard to ensure the folder
      is closed when executing. Remove hash map of timeouts and replace with
      a single timeout manager on MinimalFolder itself.
    
    * src/engine/imap-engine/imap-engine-minimal-folder.vala (MinimalFolder):
      Add timeout manager for unseen update, only start it running if closed,
      when triggered, schedule a RefreshFolderUnseen op on the account.

 .../imap-engine/imap-engine-generic-account.vala   |  196 ++++++++++----------
 .../imap-engine/imap-engine-minimal-folder.vala    |   47 ++++-
 2 files changed, 142 insertions(+), 101 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 078ad54..336b714 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -9,7 +9,6 @@
 private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
 
     private const int REFRESH_FOLDER_LIST_SEC = 2 * 60;
-    private const int REFRESH_UNSEEN_SEC = 1;
     private const Geary.SpecialFolderType[] SUPPORTED_SPECIAL_FOLDERS = {
         Geary.SpecialFolderType.DRAFTS,
         Geary.SpecialFolderType.SENT,
@@ -28,9 +27,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
     private Gee.HashMap<FolderPath, MinimalFolder> folder_map = new Gee.HashMap<
         FolderPath, MinimalFolder>();
     private Gee.HashMap<FolderPath, Folder> local_only = new Gee.HashMap<FolderPath, Folder>();
-    private Gee.HashMap<FolderPath, uint> refresh_unseen_timeout_ids
-        = new Gee.HashMap<FolderPath, uint>();
-    private Gee.HashSet<Geary.Folder> in_refresh_unseen = new Gee.HashSet<Geary.Folder>();
     private AccountProcessor? processor;
     private AccountSynchronizer sync;
     private TimeoutManager refresh_folder_timer;
@@ -111,28 +107,28 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
             }
         }
     }
-    
+
     protected override void notify_email_appended(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> 
ids) {
         base.notify_email_appended(folder, ids);
-        reschedule_unseen_update(folder);
+        schedule_unseen_update(folder);
     }
-    
+
     protected override void notify_email_inserted(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> 
ids) {
         base.notify_email_inserted(folder, ids);
-        reschedule_unseen_update(folder);
+        schedule_unseen_update(folder);
     }
-    
+
     protected override void notify_email_removed(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> 
ids) {
         base.notify_email_removed(folder, ids);
-        reschedule_unseen_update(folder);
+        schedule_unseen_update(folder);
     }
-    
+
     protected override void notify_email_flags_changed(Geary.Folder folder,
         Gee.Map<Geary.EmailIdentifier, Geary.EmailFlags> flag_map) {
         base.notify_email_flags_changed(folder, flag_map);
-        reschedule_unseen_update(folder);
+        schedule_unseen_update(folder);
     }
-    
+
     private void check_open() throws EngineError {
         if (!open)
             throw new EngineError.OPEN_REQUIRED("Account %s not opened", to_string());
@@ -348,93 +344,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         return all_folders;
     }
 
-    // Subclasses should implement this to return their flavor of a MinimalFolder with the
-    // appropriate interfaces attached.  The returned folder should have its SpecialFolderType
-    // set using either the properties from the local folder or its path.
-    //
-    // This won't be called to build the Outbox or search folder, but for all others (including Inbox) it 
will.
-    protected abstract MinimalFolder new_folder(ImapDB.Folder local_folder);
-
-    /**
-     * Hooks up and queues an {@link UpdateRemoteFolders} operation.
-     */
-    private void update_remote_folders() {
-        UpdateRemoteFolders op = new UpdateRemoteFolders(
-            this,
-            this.remote,
-            this.local,
-            this.local_only.keys,
-            get_supported_special_folders()
-        );
-        op.completed.connect(() => {
-                this.refresh_folder_timer.start();
-            });
-        try {
-            queue_operation(op);
-        } catch (Error err) {
-            // oh well
-        }
-    }
-
-    private void reschedule_unseen_update(Geary.Folder folder) {
-        if (!folder_map.has_key(folder.path))
-            return;
-        
-        if (refresh_unseen_timeout_ids.get(folder.path) != 0)
-            Source.remove(refresh_unseen_timeout_ids.get(folder.path));
-        
-        refresh_unseen_timeout_ids.set(folder.path,
-            Timeout.add_seconds(REFRESH_UNSEEN_SEC, () => on_refresh_unseen(folder)));
-    }
-    
-    private bool on_refresh_unseen(Geary.Folder folder) {
-        // If we're in the process already, reschedule for later.
-        if (in_refresh_unseen.contains(folder))
-            return true;
-        
-        // add here, remove in completed callback
-        in_refresh_unseen.add(folder);
-        
-        refresh_unseen_async.begin(folder, null, on_refresh_unseen_completed);
-        
-        refresh_unseen_timeout_ids.unset(folder.path);
-        return false;
-    }
-    
-    private void on_refresh_unseen_completed(Object? source, AsyncResult result) {
-        try {
-            refresh_unseen_async.end(result);
-        } catch (Error e) {
-            debug("Error refreshing unseen counts: %s", e.message);
-        }
-    }
-
-    private async void refresh_unseen_async(Geary.Folder folder, Cancellable? cancellable)
-    throws Error {
-        debug("Refreshing unseen counts for %s", folder.to_string());
-
-        try {
-            Imap.Folder remote_folder = yield remote.fetch_folder_cached_async(
-                folder.path,
-                true,
-                cancellable
-            );
-
-            yield local.update_folder_status_async(remote_folder, false, true, cancellable);
-        } finally {
-            // added when call scheduled (above)
-            in_refresh_unseen.remove(folder);
-        }
-    }
-
     public override Geary.ContactStore get_contact_store() {
         return local.contact_store;
     }
 
-    protected virtual Geary.SpecialFolderType[] get_supported_special_folders() {
-        return SUPPORTED_SPECIAL_FOLDERS;
-    }
-
     public override async bool folder_exists_async(Geary.FolderPath path,
         Cancellable? cancellable = null) throws Error {
         check_open();
@@ -703,6 +616,48 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         return minimal_folder;
     }
 
+    // Subclasses should implement this to return their flavor of a MinimalFolder with the
+    // appropriate interfaces attached.  The returned folder should have its SpecialFolderType
+    // set using either the properties from the local folder or its path.
+    //
+    // This won't be called to build the Outbox or search folder, but for all others (including Inbox) it 
will.
+    protected abstract MinimalFolder new_folder(ImapDB.Folder local_folder);
+
+    /**
+     * Hooks up and queues an {@link UpdateRemoteFolders} operation.
+     */
+    private void update_remote_folders() {
+        UpdateRemoteFolders op = new UpdateRemoteFolders(
+            this,
+            this.remote,
+            this.local,
+            this.local_only.keys,
+            get_supported_special_folders()
+        );
+        op.completed.connect(() => {
+                this.refresh_folder_timer.start();
+            });
+        try {
+            queue_operation(op);
+        } catch (Error err) {
+            // oh well
+        }
+    }
+
+    /**
+     * Hooks up and queues an {@link RefreshFolderUnseen} operation.
+     */
+    private void schedule_unseen_update(Geary.Folder folder) {
+        MinimalFolder? impl = folder as MinimalFolder;
+        if (impl != null) {
+            impl.refresh_unseen();
+        }
+    }
+
+    protected virtual Geary.SpecialFolderType[] get_supported_special_folders() {
+        return SUPPORTED_SPECIAL_FOLDERS;
+    }
+
     private void compile_special_search_names() {
         /*
          * Compiles the list of names used to search for special
@@ -1078,3 +1033,52 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
     }
 
 }
+
+/**
+ * Account operation that updates a folder's unseen message count.
+ *
+ * This performs a IMAP STATUS on the folder, but only if it is not
+ * open - if it is open it is already maintaining its unseen count.
+ */
+internal class Geary.ImapEngine.RefreshFolderUnseen : AccountOperation {
+
+
+    private weak Geary.Folder folder;
+    private weak Imap.Account remote;
+    private weak ImapDB.Account local;
+
+
+    internal RefreshFolderUnseen(Geary.Folder folder,
+                                 Imap.Account remote,
+                                 ImapDB.Account local) {
+        this.folder = folder;
+        this.remote = remote;
+        this.local = local;
+    }
+
+    public override bool equal_to(AccountOperation op) {
+        return (
+            base.equal_to(op) &&
+            this.folder.path.equal_to(((RefreshFolderUnseen) op).folder.path)
+        );
+    }
+
+    public override string to_string() {
+        return "%s(%s)".printf(base.to_string(), folder.path.to_string());
+    }
+
+    public override async void execute(Cancellable cancellable) throws Error {
+        if (this.folder.get_open_state() == Geary.Folder.OpenState.CLOSED) {
+            Imap.Folder remote_folder = yield remote.fetch_folder_cached_async(
+                folder.path,
+                true,
+                cancellable
+            );
+
+            yield local.update_folder_status_async(
+                remote_folder, false, true, cancellable
+            );
+        }
+    }
+
+}
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 5b90256..c1fbc40 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -26,7 +26,10 @@
 private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport.Copy,
     Geary.FolderSupport.Mark, Geary.FolderSupport.Move {
 
+
     private const int FORCE_OPEN_REMOTE_TIMEOUT_SEC = 10;
+    private const int REFRESH_UNSEEN_TIMEOUT_SEC = 1;
+
 
     public override Account account { get { return _account; } }
     
@@ -69,6 +72,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
     private int remote_count = -1;
     private Nonblocking.Mutex open_mutex = new Nonblocking.Mutex();
     private Nonblocking.Mutex close_mutex = new Nonblocking.Mutex();
+    private TimeoutManager refresh_unseen_timer;
     private Cancellable? open_cancellable = null;
 
 
@@ -117,6 +121,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         this.replay_queue = new ReplayQueue(this);
         this.email_prefetcher = new EmailPrefetcher(this);
 
+        this.refresh_unseen_timer = new TimeoutManager.seconds(
+            REFRESH_UNSEEN_TIMEOUT_SEC, on_refresh_unseen
+        );
+
         // Notify now to ensure that wait_for_close_async does not
         // block if never opened.
         this.closed_semaphore.blind_notify();
@@ -573,15 +581,19 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         
         // first open gets to name the flags, but see note above
         this.open_flags = open_flags;
-        
+
         // reset to force waiting in wait_for_open_async()
-        remote_semaphore.reset();
-        
+        this.remote_semaphore.reset();
+
         // reset to force waiting in wait_for_close_async()
-        closed_semaphore.reset();
+        this.closed_semaphore.reset();
+
+        // reset unseen count refresh since it will be updated when
+        // the remote opens
+        this.refresh_unseen_timer.reset();
 
         this.open_cancellable = new Cancellable();
-        
+
         // 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
@@ -1576,6 +1588,31 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
             remote_opened.to_string());
     }
 
+    /**
+     * Schedules a refresh of the unseen count for the folder.
+     *
+     * This will only refresh folders that are not open, since if they
+     * are open or opening, they will already be updated. Hence it is safe to be called on closed folders.
+     */
+    internal void refresh_unseen() {
+        if (this.open_count == 0) {
+            this.refresh_unseen_timer.start();
+        }
+    }
+
+    private void on_refresh_unseen() {
+        // We queue an account operation since the folder itself is
+        // closed and hence does not have a connection to use for it.
+        RefreshFolderUnseen op = new RefreshFolderUnseen(
+            this, this.remote, this.local
+        );
+        try {
+            this._account.queue_operation(op);
+        } catch (Error err) {
+            // oh well
+        }
+    }
+
     private void on_remote_ready() {
         start_open_remote();
     }