Merge branch 'leave'

Reviewed-by: Andre Magalhaes (andrunko) <andre.magalhaes@collabora.co.uk>

7 files changed, 343 insertions, 9 deletions

diff --git a/NEWS b/NEWS
index 21c6d623..e4b0d975 100644
--- a/NEWS
+++ b/NEWS
@@ -18,7 +18,8 @@ Enhancements:
 Fixes:
  * Properly install TelepathyQt4/ConnectionManagerLowLevel.
  * A race condition causing proxies to be needlessly dropped from the factory
-   cache and hence new proxies built for a future request.
+   cache and hence new proxies built for a future request, and eventually
+   hitting an assert in onProxyInvalidated as a result
  * Memory leaks when using Connection::FeatureRoster/RosterGroups where the
    connection and roster channels were leaking.
  * fd.o#29728 - ContactManager::addGroup and removeGroup are confusing/broken.
diff --git a/TelepathyQt4/CMakeLists.txt b/TelepathyQt4/CMakeLists.txt
index 4fbb690d..d00c428a 100644
--- a/TelepathyQt4/CMakeLists.txt
+++ b/TelepathyQt4/CMakeLists.txt
@@ -20,11 +20,11 @@ set(telepathy_qt4_SRCS
     capabilities-base.cpp
     channel.cpp
     channel-class-spec.cpp
-    channel-factory.cpp
-    channel-factory.h
-    channel-request.cpp
     channel-dispatcher.cpp
     channel-dispatch-operation.cpp
+    channel-factory.cpp
+    channel-internal.h
+    channel-request.cpp
     client.cpp
     client-registrar.cpp
     client-registrar-internal.h
@@ -368,6 +368,7 @@ set(telepathy_qt4_MOC_SRCS
     account-set-internal.h
     channel.h
     channel-dispatch-operation.h
+    channel-internal.h
     channel-request.h
     client-registrar.h
     client-registrar-internal.h
diff --git a/TelepathyQt4/channel-internal.h b/TelepathyQt4/channel-internal.h
new file mode 100644
index 00000000..74e02f63
--- /dev/null
+++ b/TelepathyQt4/channel-internal.h
@@ -0,0 +1,49 @@
+/*
+ * This file is part of TelepathyQt4
+ *
+ * Copyright (C) 2010 Collabora Ltd. <http://www.collabora.co.uk/>
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef _TelepathyQt4_channel_internal_h_HEADER_GUARD_
+#define _TelepathyQt4_channel_internal_h_HEADER_GUARD_
+
+#include <TelepathyQt4/Channel>
+#include <TelepathyQt4/PendingOperation>
+
+namespace Tp
+{
+
+class TELEPATHY_QT4_NO_EXPORT Channel::PendingLeave : public PendingOperation
+{
+    Q_OBJECT
+
+public:
+    PendingLeave(const ChannelPtr &channel, const QString &message,
+            ChannelGroupChangeReason reason);
+
+private Q_SLOTS:
+    void onChanInvalidated(Tp::DBusProxy *proxy);
+    void onRemoveFinished(Tp::PendingOperation *);
+    void onMembersChanged(const Tp::Contacts &, const Tp::Contacts &, const Tp::Contacts &,
+            const Tp::Contacts &);
+    void onCloseFinished(Tp::PendingOperation *);
+};
+
+} // Tp
+
+#endif
diff --git a/TelepathyQt4/channel.cpp b/TelepathyQt4/channel.cpp
index dd2d5562..ea45ad81 100644
--- a/TelepathyQt4/channel.cpp
+++ b/TelepathyQt4/channel.cpp
@@ -20,10 +20,13 @@
  */
 
 #include <TelepathyQt4/Channel>
+#include "TelepathyQt4/channel-internal.h"
 
 #include "TelepathyQt4/_gen/cli-channel-body.hpp"
 #include "TelepathyQt4/_gen/cli-channel.moc.hpp"
 #include "TelepathyQt4/_gen/channel.moc.hpp"
+#include "TelepathyQt4/_gen/channel-internal.moc.hpp"
+
 #include "TelepathyQt4/debug-internal.h"
 
 #include "TelepathyQt4/future-internal.h"
@@ -1555,6 +1558,168 @@ PendingOperation *Channel::requestClose()
     return new PendingVoid(mPriv->baseInterface->Close(), ChannelPtr(this));
 }
 
+Channel::PendingLeave::PendingLeave(const ChannelPtr &chan, const QString &message,
+        ChannelGroupChangeReason reason)
+    : PendingOperation(chan)
+{
+    Q_ASSERT(chan->mPriv->group != NULL);
+
+    QDBusPendingCall call =
+        chan->mPriv->group->RemoveMembersWithReason(
+                UIntList() << chan->mPriv->groupSelfHandle,
+                message,
+                reason);
+
+    connect(chan.data(),
+            SIGNAL(invalidated(Tp::DBusProxy*,QString,QString)),
+            this,
+            SLOT(onChanInvalidated(Tp::DBusProxy*)));
+
+    connect(new PendingVoid(call, chan),
+            SIGNAL(finished(Tp::PendingOperation*)),
+            this,
+            SLOT(onRemoveFinished(Tp::PendingOperation*)));
+}
+
+void Channel::PendingLeave::onChanInvalidated(Tp::DBusProxy *proxy)
+{
+    if (isFinished()) {
+        return;
+    }
+
+    debug() << "Finishing PendingLeave successfully as the channel was invalidated";
+
+    setFinished();
+}
+
+void Channel::PendingLeave::onRemoveFinished(Tp::PendingOperation *op)
+{
+    if (isFinished()) {
+        return;
+    }
+
+    ChannelPtr chan = ChannelPtr::staticCast(object());
+
+    if (op->isValid()) {
+        debug() << "We left the channel" << chan->objectPath();
+
+        ContactPtr c = chan->groupSelfContact();
+
+        if (chan->groupContacts().contains(c)
+                || chan->groupLocalPendingContacts().contains(c)
+                || chan->groupRemotePendingContacts().contains(c)) {
+            debug() << "Waiting for self remove to be picked up";
+            connect(chan.data(),
+                    SIGNAL(groupMembersChanged(Tp::Contacts,Tp::Contacts,Tp::Contacts,Tp::Contacts,
+                            Tp::Channel::GroupMemberChangeDetails)),
+                    this,
+                    SLOT(onMembersChanged(Tp::Contacts,Tp::Contacts,Tp::Contacts,Tp::Contacts)));
+        } else {
+            setFinished();
+        }
+
+        return;
+    }
+
+    debug() << "Leave RemoveMembersWithReason failed with " << op->errorName() << op->errorMessage()
+        << "- falling back to Close";
+
+    // If the channel has been closed or otherwise invalidated already in this mainloop iteration,
+    // the requestClose() operation will early-succeed
+    connect(chan->requestClose(),
+            SIGNAL(finished(Tp::PendingOperation*)),
+            this,
+            SLOT(onCloseFinished(Tp::PendingOperation*)));
+}
+
+void Channel::PendingLeave::onMembersChanged(const Tp::Contacts &, const Tp::Contacts &,
+        const Tp::Contacts &, const Tp::Contacts &removed)
+{
+    if (isFinished()) {
+        return;
+    }
+
+    ChannelPtr chan = ChannelPtr::staticCast(object());
+    ContactPtr c = chan->groupSelfContact();
+
+    if (removed.contains(c)) {
+        debug() << "Leave event picked up for" << chan->objectPath();
+        setFinished();
+    }
+}
+
+void Channel::PendingLeave::onCloseFinished(Tp::PendingOperation *op)
+{
+    if (isFinished()) {
+        return;
+    }
+
+    ChannelPtr chan = ChannelPtr::staticCast(object());
+
+    if (op->isError()) {
+        warning() << "Closing the channel" << chan->objectPath()
+            << "as a fallback for leaving it failed with"
+            << op->errorName() << op->errorMessage() << "- so didn't leave";
+        setFinishedWithError(op->errorName(), op->errorMessage());
+    } else {
+        debug() << "We left (by closing) the channel" << chan->objectPath();
+        setFinished();
+    }
+}
+
+/**
+ * Start an asynchronous request to leave this channel as gracefully as possible.
+ *
+ * If leaving any more gracefully is not possible, this will revert to the same as requestClose. In
+ * particular, this will be the case for channels with no Group interface
+ * (TP_QT4_IFACE_CHANNEL_INTERFACE_GROUP not in the list returned by interfaces()).
+ *
+ * The returned PendingOperation object will signal the success or failure
+ * of this request; under normal circumstances, it can be expected to
+ * succeed.
+ *
+ * A message and a reason may be provided along with the request, which will be sent to the server
+ * if supported, which is indicated by ChannelGroupFlagMessageDepart and/or
+ * ChannelGroupFlagMessageReject.
+ *
+ * Attempting to leave again when we have already left, either by our request or forcibly, will be a
+ * no-op, with the returned PendingOperation immediately finishing successfully.
+ *
+ * \param message The message, which can be blank if desired.
+ * \param reason  A reason for leaving.
+ * \return A PendingOperation, which will emit PendingOperation::finished
+ *         when the call has finished.
+ */
+PendingOperation *Channel::requestLeave(const QString &message, ChannelGroupChangeReason reason)
+{
+    // Leaving a channel does not make sense if it is already closed,
+    // just silently Return.
+    if (!isValid()) {
+        return new PendingSuccess(ChannelPtr(this));
+    }
+
+    if (!isReady(Channel::FeatureCore)) {
+        return new PendingFailure(TP_QT4_ERROR_NOT_AVAILABLE,
+                QLatin1String("Channel::FeatureCore must be ready to leave a channel"),
+                ChannelPtr(this));
+    }
+
+    if (!interfaces().contains(TP_QT4_IFACE_CHANNEL_INTERFACE_GROUP)) {
+        return requestClose();
+    }
+
+    ContactPtr self = groupSelfContact();
+
+    if (!groupContacts().contains(self) && !groupLocalPendingContacts().contains(self)
+            && !groupRemotePendingContacts().contains(self)) {
+        debug() << "Channel::requestLeave() called for " << objectPath() <<
+            "which we aren't a member of";
+        return new PendingSuccess(ChannelPtr(this));
+    }
+
+    return new PendingLeave(ChannelPtr(this), message, reason);
+}
+
 /**
  * \name Group interface
  *
@@ -2044,7 +2209,7 @@ Channel::GroupMemberChangeDetails Channel::groupLocalPendingContactChangeInfo(
  * the user hasn't been removed from the group, an object for which
  * GroupMemberChangeDetails::isValid() Return <code>false</code> is returned.
  *
- * This method should be called only after the channel has been closed.
+ * This method should be called only after you've left the channel.
  * This is useful for getting the remove information after missing the
  * corresponding groupMembersChanged() signal, as the local user being
  * removed usually causes the remote Channel to be closed.
@@ -2059,8 +2224,10 @@ Channel::GroupMemberChangeDetails Channel::groupLocalPendingContactChangeInfo(
  */
 Channel::GroupMemberChangeDetails Channel::groupSelfContactRemoveInfo() const
 {
-    if (!isReady()) {
-        warning() << "Channel::groupSelfContactRemoveInfo() used channel not ready";
+    // Oftentimes, the channel will be closed as a result from being left - so checking a channel's
+    // self remove info when it has been closed and hence invalidated is valid
+    if (isValid() && !isReady(Channel::FeatureCore)) {
+        warning() << "Channel::groupSelfContactRemoveInfo() used before Channel::FeatureCore is ready";
     } else if (!interfaces().contains(QLatin1String(TELEPATHY_INTERFACE_CHANNEL_INTERFACE_GROUP))) {
         warning() << "Channel::groupSelfContactRemoveInfo() used with "
             "no group interface";
diff --git a/TelepathyQt4/channel.h b/TelepathyQt4/channel.h
index eb10e581..4d2019e0 100644
--- a/TelepathyQt4/channel.h
+++ b/TelepathyQt4/channel.h
@@ -75,6 +75,8 @@ public:
     ContactPtr initiatorContact() const;
 
     PendingOperation *requestClose();
+    PendingOperation *requestLeave(const QString &message = QString(),
+            ChannelGroupChangeReason reason = ChannelGroupChangeReasonNone);
 
     ChannelGroupFlags groupFlags() const;
 
@@ -236,6 +238,9 @@ private Q_SLOTS:
     void gotConferenceChannelRemovedActorContact(Tp::PendingOperation *op);
 
 private:
+    class PendingLeave;
+    friend class PendingLeave;
+
     struct Private;
     friend struct Private;
     Private *mPriv;
diff --git a/TelepathyQt4/streamed-media-channel.cpp b/TelepathyQt4/streamed-media-channel.cpp
index 2781c68f..5521468d 100644
--- a/TelepathyQt4/streamed-media-channel.cpp
+++ b/TelepathyQt4/streamed-media-channel.cpp
@@ -1087,7 +1087,7 @@ PendingStreamedMediaStreams *StreamedMediaChannel::requestStreams(
  */
 PendingOperation *StreamedMediaChannel::hangupCall()
 {
-    return requestClose();
+    return requestLeave();
 }
 
 /**
diff --git a/tests/dbus/chan-group.cpp b/tests/dbus/chan-group.cpp
index af78f803..f21fe2c5 100644
--- a/tests/dbus/chan-group.cpp
+++ b/tests/dbus/chan-group.cpp
@@ -57,6 +57,8 @@ private Q_SLOTS:
     void testCreateChannel();
     void testMCDGroup();
     void testPropertylessGroup();
+    void testLeave();
+    void testLeaveWithFallback();
 
     void cleanup();
     void cleanupTestCase();
@@ -214,7 +216,7 @@ void TestChanGroup::initTestCase()
             ContactFactory::create());
     QCOMPARE(mConn->isReady(), false);
 
-    QVERIFY(connect(mConn->lowlevel()->requestConnect(),
+    QVERIFY(connect(mConn->lowlevel()->requestConnect(Connection::FeatureSelfContact),
                     SIGNAL(finished(Tp::PendingOperation*)),
                     SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
     QCOMPARE(mLoop->exec(), 0);
@@ -401,6 +403,110 @@ void TestChanGroup::commonTest(gboolean properties)
     QCOMPARE(mChan->groupContacts().count(), 3);
 }
 
+void TestChanGroup::testLeave()
+{
+    QVariantMap request;
+    request.insert(QLatin1String(TELEPATHY_INTERFACE_CHANNEL ".ChannelType"),
+                   QLatin1String(TELEPATHY_INTERFACE_CHANNEL_TYPE_CONTACT_LIST));
+    request.insert(QLatin1String(TELEPATHY_INTERFACE_CHANNEL ".TargetHandleType"),
+                   (uint) Tp::HandleTypeGroup);
+    request.insert(QLatin1String(TELEPATHY_INTERFACE_CHANNEL ".TargetID"),
+                   QLatin1String("Cambridge"));
+
+    QVERIFY(connect(mConn->lowlevel()->ensureChannel(request),
+                    SIGNAL(finished(Tp::PendingOperation*)),
+                    SLOT(expectEnsureChannelFinished(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+    QVERIFY(mChan);
+
+    QVERIFY(connect(mChan->becomeReady(),
+                    SIGNAL(finished(Tp::PendingOperation*)),
+                    SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+    QCOMPARE(mChan->isReady(), true);
+
+    QVERIFY(connect(mChan->groupAddContacts(QList<ContactPtr>() << mConn->selfContact()),
+                    SIGNAL(finished(Tp::PendingOperation*)),
+                    SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+
+    QVERIFY(mChan->groupContacts().contains(mConn->selfContact()));
+
+    QString leaveMessage(QLatin1String("I'm sick of this lot"));
+    QVERIFY(connect(mChan->requestLeave(leaveMessage),
+                SIGNAL(finished(Tp::PendingOperation*)),
+                SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+
+    QVERIFY(!mChan->groupContacts().contains(mConn->selfContact()));
+
+    // We left gracefully, which we have details for.
+    // Unfortunately, the test CM used here ignores the message and the reason specified, so can't
+    // verify those. When the leave code was originally written however, it was able to carry out
+    // the almost-impossible mission of delivering the message and reason to the CM admirably, as
+    // verified by dbus-monitor.
+    QVERIFY(mChan->groupSelfContactRemoveInfo().isValid());
+    QVERIFY(mChan->groupSelfContactRemoveInfo().hasActor());
+    QVERIFY(mChan->groupSelfContactRemoveInfo().actor() == mConn->selfContact());
+
+    // Another leave should no-op succeed, because we aren't a member
+    QVERIFY(connect(mChan->requestLeave(leaveMessage),
+                SIGNAL(finished(Tp::PendingOperation*)),
+                SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+}
+
+void TestChanGroup::testLeaveWithFallback()
+{
+    mChanObjectPath = QString(QLatin1String("%1/ChannelForTpQt4LeaveTestFallback"))
+        .arg(mConn->objectPath());
+    QByteArray chanPathLatin1(mChanObjectPath.toLatin1());
+
+    // The text channel doesn't support removing, so will trigger the fallback
+    mChanService = TP_TESTS_TEXT_CHANNEL_GROUP(g_object_new(
+                TP_TESTS_TYPE_TEXT_CHANNEL_GROUP,
+                "connection", mConnService,
+                "object-path", chanPathLatin1.data(),
+                "detailed", TRUE,
+                "properties", TRUE,
+                NULL));
+    QVERIFY(mChanService != 0);
+
+    TpIntSet *members = tp_intset_sized_new(1);
+    tp_intset_add(members, mConn->selfHandle());
+
+    QVERIFY(tp_group_mixin_change_members(G_OBJECT(mChanService), "be there or be []",
+                members, NULL, NULL, NULL, 0, TP_CHANNEL_GROUP_CHANGE_REASON_NONE));
+
+    tp_intset_destroy(members);
+
+    mChan = Channel::create(mConn, mChanObjectPath, QVariantMap());
+    QVERIFY(mChan);
+
+    // Should fail, because not ready (and thus we can't know how to leave)
+    QVERIFY(connect(mChan->requestLeave(),
+                SIGNAL(finished(Tp::PendingOperation*)),
+                SLOT(expectFailure(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+
+    QVERIFY(connect(mChan->becomeReady(),
+                    SIGNAL(finished(Tp::PendingOperation*)),
+                    SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+    QCOMPARE(mChan->isReady(), true);
+
+    // Should now work
+    QString leaveMessage(QLatin1String("I'm sick of this lot"));
+    QVERIFY(connect(mChan->requestLeave(leaveMessage),
+                SIGNAL(finished(Tp::PendingOperation*)),
+                SLOT(expectSuccessfulCall(Tp::PendingOperation*))));
+    QCOMPARE(mLoop->exec(), 0);
+
+    // The Close fallback was triggered, so we weren't removed gracefully and the details were
+    // lost
+    QVERIFY(!mChan->groupSelfContactRemoveInfo().hasMessage());
+}
+
 void TestChanGroup::cleanup()
 {
     if (mChanService) {
@@ -408,6 +514,11 @@ void TestChanGroup::cleanup()
         mChanService = 0;
     }
 
+    // Avoid D-Bus event leak from one test case to another - I've seen this with the
+    // testCreateChannel groupMembersChanged leaking at least
+    processDBusQueue(mConn.data());
+    mChan.reset();
+
     cleanupImpl();
 }