SignalHandler: optimize remove(QObject*)

Don't look up the object in m_connectionCounter three times
(contains(), value(), remove()). Instead, use find() to find it once,
and std::move(it.value()) + erase() as a take() that can report
whether the element existed in the container prior to extraction.

This also means we can port the foreach loop to a ranged for loop now
without hesitation: by erasing the element from the container before
iterating over the QObject::disconnect() calls, we ensure immunity
against a potential reentrance into the object (disconnectNotify()
calls unknown code) that changes m_connectionsCounter, which the
foreach loop was immune against (d/t taking a copy of the container
under iteration).

Change-Id: I6cf36c729f488bf1334fd344ddd0191db101d103
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Arno Rehn <a.rehn@menlosystems.com>

1 files changed, 5 insertions, 5 deletions

diff --git a/src/webchannel/signalhandler_p.h b/src/webchannel/signalhandler_p.h
index f5a378a..3431f1b 100644
--- a/src/webchannel/signalhandler_p.h
+++ b/src/webchannel/signalhandler_p.h
@@ -264,12 +264,12 @@ void SignalHandler<Receiver>::clear()
 template<class Receiver>
 void SignalHandler<Receiver>::remove(const QObject *object)
 {
-    Q_ASSERT(m_connectionsCounter.contains(object));
-    const SignalConnectionHash &connections = m_connectionsCounter.value(object);
-    foreach (const ConnectionPair &connection, connections) {
+    auto it = m_connectionsCounter.find(object);
+    Q_ASSERT(it != m_connectionsCounter.cend());
+    const SignalConnectionHash connections = std::move(it.value());
+    m_connectionsCounter.erase(it);
+    for (const ConnectionPair &connection : connections)
         QObject::disconnect(connection.first);
-    }
-    m_connectionsCounter.remove(object);
 }
 
 QT_END_NAMESPACE