3 files changed, 119 insertions, 60 deletions
diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
index e45bcae..31c9be9 100644
--- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
+++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
@@ -89,6 +89,7 @@ class ChatsManagerTest : public CppUnit::TestFixture {
     CPPUNIT_TEST(testChatControllerHighlightingNotificationTesting);
     CPPUNIT_TEST(testChatControllerHighlightingNotificationDeduplicateSounds);
     CPPUNIT_TEST(testChatControllerMeMessageHandling);
+	CPPUNIT_TEST(testRestartingMUCComponentCrash);
     CPPUNIT_TEST(testChatControllerMeMessageHandlingInMUC);
 
     // Carbons tests
@@ -822,6 +823,54 @@ public:
         CPPUNIT_ASSERT_EQUAL(std::string("is feeling delighted."), window->bodyFromMessage(window->lastAddedAction_));
     }
 
+   void testRestartingMUCComponentCrash() {
+        JID mucJID = JID("teaparty@rooms.wonderland.lit");
+        JID self = JID("girl@wonderland.lit/rabbithole");
+        std::string nick = "aLiCe";
+
+        MockChatWindow* window;
+
+        auto genRemoteMUCPresence = [=]() {
+            auto presence = Presence::create();
+            presence->setFrom(mucJID.withResource(nick));
+            presence->setTo(self);
+            return presence;
+        };
+
+        // User rejoins.
+        window = new MockChatWindow();
+        mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(window);
+
+        // Join room
+        {
+            auto joinRoomEvent = std::make_shared<JoinMUCUIEvent>(mucJID, boost::optional<std::string>(), nick);
+            uiEventStream_->send(joinRoomEvent);
+        }
+
+        {
+            auto firstPresence = genRemoteMUCPresence();
+            firstPresence->setType(Presence::Unavailable);
+            auto userPayload = std::make_shared<MUCUserPayload>();
+            userPayload->addItem(MUCItem(MUCOccupant::Owner, MUCOccupant::NoRole));
+            firstPresence->addPayload(userPayload);
+            stanzaChannel_->onPresenceReceived(firstPresence);
+        }
+        CPPUNIT_ASSERT_EQUAL(std::string("Couldn't enter room: Unable to enter this room."), MockChatWindow::bodyFromMessage(window->lastAddedErrorMessage_));
+
+        {
+            auto presence = genRemoteMUCPresence();
+            presence->setType(Presence::Unavailable);
+            auto userPayload = std::make_shared<MUCUserPayload>();
+            userPayload->addStatusCode(303);
+            auto item = MUCItem(MUCOccupant::Owner, self, MUCOccupant::Moderator);
+            item.nick = nick;
+            userPayload->addItem(item);
+            userPayload->addStatusCode(110);
+            presence->addPayload(userPayload);
+            stanzaChannel_->onPresenceReceived(presence);
+        }
+    }
+
     void testChatControllerMeMessageHandlingInMUC() {
         JID mucJID("mucroom@rooms.test.com");
         std::string nickname = "toodles";
diff --git a/Swift/Controllers/UnitTest/MockChatWindow.h b/Swift/Controllers/UnitTest/MockChatWindow.h
index d7942ff..054cd31 100644
--- a/Swift/Controllers/UnitTest/MockChatWindow.h
+++ b/Swift/Controllers/UnitTest/MockChatWindow.h
@@ -40,7 +40,9 @@ namespace Swift {
                 lastAddedPresence_ = message;
             }
 
-            virtual void addErrorMessage(const ChatMessage& /*message*/) {}
+            virtual void addErrorMessage(const ChatMessage& message) {
+                lastAddedErrorMessage_ = message;
+            }
             virtual void replaceMessage(const ChatMessage& /*message*/, const std::string& /*id*/, const boost::posix_time::ptime& /*time*/) {}
             virtual void replaceWithAction(const ChatMessage& /*message*/, const std::string& /*id*/, const boost::posix_time::ptime& /*time*/) {}
             virtual void replaceLastMessage(const ChatMessage& message, const TimestampBehaviour /*timestampBehaviour*/) {
@@ -134,6 +136,7 @@ namespace Swift {
             ChatMessage lastReplacedMessage_;
             ChatMessage lastAddedSystemMessage_;
             ChatMessage lastReplacedSystemMessage_;
+			ChatMessage lastAddedErrorMessage_;
             JID lastMUCInvitationJID_;
             std::vector<SecurityLabelsCatalog::Item> labels_;
             bool labelsEnabled_;
diff --git a/Swiften/MUC/MUCImpl.cpp b/Swiften/MUC/MUCImpl.cpp
index 4bab39d..ba8d03e 100644
--- a/Swiften/MUC/MUCImpl.cpp
+++ b/Swiften/MUC/MUCImpl.cpp
@@ -10,7 +10,6 @@
 
 #include <boost/bind.hpp>
 
-#include <Swiften/Base/foreach.h>
 #include <Swiften/Client/StanzaChannel.h>
 #include <Swiften/Elements/CapsInfo.h>
 #include <Swiften/Elements/Form.h>
@@ -138,7 +137,7 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
     }
 
     MUCUserPayload::ref mucPayload;
-    foreach (MUCUserPayload::ref payload, presence->getPayloads<MUCUserPayload>()) {
+    for (MUCUserPayload::ref payload : presence->getPayloads<MUCUserPayload>()) {
         if (!payload->getItems().empty() || !payload->getStatusCodes().empty()) {
             mucPayload = payload;
         }
@@ -148,11 +147,10 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
     // (i.e. we start getting non-error presence from the MUC) or not
     if (!joinSucceeded_) {
         if (presence->getType() == Presence::Error) {
-            std::string reason;
             onJoinFailed(presence->getPayload<ErrorPayload>());
             return;
         }
-        else {
+        else if (presence->getType() == Presence::Available) {
             joinSucceeded_ = true;
             presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
             if (presenceSender->getLastSentUndirectedPresence() && !isEqualExceptID(**(presenceSender->getLastSentUndirectedPresence()), *joinRequestPresence_)) {
@@ -162,6 +160,10 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
                 presenceSender->sendPresence(latestPresence);
             }
         }
+        else if (presence->getType() == Presence::Unavailable) {
+            onJoinFailed(std::shared_ptr<ErrorPayload>());
+            return;
+        }
     }
 
     std::string nick = presence->getFrom().getResource();
@@ -188,19 +190,21 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
             if (std::dynamic_pointer_cast<MUCDestroyPayload>(mucPayload->getPayload())) {
                 type = LeaveDestroy;
             }
-            else foreach (MUCUserPayload::StatusCode status, mucPayload->getStatusCodes()) {
-                if (status.code == 307) {
-                    type = LeaveKick;
-                }
-                else if (status.code == 301) {
-                    type = LeaveBan;
-                }
-                else if (status.code == 321) {
-                    type = LeaveNotMember;
-                }
-                else if (status.code == 303) {
-                    if (mucPayload->getItems().size() == 1) {
-                        newNickname = mucPayload->getItems()[0].nick;
+            else {
+                for (MUCUserPayload::StatusCode status : mucPayload->getStatusCodes()) {
+                    if (status.code == 307) {
+                        type = LeaveKick;
+                    }
+                    else if (status.code == 301) {
+                        type = LeaveBan;
+                    }
+                    else if (status.code == 321) {
+                        type = LeaveNotMember;
+                    }
+                    else if (status.code == 303) {
+                        if (mucPayload->getItems().size() == 1) {
+                            newNickname = mucPayload->getItems()[0].nick;
+                        }
                     }
                 }
             }
@@ -254,50 +258,53 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
             onOccupantJoined(result.first->second);
         }
         onOccupantPresenceChange(presence);
-    }
-    if (mucPayload && !joinComplete_) {
-        bool isLocked = false;
-        foreach (MUCUserPayload::StatusCode status, mucPayload->getStatusCodes()) {
-            if (status.code == 110) {
-                /* Simply knowing this is your presence is enough, 210 doesn't seem to be necessary. */
-                joinComplete_ = true;
-                if (ownMUCJID != presence->getFrom()) {
-                    presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
-                    ownMUCJID = presence->getFrom();
-                    presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
-                }
-            }
-            // MUC status 201: a new room has been created
-            if (status.code == 201) {
-                isLocked = true;
-                /* Room is created and locked */
-                /* Currently deal with this by making an instant room */
-                if (ownMUCJID != presence->getFrom()) {
-                    presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
-                    ownMUCJID = presence->getFrom();
-                    presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
-                }
-                if (createAsReservedIfNew) {
-                    unlocking = true;
-                    requestConfigurationForm();
+
+        if (mucPayload && !joinComplete_) {
+            bool isLocked = false;
+            for (MUCUserPayload::StatusCode status : mucPayload->getStatusCodes()) {
+                if (status.code == 110) {
+                    /* Simply knowing this is your presence is enough, 210 doesn't seem to be necessary. */
+                    joinComplete_ = true;
+                    if (ownMUCJID != presence->getFrom()) {
+                        presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
+                        ownMUCJID = presence->getFrom();
+                        presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
+                    }
                 }
-                else {
-                    // Accept default room configuration and create an instant room http://xmpp.org/extensions/xep-0045.html#createroom-instant
-                    MUCOwnerPayload::ref mucPayload(new MUCOwnerPayload());
-                    presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
-                    mucPayload->setPayload(std::make_shared<Form>(Form::SubmitType));
-                    std::shared_ptr< GenericRequest<MUCOwnerPayload> > request = std::make_shared< GenericRequest<MUCOwnerPayload> >(IQ::Set, getJID(), mucPayload, iqRouter_);
-                    request->onResponse.connect(boost::bind(&MUCImpl::handleCreationConfigResponse, this, _1, _2));
-                    request->send();
+                // MUC status 201: a new room has been created
+                if (status.code == 201) {
+                    isLocked = true;
+                    /* Room is created and locked */
+                    /* Currently deal with this by making an instant room */
+                    if (ownMUCJID != presence->getFrom()) {
+                        presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
+                        ownMUCJID = presence->getFrom();
+                        presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
+                    }
+                    if (createAsReservedIfNew) {
+                        unlocking = true;
+                        requestConfigurationForm();
+                    }
+                    else {
+                        // Accept default room configuration and create an instant room http://xmpp.org/extensions/xep-0045.html#createroom-instant
+                        MUCOwnerPayload::ref mucPayload(new MUCOwnerPayload());
+                        presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
+                        mucPayload->setPayload(std::make_shared<Form>(Form::SubmitType));
+                        std::shared_ptr< GenericRequest<MUCOwnerPayload> > request = std::make_shared< GenericRequest<MUCOwnerPayload> >(IQ::Set, getJID(), mucPayload, iqRouter_);
+                        request->onResponse.connect(boost::bind(&MUCImpl::handleCreationConfigResponse, this, _1, _2));
+                        request->send();
+                    }
                 }
             }
-        }
-        if (joinComplete_ && !isLocked) {
-            onJoinComplete(getOwnNick());
-        }
-        if (!isLocked && !isUnlocked_ && (presence->getFrom() == ownMUCJID)) {
-            isUnlocked_ = true;
-            onUnlocked();
+
+            if (joinComplete_ && !isLocked) {
+                assert(hasOccupant(getOwnNick()));
+                onJoinComplete(getOwnNick());
+            }
+            if (!isLocked && !isUnlocked_ && (presence->getFrom() == ownMUCJID)) {
+                isUnlocked_ = true;
+                onUnlocked();
+            }
         }
     }
 }
@@ -377,7 +384,7 @@ void MUCImpl::handleAffiliationListResponse(MUCAdminPayload::ref payload, ErrorP
     }
     else {
         std::vector<JID> jids;
-        foreach (MUCItem item, payload->getItems()) {
+        for (MUCItem item : payload->getItems()) {
             if (item.realJID) {
                 jids.push_back(*item.realJID);
             }