[folks] Bug 677166 â Salut personas survive disconnection

[Philip Withnall <pwithnall src gnome org>]

commit c8fa98d3ae271c56d7e760f9c2eed37585108541
Author: Philip Withnall <philip tecnocode co uk>
Date:   Mon Jun 18 11:04:57 2012 +0100

    Bug 677166 â Salut personas survive disconnection
    
    Handle TpAccounts being disabled by listening for
    TpAccountManager::account-disabled rather than (erroneously) assuming that
    TpAccount:enabled will have been changed by the time the corresponding
    TpConnection is disconnected.
    
    This fixes a race condition when accounts are disabled, where the accountâs
    personas would persist if TpAccount:enabled hadnât changed by the time the
    connection was disconnected.
    
    This comes at the cost of potentially storing and re-loading the set of
    personas for that account to the cache, only to later delete the cache file
    when TpAccount:enabled changes. I canât think of a simple fix for this.
    
    Closes: https://bugzilla.gnome.org/show_bug.cgi?id=677166

 NEWS                                          |    3 +-
 backends/telepathy/lib/tpf-persona-store.vala |   50 ++++++++++++++----------
 2 files changed, 31 insertions(+), 22 deletions(-)
---
diff --git a/NEWS b/NEWS
index bd37740..6107593 100644
--- a/NEWS
+++ b/NEWS
@@ -3,7 +3,8 @@ Overview of changes from libfolks 0.7.1 to libfolks 0.7.2
 
 Dependencies:
 
-Bugs Fixed:
+Bugs fixed:
+â Bug 677166 â Salut personas survive disconnection
 
 API changes:
 
diff --git a/backends/telepathy/lib/tpf-persona-store.vala b/backends/telepathy/lib/tpf-persona-store.vala
index 37dc447..6d3ff84 100644
--- a/backends/telepathy/lib/tpf-persona-store.vala
+++ b/backends/telepathy/lib/tpf-persona-store.vala
@@ -460,24 +460,35 @@ public class Tpf.PersonaStore : Folks.PersonaStore
               this._account_manager.invalidated.connect (
                   this._account_manager_invalidated_cb);
 
+              /* Note: For the three signal handlers below, we do *not* need to
+               * store personas to the cache before removing the store, as
+               * _remove_store() deletes the cache file. */
               this._account_manager.account_removed.connect ((a) =>
                 {
                   if (this.account == a)
                     {
+                      debug ("Account %p (â%sâ) removed.", a, a.display_name);
                       this._remove_store ();
                     }
                 });
               this._account_manager.account_validity_changed.connect (
                   (a, valid) =>
                     {
-                      debug ("Account validity changed for %p (â%sâ) to %s.",
-                          a, a.display_name, valid ? "true" : "false");
-
                       if (!valid && this.account == a)
                         {
+                          debug ("Account %p (â%sâ) invalid.", a,
+                              a.display_name);
                           this._remove_store ();
                         }
                     });
+              this._account_manager.account_disabled.connect ((a) =>
+                {
+                  if (this.account == a)
+                    {
+                      debug ("Account %p (â%sâ) disabled.", a, a.display_name);
+                      this._remove_store ();
+                    }
+                });
 
               this._favourite_ids.clear ();
               this._logger = new Logger (this.id);
@@ -633,27 +644,24 @@ public class Tpf.PersonaStore : Folks.PersonaStore
               var old_personas = this._persona_set;
               this._reset ();
 
-              this._store_cache.begin (old_personas, (o, r) =>
+              /* Only store/load the cache if the account is enabled and valid;
+               * otherwise, the PersonaStore will get removed and the cache
+               * deleted later anyway. */
+              if (this.account.enabled && this.account.valid)
                 {
-                  this._store_cache.end (r);
-
-                  this._load_cache.begin (old_personas, (o2, r2) =>
+                  this._store_cache.begin (old_personas, (o, r) =>
                     {
-                      this._load_cache.end (r2);
-                    });
-                });
-            }
+                      this._store_cache.end (r);
 
-          /* If the account was disabled, remove it. We do this here rather than
-           * in a handler for the AccountManager::account-disabled signal so
-           * that we can wait until the personas have been stored to the cache,
-           * which only happens once the account is disconnected (above). We can
-           * do this because it's guaranteed that the account will be
-           * disconnected after being disabled (if it was connected to begin
-           * with). */
-          if (this.account.enabled == false)
-            {
-              this._remove_store ();
+                      if (this.account.enabled && this.account.valid)
+                        {
+                          this._load_cache.begin (old_personas, (o2, r2) =>
+                            {
+                              this._load_cache.end (r2);
+                            });
+                        }
+                    });
+                }
             }
 
           /* If the persona store starts offline, we've reached a quiescent