[geary/mjog/invert-folder-class-hierarchy] engine: Update ConversationMonitor fill handling

[Michael Gratton <mjog src gnome org>]

commit 7c12b04adf4e32debbea08aef2c25e6964c05ad0
Author: Michael Gratton <mike vee net>
Date:   Sun Mar 7 10:12:04 2021 +1100

    engine: Update ConversationMonitor fill handling
    
    Simplify the fill handling code, introduce and explicit window size
    setting method since it's a potentially expensive operation. Expand the
    vector if the folder is a remote and looks like it should have enough
    email to do so. Ensure operations that should be checking the window
    afterwards are.

 .../application/application-main-window.vala       |   7 +-
 src/engine/app/app-conversation-monitor.vala       | 161 +++++++++++++++------
 .../conversation-monitor/app-append-operation.vala |   4 +
 .../app-fill-window-operation.vala                 |  12 +-
 .../conversation-monitor/app-insert-operation.vala |   4 +
 .../conversation-monitor/app-remove-operation.vala |   5 +-
 6 files changed, 134 insertions(+), 59 deletions(-)
---
diff --git a/src/client/application/application-main-window.vala 
b/src/client/application/application-main-window.vala
index 4abf94604..164c21076 100644
--- a/src/client/application/application-main-window.vala
+++ b/src/client/application/application-main-window.vala
@@ -1597,7 +1597,7 @@ public class Application.MainWindow :
 
     private async void open_conversation_monitor(Geary.App.ConversationMonitor to_open,
                                                  GLib.Cancellable cancellable) {
-        to_open.scan_completed.connect(on_scan_completed);
+        to_open.scan_completed.connect_after(on_scan_completed);
         to_open.scan_error.connect(on_scan_error);
 
         to_open.scan_completed.connect(on_conversation_count_changed);
@@ -1668,7 +1668,10 @@ public class Application.MainWindow :
 
     private void load_more() {
         if (this.conversations != null) {
-            this.conversations.min_window_count += MIN_CONVERSATION_COUNT;
+            this.conversations.update_minimum_window_size(
+                this.conversations.minimum_window_size +
+                MIN_CONVERSATION_COUNT
+            );
         }
     }
 
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index f264c9250..43d32cacf 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -22,7 +22,7 @@
  * When monitoring starts via a call to {@link
  * start_monitoring}, the folder will perform an initial
  * //scan// of messages in the base folder and load conversation load
- * based on that. Increasing {@link min_window_count} will cause
+ * based on that. Increasing {@link minimum_window_size} will cause
  * additional scan operations to be executed as needed to fill the new
  * window size.
  *
@@ -94,33 +94,18 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
     /** Determines if this monitor is monitoring the base folder. */
     public bool is_monitoring { get; private set; default = false; }
 
-    /** Determines if more conversations should be loaded. */
-    public bool should_load_more {
-        get {
-            return (this.conversations.size < this.min_window_count);
-        }
-    }
+    /** Minimum number of conversations the monitor should maintain. */
+    public int minimum_window_size { get; private set; }
 
-    /** Determines if more conversations can be loaded. */
+    /**
+     * Determines if more conversations can be loaded.
+     */
     public bool can_load_more {
         get {
-            return (
-                this.base_folder.email_total >
-                this.folder_window_size
-            ) && !this.fill_complete;
+            return (this.can_load_more_local | this.can_load_more_remote);
         }
     }
 
-    /** Minimum number of emails large conversations should contain. */
-    public int min_window_count {
-        get { return _min_window_count; }
-        set {
-            _min_window_count = value;
-            check_window_count();
-        }
-    }
-    private int _min_window_count = 0;
-
     /** Indicates progress conversations are being loaded. */
     public ProgressMonitor progress_monitor {
         get; private set; default = new SimpleProgressMonitor(ProgressType.ACTIVITY);
@@ -136,13 +121,55 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         get { return this.base_folder; }
     }
 
+    /**
+     * Determines if more conversations should be loaded.
+     *
+     * If true, the monitor will attempt to load more email from the
+     * base folder's vector.
+     */
+    internal bool should_load_more {
+        get {
+            return (this.conversations.size < this.minimum_window_size);
+        }
+    }
+
+    /**
+     * Determines if more email can be loaded from the folder.
+     *
+     * If true, then more email is present in the base folder's vector
+     * which can be loaded locally.
+     */
+    internal bool can_load_more_local {
+        get {
+            return (this.base_folder.email_total > this.folder_window_size);
+        }
+    }
+
+    /**
+     * Determines if more email can be loaded from the remote.
+     *
+     * If true, the base folder is remote-backed, there is an open
+     * connection to the remote, and more email is known to be present
+     * on the remote which could be downloaded.
+     */
+    internal bool can_load_more_remote {
+        get {
+            return (
+                this.remote != null &&
+                this.remote.is_connected &&
+                this.base_folder.email_total <
+                this.remote.remote_properties.email_total
+            );
+        }
+    }
+
     /** The set of all conversations loaded by the monitor. */
     internal ConversationSet conversations { get; private set; }
 
     /** The number of messages currently loaded from the base folder. */
     internal uint folder_window_size {
         get {
-            return (this.window.is_empty) ? 0 : this.window.size;
+            return this.window.size;
         }
     }
 
@@ -153,11 +180,11 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         }
     }
 
-    /** Determines if the fill operation can load more messages. */
-    internal bool fill_complete { get; set; default = false; }
+    // The base folder as a remote, if it is remote-backed
+    private RemoteFolder? remote = null;
 
     // Determines if the base folder was actually opened or not
-    private bool base_was_opened = false;
+    private bool remote_started_monitoring = false;
 
     // Set of in-folder email that doesn't meet required_fields (yet)
     private Gee.Set<EmailIdentifier> incomplete =
@@ -290,15 +317,16 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
      *
      * @param base_folder a Folder to monitor for conversations
      * @param required_fields See {@link Geary.Folder}
-     * @param min_window_count Minimum number of conversations that
+     * @param minimum_window_size Minimum number of conversations that
      * will be loaded
      */
     public ConversationMonitor(Folder base_folder,
                                Email.Field required_fields,
-                               int min_window_count) {
+                               int minimum_window_size) {
         this.base_folder = base_folder;
+        this.remote = base_folder as RemoteFolder;
         this.required_fields = required_fields | REQUIRED_FIELDS;
-        this._min_window_count = min_window_count;
+        this.minimum_window_size = minimum_window_size;
         this.conversations = new ConversationSet(base_folder);
         this.operation_cancellable = new Cancellable();
         this.queue = new ConversationOperationQueue(this.progress_monitor);
@@ -323,7 +351,7 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
 
         // Set early yield to guard against reentrancy
         this.is_monitoring = true;
-        this.base_was_opened = false;
+        this.remote_started_monitoring = false;
 
         var account = this.base_folder.account;
         account.email_appended_to_folder.connect(on_email_appended);
@@ -333,7 +361,6 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         account.email_complete.connect(on_email_complete);
 
         this.queue.operation_error.connect(on_operation_error);
-        this.queue.add(new FillWindowOperation(this));
 
         // Take the union of the two cancellables so that of the
         // monitor is closed while it is opening, the folder open is
@@ -344,14 +371,16 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
             cancellable.cancelled.connect(() => opening.cancel());
         }
 
-        var remote = this.base_folder as RemoteFolder;
-        if (remote != null && !remote.is_monitoring) {
-            remote.start_monitoring();
-            this.base_was_opened = true;
+        if (this.remote != null) {
+            this.remote.notify["is-connected"].connect(on_remote_connected);
+            if (!this.remote.is_monitoring) {
+                this.remote.start_monitoring();
+                this.remote_started_monitoring = true;
+            }
         }
 
-        // Now the folder is open, start the queue running. The here
-        // is needed for the same reason as the one immediately above.
+        this.check_window_count.begin();
+
         if (this.is_monitoring) {
             this.queue.run_process_async.begin();
         }
@@ -388,9 +417,12 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
             // Cancel outstanding ops so they don't block the queue closing
             this.operation_cancellable.cancel();
 
-            if (this.base_was_opened) {
-                ((Geary.RemoteFolder) this.base_folder).stop_monitoring();
-                this.base_was_opened = false;
+            if (this.remote != null) {
+                this.remote.notify["is-connected"].disconnect(on_remote_connected);
+                if (this.remote_started_monitoring) {
+                    this.remote.stop_monitoring();
+                    this.remote_started_monitoring = false;
+                }
             }
 
             is_closing = true;
@@ -398,6 +430,17 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         return is_closing;
     }
 
+    /**
+     * Set the minimum conversation window size.
+     *
+     * Calling this will cause the monitor to load more conversations
+     * if the current window size is less than the new minimum.
+     */
+    public void update_minimum_window_size(int size) {
+        this.minimum_window_size = size;
+        this.check_window_count.begin();
+    }
+
     /** Ensures the given email are loaded in the monitor. */
     public async void load_email(Gee.Collection<Geary.EmailIdentifier> to_load,
                                  GLib.Cancellable? cancellable)
@@ -431,20 +474,38 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
     public Logging.State to_logging_state() {
         return new Logging.State(
             this,
-            "size=%d, min_window_count=%u, can_load_more=%s, should_load_more=%s",
+            "size=%d, minimum_window_size=%u, can_load_more_local=%s, can_load_more_remote=%s, 
should_load_more=%s",
             this.size,
-            this.min_window_count,
-            this.can_load_more.to_string(),
+            this.minimum_window_size,
+            this.can_load_more_local.to_string(),
+            this.can_load_more_remote.to_string(),
             this.should_load_more.to_string()
         );
     }
 
     /** Ensures enough conversations are present, otherwise loads more. */
-    internal void check_window_count() {
-        if (this.is_monitoring &&
-            this.can_load_more &&
-            this.should_load_more) {
-            this.queue.add(new FillWindowOperation(this));
+    internal async void check_window_count() {
+        if (this.is_monitoring && this.should_load_more) {
+            if (this.can_load_more_local) {
+                this.queue.add(new FillWindowOperation(this));
+            } else {
+                // Load more email from the remote if it is connected,
+                // and there is more email on the remote to be loaded
+                if (this.can_load_more_remote) {
+                    int missing = this.minimum_window_size - this.conversations.size;
+                    debug("Expanding vector by %d email", missing);
+                    try {
+                        yield remote.expand_vector(
+                            null, missing, operation_cancellable
+                        );
+                    } catch (GLib.IOError.CANCELLED err) {
+                        // fine
+                    } catch (GLib.Error err) {
+                        debug("Error expanding vector for conversation: %s",
+                              err.message);
+                    }
+                }
+            }
         }
     }
 
@@ -809,6 +870,10 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
               needed_message_ids.size, needed_messages.size);
     }
 
+    private void on_remote_connected() {
+        this.check_window_count.begin();
+    }
+
     private void on_email_appended(Gee.Collection<EmailIdentifier> appended,
                                    Folder folder) {
         if (folder == this.base_folder) {
diff --git a/src/engine/app/conversation-monitor/app-append-operation.vala 
b/src/engine/app/conversation-monitor/app-append-operation.vala
index f8960a008..fb62cb727 100644
--- a/src/engine/app/conversation-monitor/app-append-operation.vala
+++ b/src/engine/app/conversation-monitor/app-append-operation.vala
@@ -20,6 +20,10 @@ private class Geary.App.AppendOperation : BatchOperation<EmailIdentifier> {
               batch.size, this.monitor.base_folder.to_string());
 
         yield this.monitor.load_by_sparse_id(batch);
+
+        // Check the window in case the remote is being expanded and
+        // there is more email that can subsequently be pulled down.
+        yield this.monitor.check_window_count();
     }
 
 }
diff --git a/src/engine/app/conversation-monitor/app-fill-window-operation.vala 
b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
index 187aa77da..d68af8afd 100644
--- a/src/engine/app/conversation-monitor/app-fill-window-operation.vala
+++ b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
@@ -29,7 +29,7 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
 
     public override async void execute_async() throws Error {
         int num_to_load = (int) (
-            (this.monitor.min_window_count - this.monitor.conversations.size)
+            (this.monitor.minimum_window_size - this.monitor.conversations.size)
         );
         if (num_to_load < MIN_FILL_COUNT) {
             num_to_load = MIN_FILL_COUNT;
@@ -56,12 +56,10 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
         );
 
         if (loaded == num_to_load) {
-            // Loaded the maximum number of messages, so go see if
-            // there are any more needed.
-            this.monitor.check_window_count();
-        } else {
-            this.monitor.fill_complete = true;
+            // Loaded either no email, or the maximum number of email,
+            // so go see if there are any more needed.
+            yield this.monitor.check_window_count();
         }
-
     }
+
 }
diff --git a/src/engine/app/conversation-monitor/app-insert-operation.vala 
b/src/engine/app/conversation-monitor/app-insert-operation.vala
index 3ee8fac56..7c34d74d9 100644
--- a/src/engine/app/conversation-monitor/app-insert-operation.vala
+++ b/src/engine/app/conversation-monitor/app-insert-operation.vala
@@ -43,5 +43,9 @@ private class Geary.App.InsertOperation : BatchOperation<EmailIdentifier> {
             debug("Inserting no messages into %s, none needed",
                   this.monitor.base_folder.to_string());
         }
+
+        // Check the window in case the remote is being expanded and
+        // there is more email that can subsequently be pulled down.
+        yield this.monitor.check_window_count();
     }
 }
diff --git a/src/engine/app/conversation-monitor/app-remove-operation.vala 
b/src/engine/app/conversation-monitor/app-remove-operation.vala
index f977e9023..c0f56b550 100644
--- a/src/engine/app/conversation-monitor/app-remove-operation.vala
+++ b/src/engine/app/conversation-monitor/app-remove-operation.vala
@@ -41,8 +41,9 @@ private class Geary.App.RemoveOperation : BatchOperation<EmailIdentifier> {
             (this.source_folder == this.monitor.base_folder) ? batch : null
         );
 
-        // Queue an update since many emails may have been removed
-        this.monitor.check_window_count();
+        // Check the window since many emails may have been removed
+        // and hence more are needed to fill it back up.
+        yield this.monitor.check_window_count();
     }
 
 }