QTimerInfoList: don't inherit from a container in an exported class

Instead use composition by using a container member; inheriting from
containers has a couple of issues:
- containers (STL and Qt) don't have virtual destructors, which means
  that deleting via a pointer to the base-class would be troublesome
- as it turns out, inheriting from containers, QList in this case,
  in an exported derived-class could lead to binary-incompatibility
  issues (see linked bug report)

Drive-by change:
- group all private members in one place
- make timerInsert() private, nothing should use it from outside the
  class anyway

Change-Id: I69843835d8c854fa4b13e8b4ba3fb69f1484f6a6
Fixes: QTBUG-111959
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

6 files changed, 44 insertions, 34 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_cf.mm b/src/corelib/kernel/qeventdispatcher_cf.mm
index 770cb87e69..b93a38c8a1 100644
--- a/src/corelib/kernel/qeventdispatcher_cf.mm
+++ b/src/corelib/kernel/qeventdispatcher_cf.mm
@@ -195,7 +195,7 @@ void QEventDispatcherCoreFoundation::startingUp()
 QEventDispatcherCoreFoundation::~QEventDispatcherCoreFoundation()
 {
     invalidateTimer();
-    qDeleteAll(m_timerInfoList);
+    m_timerInfoList.clearTimers();
 
     m_cfSocketNotifier.removeSocketNotifiers();
 }
diff --git a/src/corelib/kernel/qeventdispatcher_glib.cpp b/src/corelib/kernel/qeventdispatcher_glib.cpp
index 58a9dc5e49..1e2777b08b 100644
--- a/src/corelib/kernel/qeventdispatcher_glib.cpp
+++ b/src/corelib/kernel/qeventdispatcher_glib.cpp
@@ -335,7 +335,7 @@ QEventDispatcherGlib::~QEventDispatcherGlib()
     Q_D(QEventDispatcherGlib);
 
     // destroy all timer sources
-    qDeleteAll(d->timerSource->timerList);
+    d->timerSource->timerList.clearTimers();
     d->timerSource->timerList.~QTimerInfoList();
     g_source_destroy(&d->timerSource->source);
     g_source_unref(&d->timerSource->source);
diff --git a/src/corelib/kernel/qeventdispatcher_unix.cpp b/src/corelib/kernel/qeventdispatcher_unix.cpp
index c75b59acfe..d08548e2c4 100644
--- a/src/corelib/kernel/qeventdispatcher_unix.cpp
+++ b/src/corelib/kernel/qeventdispatcher_unix.cpp
@@ -195,7 +195,7 @@ QEventDispatcherUNIXPrivate::QEventDispatcherUNIXPrivate()
 QEventDispatcherUNIXPrivate::~QEventDispatcherUNIXPrivate()
 {
     // cleanup timers
-    qDeleteAll(timerList);
+    timerList.clearTimers();
 }
 
 void QEventDispatcherUNIXPrivate::setSocketNotifierPending(QSocketNotifier *notifier)
diff --git a/src/corelib/kernel/qtimerinfo_unix.cpp b/src/corelib/kernel/qtimerinfo_unix.cpp
index 3a823cf53e..eb36478f22 100644
--- a/src/corelib/kernel/qtimerinfo_unix.cpp
+++ b/src/corelib/kernel/qtimerinfo_unix.cpp
@@ -41,9 +41,9 @@ steady_clock::time_point QTimerInfoList::updateCurrentTime()
 */
 bool QTimerInfoList::hasPendingTimers()
 {
-    if (isEmpty())
+    if (timers.isEmpty())
         return false;
-    return updateCurrentTime() < constFirst()->timeout;
+    return updateCurrentTime() < timers.at(0)->timeout;
 }
 
 static bool byTimeout(const QTimerInfo *a, const QTimerInfo *b)
@@ -54,8 +54,8 @@ static bool byTimeout(const QTimerInfo *a, const QTimerInfo *b)
 */
 void QTimerInfoList::timerInsert(QTimerInfo *ti)
 {
-    insert(std::upper_bound(cbegin(), cend(), ti, byTimeout),
-           ti);
+    timers.insert(std::upper_bound(timers.cbegin(), timers.cend(), ti, byTimeout),
+                  ti);
 }
 
 static constexpr milliseconds roundToMillisecond(nanoseconds val)
@@ -236,12 +236,11 @@ bool QTimerInfoList::timerWait(timespec &tm)
 
     auto isWaiting = [](QTimerInfo *tinfo) { return !tinfo->activateRef; };
     // Find first waiting timer not already active
-    auto it = std::find_if(cbegin(), cend(), isWaiting);
-    if (it == cend())
+    auto it = std::find_if(timers.cbegin(), timers.cend(), isWaiting);
+    if (it == timers.cend())
         return false;
 
-    QTimerInfo *t = *it;
-    nanoseconds timeToWait = t->timeout - now;
+    nanoseconds timeToWait = (*it)->timeout - now;
     if (timeToWait > 0ns)
         tm = durationToTimespec(roundToMillisecond(timeToWait));
     else
@@ -265,7 +264,7 @@ milliseconds QTimerInfoList::remainingDuration(int timerId)
     const steady_clock::time_point now = updateCurrentTime();
 
     auto it = findTimerById(timerId);
-    if (it == cend()) {
+    if (it == timers.cend()) {
 #ifndef QT_NO_DEBUG
         qWarning("QTimerInfoList::timerRemainingTime: timer id %i not found", timerId);
 #endif
@@ -335,7 +334,7 @@ void QTimerInfoList::registerTimer(int timerId, milliseconds interval,
 bool QTimerInfoList::unregisterTimer(int timerId)
 {
     auto it = findTimerById(timerId);
-    if (it == cend())
+    if (it == timers.cend())
         return false; // id not found
 
     // set timer inactive
@@ -345,13 +344,13 @@ bool QTimerInfoList::unregisterTimer(int timerId)
     if (t->activateRef)
         *(t->activateRef) = nullptr;
     delete t;
-    erase(it);
+    timers.erase(it);
     return true;
 }
 
 bool QTimerInfoList::unregisterTimers(QObject *object)
 {
-    if (isEmpty())
+    if (timers.isEmpty())
         return false;
 
     auto associatedWith = [this](QObject *o) {
@@ -368,14 +367,14 @@ bool QTimerInfoList::unregisterTimers(QObject *object)
         };
     };
 
-    qsizetype count = removeIf(associatedWith(object));
+    qsizetype count = timers.removeIf(associatedWith(object));
     return count > 0;
 }
 
 QList<QAbstractEventDispatcher::TimerInfo> QTimerInfoList::registeredTimers(QObject *object) const
 {
     QList<QAbstractEventDispatcher::TimerInfo> list;
-    for (const QTimerInfo *const t : std::as_const(*this)) {
+    for (const auto &t : timers) {
         if (t->obj == object)
             list.emplaceBack(t->id, t->interval.count(), t->timerType);
     }
@@ -387,7 +386,7 @@ QList<QAbstractEventDispatcher::TimerInfo> QTimerInfoList::registeredTimers(QObj
 */
 int QTimerInfoList::activateTimers()
 {
-    if (qt_disable_lowpriority_timers || isEmpty())
+    if (qt_disable_lowpriority_timers || timers.isEmpty())
         return 0; // nothing to do
 
     firstTimerInfo = nullptr;
@@ -397,16 +396,16 @@ int QTimerInfoList::activateTimers()
     // Find out how many timer have expired
     auto stillActive = [&now](const QTimerInfo *t) { return now < t->timeout; };
     // Find first one still active (list is sorted by timeout)
-    auto it = std::find_if(cbegin(), cend(), stillActive);
-    auto maxCount = it - cbegin();
+    auto it = std::find_if(timers.cbegin(), timers.cend(), stillActive);
+    auto maxCount = it - timers.cbegin();
 
     int n_act = 0;
     //fire the timers.
     while (maxCount--) {
-        if (isEmpty())
+        if (timers.isEmpty())
             break;
 
-        QTimerInfo *currentTimerInfo = constFirst();
+        QTimerInfo *currentTimerInfo = timers.constFirst();
         if (now < currentTimerInfo->timeout)
             break; // no timer has expired
 
@@ -422,12 +421,12 @@ int QTimerInfoList::activateTimers()
 
         // determine next timeout time
         calculateNextTimeout(currentTimerInfo, now);
-        if (size() > 1) {
+        if (timers.size() > 1) {
             // Find where "currentTimerInfo" should be in the list so as
             // to keep the list ordered by timeout
-            auto afterCurrentIt = begin() + 1;
-            auto iter = std::upper_bound(afterCurrentIt, end(), currentTimerInfo, byTimeout);
-            currentTimerInfo = *std::rotate(begin(), afterCurrentIt, iter);
+            auto afterCurrentIt = timers.begin() + 1;
+            auto iter = std::upper_bound(afterCurrentIt, timers.end(), currentTimerInfo, byTimeout);
+            currentTimerInfo = *std::rotate(timers.begin(), afterCurrentIt, iter);
         }
 
         if (currentTimerInfo->interval > 0ms)
diff --git a/src/corelib/kernel/qtimerinfo_unix_p.h b/src/corelib/kernel/qtimerinfo_unix_p.h
index 7c23f93c27..0ee81f6967 100644
--- a/src/corelib/kernel/qtimerinfo_unix_p.h
+++ b/src/corelib/kernel/qtimerinfo_unix_p.h
@@ -34,11 +34,8 @@ struct QTimerInfo {
     QTimerInfo **activateRef; // - ref from activateTimers
 };
 
-class Q_CORE_EXPORT QTimerInfoList : public QList<QTimerInfo*>
+class Q_CORE_EXPORT QTimerInfoList
 {
-    // state variables used by activateTimers()
-    QTimerInfo *firstTimerInfo = nullptr;
-
 public:
     QTimerInfoList();
 
@@ -60,14 +57,28 @@ public:
     int activateTimers();
     bool hasPendingTimers();
 
-    QList::const_iterator findTimerById(int timerId) const
+    void clearTimers()
     {
-        auto matchesId = [timerId](const QTimerInfo *t) { return t->id == timerId; };
-        return std::find_if(cbegin(), cend(), matchesId);
+        qDeleteAll(timers);
+        timers.clear();
+    }
+
+    bool isEmpty() const { return timers.empty(); }
+
+    qsizetype size() const { return timers.size(); }
+
+    auto findTimerById(int timerId)
+    {
+        auto matchesId = [timerId](const auto &t) { return t->id == timerId; };
+        return std::find_if(timers.cbegin(), timers.cend(), matchesId);
     }
 
 private:
     std::chrono::steady_clock::time_point updateCurrentTime();
+
+    // state variables used by activateTimers()
+    QTimerInfo *firstTimerInfo = nullptr;
+    QList<QTimerInfo *> timers;
 };
 
 QT_END_NAMESPACE
diff --git a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
index fd0a4b4717..41caac281b 100644
--- a/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
+++ b/src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm
@@ -957,7 +957,7 @@ QCocoaEventDispatcher::~QCocoaEventDispatcher()
 {
     Q_D(QCocoaEventDispatcher);
 
-    qDeleteAll(d->timerInfoList);
+    d->timerInfoList.clearTimers();
     d->maybeStopCFRunLoopTimer();
     CFRunLoopRemoveSource(mainRunLoop(), d->activateTimersSourceRef, kCFRunLoopCommonModes);
     CFRelease(d->activateTimersSourceRef);