[geary] Ensure archived/trashed messages removed after timeout: Bug #746314

[Jim Nelson <jnelson src gnome org>]

commit 6a39622633cd441643830dcd6471e0fa8bee4471
Author: Jim Nelson <jim yorba org>
Date:   Tue Mar 17 16:27:13 2015 -0700

    Ensure archived/trashed messages removed after timeout: Bug #746314
    
    Move Revokable has 1 minute commit timeout that is cancelled under
    certain circumstances.  Bug was that the "valid" property change would
    trigger a cancel even if valid was set to true (but unchanged).
    
    This patch now checks for !valid before cancelling.  Also, the one-way
    transition from valid -> !valid is now enforced through Revokable's
    interface.

 src/engine/api/geary-revokable.vala                |   20 ++++++++++++++++++--
 .../imap-engine-revokable-committed-move.vala      |    4 ++--
 .../imap-engine/imap-engine-revokable-move.vala    |   14 +++++++++-----
 3 files changed, 29 insertions(+), 9 deletions(-)
---
diff --git a/src/engine/api/geary-revokable.vala b/src/engine/api/geary-revokable.vala
index fd4ce56..6f42737 100644
--- a/src/engine/api/geary-revokable.vala
+++ b/src/engine/api/geary-revokable.vala
@@ -23,8 +23,10 @@ public abstract class Geary.Revokable : BaseObject {
      *
      * Due to later operations or notifications, it's possible for the Revokable to go invalid
      * after being issued to the caller.
+     *
+     * @see set_invalid
      */
-    public bool valid { get; protected set; default = true; }
+    public bool valid { get; private set; default = true; }
     
     /**
      * Indicates a { link revoke_async} or { link commit_async} operation is underway.
@@ -72,7 +74,10 @@ public abstract class Geary.Revokable : BaseObject {
         // ref to this object within the event loop
         revoked.connect(cancel_timed_commit);
         committed.connect(cancel_timed_commit);
-        notify[PROP_VALID].connect(cancel_timed_commit);
+        notify[PROP_VALID].connect(() => {
+            if (!valid)
+                cancel_timed_commit();
+        });
     }
     
     ~Revokable() {
@@ -88,6 +93,17 @@ public abstract class Geary.Revokable : BaseObject {
     }
     
     /**
+     * Mark the { link Revokable} as invalid.
+     *
+     * Once invalid, a Revokable may never transit back to a valid state.
+     *
+     * @see valid
+     */
+    protected void set_invalid() {
+        valid = false;
+    }
+    
+    /**
      * Revoke (undo) the operation.
      *
      * If the call throws an Error that does not necessarily mean the { link Revokable} is
diff --git a/src/engine/imap-engine/imap-engine-revokable-committed-move.vala 
b/src/engine/imap-engine/imap-engine-revokable-committed-move.vala
index 48a03e4..b07f7e5 100644
--- a/src/engine/imap-engine/imap-engine-revokable-committed-move.vala
+++ b/src/engine/imap-engine/imap-engine-revokable-committed-move.vala
@@ -51,14 +51,14 @@ private class Geary.ImapEngine.RevokableCommittedMove : Revokable {
                 }
             }
             
-            valid = false;
+            set_invalid();
         }
     }
     
     protected override async void internal_commit_async(Cancellable? cancellable) throws Error {
         // pretty simple: already committed, so done
         notify_committed(null);
-        valid = false;
+        set_invalid();
     }
 }
 
diff --git a/src/engine/imap-engine/imap-engine-revokable-move.vala 
b/src/engine/imap-engine/imap-engine-revokable-move.vala
index b92a78c..f8ecee5 100644
--- a/src/engine/imap-engine/imap-engine-revokable-move.vala
+++ b/src/engine/imap-engine/imap-engine-revokable-move.vala
@@ -53,6 +53,9 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
                 debug("Move from %s to %s failed: %s", source.path.to_string(), destination.to_string(),
                     err.message);
             }
+        } else if (valid) {
+            debug("Not scheduling freed move revokable for %s, open_state=%s",
+                source.path.to_string(), source.get_open_state().to_string());
         }
     }
     
@@ -64,7 +67,7 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
             // valid must still be true before firing
             notify_revoked();
         } finally {
-            valid = false;
+            set_invalid();
         }
     }
     
@@ -76,7 +79,7 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
             // valid must still be true before firing
             notify_committed(new RevokableCommittedMove(account, source.path, destination, 
op.destination_uids));
         } finally {
-            valid = false;
+            set_invalid();
         }
     }
     
@@ -85,7 +88,7 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
         if (unavailable != null) {
             foreach (Folder folder in unavailable) {
                 if (folder.path.equal_to(source.path) || folder.path.equal_to(destination)) {
-                    valid = false;
+                    set_invalid();
                     
                     break;
                 }
@@ -101,7 +104,8 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
         foreach (EmailIdentifier id in ids)
             move_ids.remove((ImapDB.EmailIdentifier) id);
         
-        valid = move_ids.size > 0;
+        if (move_ids.size <= 0)
+            set_invalid();
     }
     
     private void on_source_closing(Gee.List<ReplayOperation> final_ops) {
@@ -109,7 +113,7 @@ private class Geary.ImapEngine.RevokableMove : Revokable {
             return;
         
         final_ops.add(new MoveEmailCommit(source, move_ids, destination, null));
-        valid = false;
+        set_invalid();
     }
 }