Avoid double close in TabSupervisor

In #5447, the `TabSupervisor` logic was changed to go through
`closeRequest` instead of `deleteLater`.

Unfortunately, this introduced a bug where we close every (most) tabs
twice due to `closeRequest` calling `close()`, which is a high-level
widget function that triggers a whole lot of events, including
`QHideEvent`s, which manifests through #5697. #5735 was a tentative fix
for the issue, but it mis-diagnosed the problem and does not actually
fix it, as shown by #5740.

This patch makes `closeRequest` call `deleteLater` instead, which is a
low-level function that doesn't trigger all those events. To make sure
slots happening on tab close are still processed, use the
`QObject::destroyed` signal instead of the custom `Tab::closed` event.

This also fixes what I believe was the missing piece in #5447:
`TabSupervisor`'s destructor calling `TabSupervisor::stop`, which in
turn sends out a bunch of signals which is never a good idea while in
the middle of destruction. Instead, `TabSupervisor`'s destructor now
simply deletes its (non-`QObject`) children.

I suspect that with `TabSupervisor`'s destructor no longer calling
`TabSupervisor::stop` the `closeRequest` API can be simplified again;
this is left for later as this PR is focused on fixing the crashes.

Supersedes #5735 and #5740.

Fixes #5697 (again).

cockatrice/src/client/tabs/abstract_tab_deck_editor.cpp

@@ -554,5 +554,5 @@ void AbstractTabDeckEditor::closeRequest(bool forced)
     }
 
     emit deckEditorClosing(this);
-    close();
+    deleteLater();
-}
+}

cockatrice/src/client/tabs/tab.cpp

@@ -44,16 +44,7 @@ void Tab::deleteCardInfoPopup(const QString &cardName)
     }
 }
 
-/**
- * Overrides the closeEvent in order to emit a close signal
- */
-void Tab::closeEvent(QCloseEvent *event)
-{
-    emit closed();
-    event->accept();
-}
-
 void Tab::closeRequest(bool /*forced*/)
 {
-    close();
+    deleteLater();
-}
+}

cockatrice/src/client/tabs/tab.h

@@ -13,12 +13,6 @@ class Tab : public QMainWindow
 signals:
     void userEvent(bool globalEvent = true);
     void tabTextChanged(Tab *tab, const QString &newTabText);
-    /**
-     * Emitted when the tab is closed (because Qt doesn't provide a built-in close signal)
-     * This signal is emitted from this class's overridden Tab::closeEvent method.
-     * Make sure any subclasses that override closeEvent still emit this signal from there.
-     */
-    void closed();
 
 protected:
     TabSupervisor *tabSupervisor;
@@ -29,7 +23,6 @@ protected:
 protected slots:
     void showCardInfoPopup(const QPoint &pos, const QString &cardName, const QString &providerId);
     void deleteCardInfoPopup(const QString &cardName);
-    void closeEvent(QCloseEvent *event) override;
 
 private:
     QString currentCardName, currentProviderId;

cockatrice/src/client/tabs/tab_game.cpp

@@ -405,7 +405,7 @@ void TabGame::closeRequest(bool forced)
 
     emit gameClosing(this);
 
-    close();
+    deleteLater();
 }
 
 void TabGame::replayNextEvent(Player::EventProcessingOptions options)

cockatrice/src/client/tabs/tab_message.cpp

@@ -89,7 +89,7 @@ QString TabMessage::getTabText() const
 void TabMessage::closeRequest(bool /*forced*/)
 {
     emit talkClosing(this);
-    close();
+    deleteLater();
 }
 
 void TabMessage::sendMessage()

cockatrice/src/client/tabs/tab_room.cpp

@@ -175,7 +175,7 @@ void TabRoom::closeRequest(bool /*forced*/)
 {
     sendRoomCommand(prepareRoomCommand(Command_LeaveRoom()));
     emit roomClosing(this);
-    close();
+    deleteLater();
 }
 
 void TabRoom::tabActivated()

cockatrice/src/client/tabs/tab_supervisor.cpp

@@ -169,7 +169,17 @@ TabSupervisor::TabSupervisor(AbstractClient *_client, QMenu *tabsMenu, QWidget *
 
 TabSupervisor::~TabSupervisor()
 {
-    stop();
+    // Note: this used to call stop(), but stop() is doing a bunch of stuff
+    // including emitting some signals and we don't want to do that in a
+    // destructor.
+
+    for (auto &localClient : localClients) {
+        localClient->deleteLater();
+    }
+    localClients.clear();
+
+    delete userInfo;
+    userInfo = nullptr;
 }
 
 void TabSupervisor::retranslateUi()
@@ -251,20 +261,6 @@ void TabSupervisor::closeEvent(QCloseEvent *event)
             event->ignore();
         }
     }
-
-    // Close the game tabs in order to make sure they store their layout.
-    QSet<int> gameTabsToRemove;
-    for (auto it = gameTabs.begin(), end = gameTabs.end(); it != end; ++it) {
-        if (it.value()->close()) {
-            gameTabsToRemove.insert(it.key());
-        } else {
-            event->ignore();
-        }
-    }
-
-    for (auto tabId : gameTabsToRemove) {
-        gameTabs.remove(tabId);
-    }
 }
 
 AbstractClient *TabSupervisor::getClient() const
@@ -495,7 +491,7 @@ void TabSupervisor::openTabVisualDeckStorage()
 {
     tabVisualDeckStorage = new TabDeckStorageVisual(this);
     myAddTab(tabVisualDeckStorage, aTabVisualDeckStorage);
-    connect(tabVisualDeckStorage, &Tab::closed, this, [this] {
+    connect(tabVisualDeckStorage, &QObject::destroyed, this, [this] {
         tabVisualDeckStorage = nullptr;
         aTabVisualDeckStorage->setChecked(false);
     });
@@ -518,7 +514,7 @@ void TabSupervisor::openTabServer()
     tabServer = new TabServer(this, client);
     connect(tabServer, &TabServer::roomJoined, this, &TabSupervisor::addRoomTab);
     myAddTab(tabServer, aTabServer);
-    connect(tabServer, &Tab::closed, this, [this] {
+    connect(tabServer, &QObject::destroyed, this, [this] {
         tabServer = nullptr;
         aTabServer->setChecked(false);
     });
@@ -543,7 +539,7 @@ void TabSupervisor::openTabAccount()
     connect(tabAccount, &TabAccount::userJoined, this, &TabSupervisor::processUserJoined);
     connect(tabAccount, &TabAccount::userLeft, this, &TabSupervisor::processUserLeft);
     myAddTab(tabAccount, aTabAccount);
-    connect(tabAccount, &Tab::closed, this, [this] {
+    connect(tabAccount, &QObject::destroyed, this, [this] {
         tabAccount = nullptr;
         aTabAccount->setChecked(false);
     });
@@ -566,7 +562,7 @@ void TabSupervisor::openTabDeckStorage()
     tabDeckStorage = new TabDeckStorage(this, client, userInfo);
     connect(tabDeckStorage, &TabDeckStorage::openDeckEditor, this, &TabSupervisor::addDeckEditorTab);
     myAddTab(tabDeckStorage, aTabDeckStorage);
-    connect(tabDeckStorage, &Tab::closed, this, [this] {
+    connect(tabDeckStorage, &QObject::destroyed, this, [this] {
         tabDeckStorage = nullptr;
         aTabDeckStorage->setChecked(false);
     });
@@ -589,7 +585,7 @@ void TabSupervisor::openTabReplays()
     tabReplays = new TabReplays(this, client, userInfo);
     connect(tabReplays, &TabReplays::openReplay, this, &TabSupervisor::openReplay);
     myAddTab(tabReplays, aTabReplays);
-    connect(tabReplays, &Tab::closed, this, [this] {
+    connect(tabReplays, &QObject::destroyed, this, [this] {
         tabReplays = nullptr;
         aTabReplays->setChecked(false);
     });
@@ -612,7 +608,7 @@ void TabSupervisor::openTabAdmin()
     tabAdmin = new TabAdmin(this, client, (userInfo->user_level() & ServerInfo_User::IsAdmin));
     connect(tabAdmin, &TabAdmin::adminLockChanged, this, &TabSupervisor::adminLockChanged);
     myAddTab(tabAdmin, aTabAdmin);
-    connect(tabAdmin, &Tab::closed, this, [this] {
+    connect(tabAdmin, &QObject::destroyed, this, [this] {
         tabAdmin = nullptr;
         aTabAdmin->setChecked(false);
     });
@@ -634,7 +630,7 @@ void TabSupervisor::openTabLog()
 {
     tabLog = new TabLog(this, client);
     myAddTab(tabLog, aTabLog);
-    connect(tabLog, &Tab::closed, this, [this] {
+    connect(tabLog, &QObject::destroyed, this, [this] {
         tabLog = nullptr;
         aTabAdmin->setChecked(false);
     });