From 44212b2131e958c40ca26b9f49289590d8f83145 Mon Sep 17 00:00:00 2001
From: Manuel Nickschas <sputnick@quassel-irc.org>
Date: Sun, 2 Mar 2014 21:19:34 +0100
Subject: [PATCH] Simplify clean-up-on-network-disconnect handling

Previously, disconnecting from a network would trigger lots of unnecessary
stuff: because the NetworkModel needs to remove affected IrcChannels and
IrcUsers from its model items, we would trigger (on the core side!) an
IrcUser::quit() for every known user in that network, which would then remove
itself from all channels it's in, triggering the corresponding updates in the
related SyncableObjects, which would send lots of signals to the client which
would then perform its own cleanups per IrcUser, followed by throwing away
all IrcChannels and IrcUsers in that network anyway. By the time we reached
Network::removeChansAndUsers(), everything would actually be already all
gone triggered through syncobject updates.

Except in some rare cases when there was still something left behind, triggering
the dreaded "!_ircChannel && ircChannel" assert, that users have been reporting
for years. I still haven't figured out how that could possibly happen.

In any case, the only side effect that explicit call to IrcUser::quit() was
supposed to trigger was the removal of the relevant references in NetworkModel's
items. So now we just brutally delete all IrcUsers and IrcChannels on disconnect,
and have the NetworkModel items listen to the relevant destroyed() signals so
they can do their cleanup. This saves us from sending lots of stuff over the
network, and also should fix the assert (which we've replaced by a warning now,
just in case).

Fixes #1151 and a bunch of duplicates.
---
 src/client/networkmodel.cpp | 28 ++++++++++++++++++++++++++--
 src/client/networkmodel.h   |  2 ++
 src/common/network.cpp      | 14 --------------
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/src/client/networkmodel.cpp b/src/client/networkmodel.cpp
index 10e7575f..dc985bd8 100644
--- a/src/client/networkmodel.cpp
+++ b/src/client/networkmodel.cpp
@@ -153,7 +153,7 @@ void NetworkItem::attachNetwork(Network *network)
     connect(network, SIGNAL(connectedSet(bool)),
         this, SIGNAL(networkDataChanged()));
     connect(network, SIGNAL(destroyed()),
-        this, SIGNAL(networkDataChanged()));
+        this, SLOT(onNetworkDestroyed()));
 
     emit networkDataChanged();
 }
@@ -233,6 +233,14 @@ void NetworkItem::onBeginRemoveChilds(int start, int end)
 }
 
 
+void NetworkItem::onNetworkDestroyed()
+{
+    _network = 0;
+    emit networkDataChanged();
+    removeAllChilds();
+}
+
+
 /*****************************************
 *  Fancy Buffer Items
 *****************************************/
@@ -520,6 +528,7 @@ void QueryBufferItem::setIrcUser(IrcUser *ircUser)
     }
 
     if (ircUser) {
+        connect(ircUser, SIGNAL(destroyed(QObject*)), SLOT(removeIrcUser()));
         connect(ircUser, SIGNAL(quited()), this, SLOT(removeIrcUser()));
         connect(ircUser, SIGNAL(awaySet(bool)), this, SIGNAL(dataChanged()));
         connect(ircUser, SIGNAL(encryptedSet(bool)), this, SLOT(setEncrypted(bool)));
@@ -595,10 +604,15 @@ QString ChannelBufferItem::toolTip(int column) const
 
 void ChannelBufferItem::attachIrcChannel(IrcChannel *ircChannel)
 {
-    Q_ASSERT(!_ircChannel && ircChannel);
+    if (_ircChannel) {
+        qWarning() << Q_FUNC_INFO << "IrcChannel already set; cleanup failed!?";
+        disconnect(_ircChannel, 0, this, 0);
+    }
 
     _ircChannel = ircChannel;
 
+    connect(ircChannel, SIGNAL(destroyed(QObject*)),
+        this, SLOT(ircChannelDestroyed()));
     connect(ircChannel, SIGNAL(topicSet(QString)),
         this, SLOT(setTopic(QString)));
     connect(ircChannel, SIGNAL(encryptedSet(bool)),
@@ -633,6 +647,16 @@ void ChannelBufferItem::ircChannelParted()
 }
 
 
+void ChannelBufferItem::ircChannelDestroyed()
+{
+    if (_ircChannel) {
+        _ircChannel = 0;
+        emit dataChanged();
+        removeAllChilds();
+    }
+}
+
+
 void ChannelBufferItem::join(const QList<IrcUser *> &ircUsers)
 {
     addUsersToCategory(ircUsers);
diff --git a/src/client/networkmodel.h b/src/client/networkmodel.h
index dff7fa51..8c4597d8 100644
--- a/src/client/networkmodel.h
+++ b/src/client/networkmodel.h
@@ -72,6 +72,7 @@ signals:
 
 private slots:
     void onBeginRemoveChilds(int start, int end);
+    void onNetworkDestroyed();
 
 private:
     NetworkId _networkId;
@@ -211,6 +212,7 @@ public slots:
 
 private slots:
     void ircChannelParted();
+    void ircChannelDestroyed();
 
 private:
     IrcChannel *_ircChannel;
diff --git a/src/common/network.cpp b/src/common/network.cpp
index d18f7130..09c33ff1 100644
--- a/src/common/network.cpp
+++ b/src/common/network.cpp
@@ -288,20 +288,6 @@ void Network::removeChansAndUsers()
     QList<IrcChannel *> channels = ircChannels();
     _ircChannels.clear();
 
-    foreach(IrcChannel *channel, channels) {
-        proxy()->detachObject(channel);
-        disconnect(channel, 0, this, 0);
-    }
-    foreach(IrcUser *user, users) {
-        proxy()->detachObject(user);
-        disconnect(user, 0, this, 0);
-    }
-
-    // the second loop is needed because quit can have sideffects
-    foreach(IrcUser *user, users) {
-        user->quit();
-    }
-
     qDeleteAll(users);
     qDeleteAll(channels);
 }
-- 
2.20.1