From dccb81fcb1bc3e79cc172aad9e5be08c862b8769 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 00:49:15 +1100 Subject: [PATCH] Rework Geary.Engine lifecycle managment Now that its singleton is gone, and since it's ::open_asyc and ::close_async methods don't do anything async, just merge the former with the class's ctor and make the latter non-async. Explicitly construct an instance in Application.Client and ensure it is closed there as well instead of in the controller, for consistency. --- .../application/application-client.vala | 13 +++++ .../application/application-controller.vala | 18 +------ src/engine/api/geary-engine.vala | 50 +++++-------------- test/engine/api/geary-engine-test.vala | 8 +-- 4 files changed, 28 insertions(+), 61 deletions(-) diff --git a/src/client/application/application-client.vala b/src/client/application/application-client.vala index 3534f8cf..aa853fba 100644 --- a/src/client/application/application-client.vala +++ b/src/client/application/application-client.vala @@ -393,6 +393,7 @@ public class Application.Client : Gtk.Application { // Calls Gtk.init(), amongst other things base.startup(); + this.engine = new Geary.Engine(get_resource_directory()); this.config = new Configuration(SCHEMA_ID); this.autostart = new StartupManager( this.config, this.get_desktop_directory() @@ -460,6 +461,16 @@ public class Application.Client : Gtk.Application { } public override void shutdown() { + try { + this.engine.close(); + } catch (GLib.Error error) { + warning("Error shutting down the engine: %s", error.message); + } + + this.engine = null; + this.config = null; + this.autostart = null; + Util.Date.terminate(); Geary.Logging.clear(); base.shutdown(); @@ -731,6 +742,8 @@ public class Application.Client : Gtk.Application { if (this.controller == null || this.controller.check_open_composers()) { + this.last_active_main_window = null; + bool controller_closed = false; this.destroy_controller.begin((obj, res) => { this.destroy_controller.end(res); diff --git a/src/client/application/application-controller.vala b/src/client/application/application-controller.vala index a74ccf7d..909cade5 100644 --- a/src/client/application/application-controller.vala +++ b/src/client/application/application-controller.vala @@ -220,14 +220,11 @@ internal class Application.Controller : Geary.BaseObject { warning("Error opening GOA: %s", err.message); } - // Start the engine and load our accounts + // Start loading accounts try { - yield application.engine.open_async( - this.application.get_resource_directory(), cancellable - ); yield this.account_manager.load_accounts(cancellable); } catch (Error e) { - warning("Error opening Geary.Engine instance: %s", e.message); + warning("Error loading accounts: %s", e.message); } // Since the accounts may still be loading folders, when the @@ -369,17 +366,6 @@ internal class Application.Controller : Geary.BaseObject { debug("Error waiting at account barrier: %s", err.message); } - // Release last refs to the accounts - closing_accounts.clear(); - - // Turn off the lights and lock the door behind you - try { - debug("Closing Engine..."); - yield this.application.engine.close_async(null); - } catch (GLib.Error err) { - warning("Error closing Geary Engine instance: %s", err.message); - } - debug("Closed Application.Controller"); } diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala index c261ebd9..97b86a77 100644 --- a/src/engine/api/geary-engine.vala +++ b/src/engine/api/geary-engine.vala @@ -10,10 +10,10 @@ * Manages email account instances and their life-cycle. * * An engine represents and contains interfaces into the rest of the - * email library. Instances are initialized by calling {@link - * open_async} and closed with {@link close_async}. Use this class for - * verifying and adding {@link AccountInformation} objects to check - * and start using email accounts. + * email library. Instances are initialized by constructing them and + * closed by calling {@link close}. Also use this class for verifying + * and adding {@link AccountInformation} objects to check and start + * using email accounts. */ public class Geary.Engine : BaseObject { @@ -73,9 +73,9 @@ public class Geary.Engine : BaseObject { } /** Location of the directory containing shared resource files. */ - public File? resource_dir { get; private set; default = null; } + public File resource_dir { get; private set; } - private bool is_open = false; + private bool is_open = true; private Gee.List accounts = new Gee.ArrayList(); // Would use a `weak Endpoint` value type for this map instead of @@ -85,16 +85,6 @@ public class Geary.Engine : BaseObject { private Gee.Map shared_endpoints = new Gee.HashMap(); - /** - * Fired when the engine is opened and all the existing accounts are loaded. - */ - public signal void opened(); - - /** - * Fired when the engine is closed. - */ - public signal void closed(); - /** * Fired when an account becomes available in the engine. * @@ -113,40 +103,24 @@ public class Geary.Engine : BaseObject { /** Constructs a new engine instance. */ - public Engine() { + public Engine(GLib.File resource_dir) { Engine.initialize_library(); - } - - /** - * Initializes the engine so that accounts can be added to it. - */ - public async void open_async(GLib.File resource_dir, - GLib.Cancellable? cancellable = null) - throws GLib.Error { - if (is_open) { - throw new EngineError.ALREADY_OPEN("Already open"); - } - this.resource_dir = resource_dir; - this.is_open = true; - - opened(); } /** * Uninitializes the engine, and removes all known accounts. */ - public async void close_async(Cancellable? cancellable = null) + public void close() throws GLib.Error { if (is_open) { - foreach (var account in this.accounts) { - account_unavailable(account.information); + // Copy the collection of accounts so they can be removed + // from it + foreach (var account in traverse(this.accounts).to_linked_list()) { + remove_account(account.information); } this.accounts.clear(); - this.resource_dir = null; this.is_open = false; - - closed(); } } diff --git a/test/engine/api/geary-engine-test.vala b/test/engine/api/geary-engine-test.vala index aa98a970..4a343d65 100644 --- a/test/engine/api/geary-engine-test.vala +++ b/test/engine/api/geary-engine-test.vala @@ -44,13 +44,7 @@ class Geary.EngineTest : TestCase { this.res = this.tmp.get_child("res"); this.res.make_directory(); - this.engine = new Engine(); - this.engine.open_async.begin( - res, null, - (obj, res) => { - async_complete(res); - }); - this.engine.open_async.end(async_result()); + this.engine = new Engine(res); this.account = new AccountInformation( "test",