[geary/wip/728002-webkit2: 13/13] Update composer's draft manager lifecycle management.

[Michael Gratton <mjog src gnome org>]

commit 2b45ef51672da046464b2b3c9bbd19d1dd2f9fd5
Author: Michael James Gratton <mike vee net>
Date:   Mon Jan 16 15:55:38 2017 +1100

    Update composer's draft manager lifecycle management.
    
    * src/client/composer/composer-widget.vala
      (ComposerWidget::initialize_actions): Disable close-and-save by
      default, opening draft manager will enable it if needed.
      (ComposerWidget::restore_draft_state_async): Update doc comment noting
      use constraints, remove account param since we should be using the
      instance's instead. Update call site.
      (ComposerWidget::open_draft_manager_async): Don't try to close the
      manager beforehand, we should be able to manage that externally.
      (ComposerWidget::reopen_draft_manager_async): New async method to
      handle closing and reopening the draft manager in the appropriate
      order, since it's one caller is not async.
      (ComposerWidget::discard_and_exit_async): Only attempt to discard the
      draft if we have a draft manager. We need the guard since this may also
      get called even if drafts are not being saved.
      (ComposerWidget::set_header_recipients): Don't flag draft as having
      changed, these issues should only be cosmetic.
      (ComposerWidget::on_from_changed): Move to a more appropriate location
      in the source, just call ::reopen_draft_manager_async instead of trying
      to handle it here.

 src/client/application/geary-controller.vala |   19 +++--
 src/client/composer/composer-widget.vala     |  106 ++++++++++++++------------
 2 files changed, 67 insertions(+), 58 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index b151abc..1c780a0 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -2318,10 +2318,18 @@ public class GearyController : Geary.BaseObject {
             }
 
             widget = new ComposerWidget(current_account, compose_type, application.config, full, quote, 
is_draft);
-            if (is_draft) {
-                yield widget.restore_draft_state_async(current_account);
-            }
         }
+
+        Geary.EmailIdentifier? draft_id = null;
+        if (is_draft) {
+            draft_id = referred.id;
+            // Restore widget state before displaying the composer and
+            // opening the manager, so the changing widgets do not
+            // flash at the user, or make it look like the draft has
+            // changed hence triggering a redundant save
+            yield widget.restore_draft_state_async();
+        }
+
         widget.show_all();
 
         // We want to keep track of the open composer windows, so we can allow the user to cancel
@@ -2353,11 +2361,6 @@ public class GearyController : Geary.BaseObject {
         // set up its focus.
         widget.set_focus();
 
-        Geary.EmailIdentifier? draft_id = null;
-        if (is_draft) {
-            draft_id = referred.id;
-        }
-
         try {
             yield widget.open_draft_manager_async(draft_id);
         } catch (Error e) {
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index da57498..b889754 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -522,7 +522,10 @@ public class ComposerWidget : Gtk.EventBox {
 
         // Don't do this in an overridden version of the destroy
         // method, it somehow ends up in an infinite loop
-        destroy.connect(() => { close_draft_manager_async.begin(null); });
+        destroy.connect(() => {
+                if (this.draft_manager != null)
+                    close_draft_manager_async.begin(null);
+            });
     }
 
     public ComposerWidget.from_mailto(Geary.Account account, string mailto, Configuration config) {
@@ -584,6 +587,8 @@ public class ComposerWidget : Gtk.EventBox {
         insert_action_group("cmp", this.actions);
         this.header.insert_action_group("cmh", this.actions);
 
+        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(false);
+
         get_action(ACTION_UNDO).set_enabled(false);
         get_action(ACTION_REDO).set_enabled(false);
     }
@@ -614,15 +619,19 @@ public class ComposerWidget : Gtk.EventBox {
 
     /**
      * Restores the composer's widget state from its draft.
+     *
+     * This should only be called once, after the composer has been
+     * opened, and the draft manager should not be opened until after
+     * this has completed.
      */
-    public async void restore_draft_state_async(Geary.Account account) {
+    public async void restore_draft_state_async() {
         bool first_email = true;
-        
+
         foreach (Geary.RFC822.MessageID mid in this.in_reply_to) {
             Gee.MultiMap<Geary.Email, Geary.FolderPath?>? email_map;
             try {
                 email_map =
-                    yield account.local_search_message_id_async(mid, Geary.Email.Field.ENVELOPE,
+                    yield this.account.local_search_message_id_async(mid, Geary.Email.Field.ENVELOPE,
                     true, null, new Geary.EmailFlags.with(Geary.EmailFlags.DRAFT)); // TODO: Folder blacklist
             } catch (Error error) {
                 continue;
@@ -687,13 +696,6 @@ public class ComposerWidget : Gtk.EventBox {
         Geary.EmailIdentifier? editing_draft_id = null,
         Cancellable? cancellable = null)
     throws Error {
-        this.draft_save_text = "";
-        SimpleAction close_and_save = get_action(ACTION_CLOSE_AND_SAVE);
-
-        close_and_save.set_enabled(false);
-
-        yield close_draft_manager_async(cancellable);
-
         if (!this.account.information.save_drafts) {
             this.header.save_and_close_button.hide();
             return;
@@ -711,7 +713,7 @@ public class ComposerWidget : Gtk.EventBox {
         }
 
         update_draft_state();
-        close_and_save.set_enabled(true);
+        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(true);
         this.header.save_and_close_button.show();
 
         this.draft_manager.notify[Geary.App.DraftManager.PROP_DRAFT_STATE]
@@ -1307,11 +1309,23 @@ public class ComposerWidget : Gtk.EventBox {
         this.container.close_container();
     }
 
-    private async void close_draft_manager_async(Cancellable? cancellable) throws Error {
-        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(false);
-        if (this.draft_manager == null)
-            return;
+    /**
+     * Closes current draft manager, if any, then opens a new one.
+     */
+    private async void reopen_draft_manager_async(Cancellable? cancellable)
+    throws Error {
+        if (this.draft_manager != null) {
+            yield close_draft_manager_async(cancellable);
+        }
+        // XXX Need to work out what do to with any existing draft in
+        // this case. See Bug 713533.
+        yield open_draft_manager_async(null);
+    }
 
+    private async void close_draft_manager_async(Cancellable? cancellable)
+    throws Error {
+        this.draft_save_text = "";
+        get_action(ACTION_CLOSE_AND_SAVE).set_enabled(false);
         disconnect_from_draft_manager();
 
         // drop ref even if close failed
@@ -1425,13 +1439,16 @@ public class ComposerWidget : Gtk.EventBox {
         make_gui_insensitive();
         this.is_closing = true;
 
-        discard_draft();
-        if (draft_manager != null)
+        // This method can be called even if drafts are not being
+        // saved, hence we need to check the draft manager
+        if (draft_manager != null) {
+            discard_draft();
             draft_manager.discard_on_close = true;
-        try {
-            yield close_draft_manager_async(null);
-        } catch (Error err) {
-            // ignored
+            try {
+                yield close_draft_manager_async(null);
+            } catch (Error err) {
+                // ignored
+            }
         }
 
         this.container.close_container();
@@ -1651,8 +1668,6 @@ public class ComposerWidget : Gtk.EventBox {
                     tooltip.append(_("Reply-To: ") + addr.get_full_address() + "\n");
             this.header.set_recipients(label, tooltip.str.slice(0, -1));  // Remove trailing \n
         }
-
-        draft_changed();
     }
 
     private void on_justify(SimpleAction action, Variant? param) {
@@ -2138,32 +2153,6 @@ public class ComposerWidget : Gtk.EventBox {
         return !set_active;
     }
 
-    private void on_from_changed() {
-        bool changed = false;
-        try {
-            changed = update_from_account();
-        } catch (Error err) {
-            debug("Unable to update From: Account in composer: %s", err.message);
-        }
-
-        // if the Geary.Account didn't change and the drafts folder is open(ing), do nothing more;
-        // need to check for the drafts folder because opening it in the case of multiple From:
-        // is handled here alone, so changed open it if not already
-        if (!changed && this.draft_manager != null)
-            return;
-
-        // XXX Need to work out what do to with any existing draft in
-        // this case. See Bug 713533.
-        this.open_draft_manager_async.begin(null, null, (obj, res) => {
-                try {
-                    this.open_draft_manager_async.end(res);
-                    draft_changed();
-                } catch (Error e) {
-                    // Oh well?
-                }
-            });
-    }
-
     private bool update_from_account() throws Error {
         int index = this.from_multiple.get_active();
         if (index < 0)
@@ -2230,6 +2219,23 @@ public class ComposerWidget : Gtk.EventBox {
         update_draft_state();
     }
 
+    private void on_from_changed() {
+        bool changed = false;
+        try {
+            changed = update_from_account();
+        } catch (Error err) {
+            debug("Unable to update From: Account in composer: %s", err.message);
+        }
+
+        // if the Geary.Account didn't change and the drafts manager
+        // is open(ing), do nothing more; need to check for the drafts
+        // manager because opening it in the case of multiple From: is
+        // handled here alone, so if changed open it if not already
+        if (changed || this.draft_manager == null) {
+            reopen_draft_manager_async.begin(null);
+        }
+    }
+
     private void on_selection_changed(bool has_selection) {
         get_action(ACTION_CUT).set_enabled(has_selection);
         get_action(ACTION_COPY).set_enabled(has_selection);