diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala index 51a963ca..3dc5ac57 100644 --- a/src/client/components/main-window.vala +++ b/src/client/components/main-window.vala @@ -429,7 +429,6 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface { this.conversations = new Geary.App.ConversationMonitor( to_select, - NO_DELAY, // Include fields for the conversation viewer as well so // conversations can be displayed without having to go // back to the db @@ -1035,11 +1034,12 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface { to_open.conversations_added.connect(on_conversation_count_changed); to_open.conversations_removed.connect(on_conversation_count_changed); - to_open.start_monitoring_async.begin( + to_open.start_monitoring.begin( + NO_DELAY, cancellable, (obj, res) => { try { - to_open.start_monitoring_async.end(res); + to_open.start_monitoring.end(res); } catch (GLib.Error err) { handle_error(to_open.base_folder.account.information, err); } @@ -1055,11 +1055,11 @@ public class MainWindow : Gtk.ApplicationWindow, Geary.BaseInterface { to_close.conversations_added.disconnect(on_conversation_count_changed); to_close.conversations_removed.disconnect(on_conversation_count_changed); - to_close.stop_monitoring_async.begin( + to_close.stop_monitoring.begin( null, (obj, res) => { try { - to_close.stop_monitoring_async.end(res); + to_close.stop_monitoring.end(res); } catch (GLib.Error err) { warning( "Error closing conversation monitor %s: %s", diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala index c3f522a1..8184da6b 100644 --- a/src/engine/app/app-conversation-monitor.vala +++ b/src/engine/app/app-conversation-monitor.vala @@ -20,7 +20,7 @@ * conversations can be loaded afterwards as needed. * * When monitoring starts via a call to {@link - * start_monitoring_async}, the folder will perform an initial + * 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 * additional scan operations to be executed as needed to fill the new @@ -138,9 +138,8 @@ public class Geary.App.ConversationMonitor : BaseObject { internal bool fill_complete { get; set; default = false; } private Geary.Email.Field required_fields; - private Geary.Folder.OpenFlags open_flags; - private ConversationOperationQueue queue = null; - private Cancellable? operation_cancellable = null; + private ConversationOperationQueue queue; + private GLib.Cancellable operation_cancellable = new GLib.Cancellable(); // Set of known, in-folder emails, explicitly loaded for the // monitor's window. This exists purely to support the window_size @@ -276,19 +275,18 @@ public class Geary.App.ConversationMonitor : BaseObject { * Creates a conversation monitor for the given folder. * * @param base_folder a Folder to monitor for conversations - * @param open_flags See {@link Geary.Folder} * @param required_fields See {@link Geary.Folder} * @param min_window_count Minimum number of conversations that will be loaded */ public ConversationMonitor(Folder base_folder, - Folder.OpenFlags open_flags, Email.Field required_fields, int min_window_count) { this.base_folder = base_folder; - this.open_flags = open_flags; this.required_fields = required_fields | REQUIRED_FIELDS; this._min_window_count = min_window_count; this.conversations = new ConversationSet(base_folder); + this.operation_cancellable = new Cancellable(); + this.queue = new ConversationOperationQueue(this.progress_monitor); } /** @@ -301,16 +299,19 @@ public class Geary.App.ConversationMonitor : BaseObject { * The //cancellable// parameter will be used when opening the * folder, but not subsequently when scanning for new messages. To * cancel any such operations, simply close the monitor via {@link - * stop_monitoring_async}. + * stop_monitoring}. + * + * @param open_flags See {@link Geary.Folder} + * @param cancellable Passed to the folder open operation */ - public async bool start_monitoring_async(Cancellable? cancellable = null) - throws Error { + public async bool start_monitoring(Folder.OpenFlags open_flags, + GLib.Cancellable? cancellable) + throws GLib.Error { if (this.is_monitoring) return false; // Set early yield to guard against reentrancy this.is_monitoring = true; - this.operation_cancellable = new Cancellable(); this.base_folder.email_appended.connect(on_folder_email_appended); this.base_folder.email_inserted.connect(on_folder_email_inserted); @@ -323,25 +324,37 @@ public class Geary.App.ConversationMonitor : BaseObject { this.base_folder.account.email_removed.connect(on_account_email_removed); this.base_folder.account.email_flags_changed.connect(on_account_email_flags_changed); - this.queue = new ConversationOperationQueue(this.progress_monitor); 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 + // also cancelled + GLib.Cancellable opening = new GLib.Cancellable(); + if (cancellable != null) { + cancellable.cancelled.connect(() => opening.cancel()); + } + this.operation_cancellable.cancelled.connect(() => opening.cancel()); + try { - yield this.base_folder.open_async(open_flags, cancellable); + yield this.base_folder.open_async(open_flags, opening); } catch (Error err) { - try { - yield stop_monitoring_internal(false, null); - } catch (Error stop_error) { - warning( - "Error cleaning up after folder open error: %s", err.message - ); + if (this.is_monitoring) { + try { + yield stop_monitoring_internal(false, null); + } catch (Error stop_error) { + warning( + "Error cleaning up after folder open error: %s", err.message + ); + } + throw err; } - throw err; } // Now the folder is open, start the queue running - this.queue.run_process_async.begin(); + if (this.is_monitoring) { + this.queue.run_process_async.begin(); + } return true; } @@ -356,11 +369,13 @@ public class Geary.App.ConversationMonitor : BaseObject { * internal monitor operations to complete, but will not prevent * attempts to close the base folder. */ - public async bool stop_monitoring_async(Cancellable? cancellable) throws Error { - if (!is_monitoring) - return false; - - return yield stop_monitoring_internal(true, cancellable); + public async bool stop_monitoring(GLib.Cancellable? cancellable) + throws GLib.Error { + bool is_closing = false; + if (this.is_monitoring) { + is_closing = yield stop_monitoring_internal(true, cancellable); + } + return is_closing; } /** @@ -603,7 +618,7 @@ public class Geary.App.ConversationMonitor : BaseObject { email_flags_changed(conversation, email); } - private async bool stop_monitoring_internal(bool close_folder, + private async bool stop_monitoring_internal(bool folder_was_opened, Cancellable? cancellable) throws Error { // set now to prevent reentrancy during yield or signal @@ -631,19 +646,15 @@ public class Geary.App.ConversationMonitor : BaseObject { } catch (Error err) { close_err = err; } - this.queue = null; - - this.operation_cancellable = null; bool closing = false; - if (close_folder) { + if (folder_was_opened) { try { // Always close the folder to prevent open leaks closing = yield this.base_folder.close_async(null); } catch (Error err) { warning("Unable to close monitored folder %s: %s", this.base_folder.to_string(), err.message); - close_err = err; } } diff --git a/test/engine/app/app-conversation-monitor-test.vala b/test/engine/app/app-conversation-monitor-test.vala index a120e714..3d10684e 100644 --- a/test/engine/app/app-conversation-monitor-test.vala +++ b/test/engine/app/app-conversation-monitor-test.vala @@ -64,7 +64,7 @@ class Geary.App.ConversationMonitorTest : TestCase { public void start_stop_monitoring() throws Error { ConversationMonitor monitor = new ConversationMonitor( - this.base_folder, Folder.OpenFlags.NONE, Email.Field.NONE, 10 + this.base_folder, Email.Field.NONE, 10 ); Cancellable test_cancellable = new Cancellable(); @@ -73,27 +73,24 @@ class Geary.App.ConversationMonitorTest : TestCase { monitor.scan_started.connect(() => { saw_scan_started = true; }); monitor.scan_completed.connect(() => { saw_scan_completed = true; }); - this.base_folder.expect_call( - "open_async", - { MockObject.int_arg(Folder.OpenFlags.NONE), test_cancellable } - ); + this.base_folder.expect_call("open_async"); this.base_folder.expect_call("list_email_by_id_async"); this.base_folder.expect_call("close_async"); - monitor.start_monitoring_async.begin( - test_cancellable, (obj, res) => { async_complete(res); } + monitor.start_monitoring.begin( + NONE, test_cancellable, (obj, res) => { async_complete(res); } ); - monitor.start_monitoring_async.end(async_result()); + monitor.start_monitoring.end(async_result()); // Process all of the async tasks arising from the open while (this.main_loop.pending()) { this.main_loop.iteration(true); } - monitor.stop_monitoring_async.begin( + monitor.stop_monitoring.begin( test_cancellable, (obj, res) => { async_complete(res); } ); - monitor.stop_monitoring_async.end(async_result()); + monitor.stop_monitoring.end(async_result()); assert_true(saw_scan_started, "scan_started not fired"); assert_true(saw_scan_completed, "scan_completed not fired"); @@ -103,18 +100,18 @@ class Geary.App.ConversationMonitorTest : TestCase { public void open_error() throws Error { ConversationMonitor monitor = new ConversationMonitor( - this.base_folder, Folder.OpenFlags.NONE, Email.Field.NONE, 10 + this.base_folder, Email.Field.NONE, 10 ); ExpectedCall open = this.base_folder .expect_call("open_async") .throws(new EngineError.SERVER_UNAVAILABLE("Mock error")); - monitor.start_monitoring_async.begin( - null, (obj, res) => { async_complete(res); } + monitor.start_monitoring.begin( + NONE, null, (obj, res) => { async_complete(res); } ); try { - monitor.start_monitoring_async.end(async_result()); + monitor.start_monitoring.end(async_result()); assert_not_reached(); } catch (Error err) { assert_error(open.throw_error, err); @@ -248,10 +245,10 @@ class Geary.App.ConversationMonitorTest : TestCase { // Close the monitor to cancel the final load so it does not // error out during later tests this.base_folder.expect_call("close_async"); - monitor.stop_monitoring_async.begin( + monitor.stop_monitoring.begin( null, (obj, res) => { async_complete(res); } ); - monitor.stop_monitoring_async.end(async_result()); + monitor.stop_monitoring.end(async_result()); } public void external_folder_message_appended() throws Error { @@ -387,7 +384,7 @@ class Geary.App.ConversationMonitorTest : TestCase { Gee.MultiMap[] related_paths = {}) throws Error { ConversationMonitor monitor = new ConversationMonitor( - this.base_folder, Folder.OpenFlags.NONE, Email.Field.NONE, 10 + this.base_folder, Email.Field.NONE, 10 ); Cancellable test_cancellable = new Cancellable(); @@ -479,10 +476,10 @@ class Geary.App.ConversationMonitorTest : TestCase { } } - monitor.start_monitoring_async.begin( - test_cancellable, (obj, res) => { async_complete(res); } + monitor.start_monitoring.begin( + NONE, test_cancellable, (obj, res) => { async_complete(res); } ); - monitor.start_monitoring_async.end(async_result()); + monitor.start_monitoring.end(async_result()); if (base_folder_email.length == 0) { wait_for_call(list_call);