Account: only set ChangingPresence if we actually are

Previously, ChangingPresence could become True on accounts which were
not Enabled, which makes no sense: we're not changing our presence in
that case. It happened if you requested an offline presence on a
disabled account whose current RequestedPresence is something online; or
if you disabled an Enabled account whose RequestedPresence (and
CurrentPresence) was offline.

Here we change the implementation of setting RequestedPresence to only
set ChangingPresence to True if it actually takes some action. There are
situations where the presence has "changed" (maybe you changed the
message associated with being offline) but no change actually needs to
be applied.

2 files changed, 46 insertions, 7 deletions

diff --git a/src/mcd-account.c b/src/mcd-account.c
index d43a69a3..a0ddf898 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -884,20 +884,25 @@ mcd_account_request_presence_int (McdAccount *account,
         }
     }
 
-    if (changed)
-    {
-        _mcd_account_set_changing_presence (account, TRUE);
-    }
-
     if (priv->connection == NULL)
     {
         if (type >= TP_CONNECTION_PRESENCE_TYPE_AVAILABLE)
         {
+            if (changed)
+            {
+                _mcd_account_set_changing_presence (account, TRUE);
+            }
+
             _mcd_account_connection_begin (account, user_initiated);
         }
     }
     else
     {
+        if (changed)
+        {
+            _mcd_account_set_changing_presence (account, TRUE);
+        }
+
         _mcd_connection_request_presence (priv->connection,
                                           priv->req_presence_type,
 					  priv->req_presence_status,
diff --git a/tests/twisted/account-manager/presence.py b/tests/twisted/account-manager/presence.py
index 88ea6a00..5577f5a6 100755
--- a/tests/twisted/account-manager/presence.py
+++ b/tests/twisted/account-manager/presence.py
@@ -30,11 +30,45 @@ def test(q, bus, mc):
         "password": "ionstorm"}, signature='sv')
     (cm_name_ref, account) = create_fakecm_account(q, bus, mc, params)
 
-
-    # Go online with a particular presence
     presence = dbus.Struct((dbus.UInt32(cs.PRESENCE_TYPE_BUSY), 'busy',
             'Fighting conspiracies'), signature='uss')
 
+    def mk_offline(message=''):
+        return dbus.Struct((dbus.UInt32(cs.PRESENCE_TYPE_OFFLINE), 'offline',
+            message), signature='uss')
+
+    offline = mk_offline()
+
+    # While the account is disabled, pushing stuff into RequestedPresence
+    # should not make ChangingPresence become True.
+    assert not account.Properties.Get(cs.ACCOUNT, 'Enabled')
+    assert not account.Properties.Get(cs.ACCOUNT, 'ChangingPresence')
+    events = [
+        EventPattern('dbus-signal', signal='AccountPropertyChanged',
+            predicate=lambda e: 'ChangingPresence' in e.args[0]),
+        EventPattern('dbus-method-call', method='RequestConnection'),
+        ]
+    q.forbid_events(events)
+    account.Properties.Set(cs.ACCOUNT, 'RequestedPresence', presence)
+    account.Properties.Set(cs.ACCOUNT, 'RequestedPresence', offline)
+    account.Properties.Set(cs.ACCOUNT, 'RequestedPresence', presence)
+    account.Properties.Set(cs.ACCOUNT, 'RequestedPresence', offline)
+
+    # Check that changing the message associated with our requested offline
+    # presence doesn't make anything happen either.
+    account.Properties.Set(cs.ACCOUNT, 'RequestedPresence',
+        mk_offline('byeeee'))
+
+    # Enable the account; RequestedPresence is still offline, so this should
+    # have no effect on ChangingPresence.
+    account.Properties.Set(cs.ACCOUNT, 'Enabled', True)
+    account.Properties.Set(cs.ACCOUNT, 'Enabled', False)
+
+    sync_dbus(bus, q, account)
+    assert not account.Properties.Get(cs.ACCOUNT, 'ChangingPresence')
+    q.unforbid_events(events)
+
+    # Go online with a particular presence
     log = []
 
     # FIXME: using predicate for its side-effects here is weird