Room::ensureEvent(), take 3, hopefully final

Quotient/room.cpp

@@ -79,8 +79,10 @@ using std::llround;
 namespace {
 enum EventsPlacement : int { Older = -1, Newer = 1 };
 
-struct EventPromise : QPromise<const RoomEvent*> {
+struct EventRequest {
+    EventId eventId;
     QDeadlineTimer deadline;
+    QPromise<const RoomEvent*> promise{};
 };
 }
 
@@ -152,7 +154,7 @@ class Q_DECL_HIDDEN Room::Private {
     std::optional<QString> prevBatch = QString();
     int lastRequestedHistorySize = 0;
     JobHandle<GetRoomEventsJob> eventsHistoryJob;
-    std::vector<std::pair<EventId, EventPromise>> eventPromises;
+    std::vector<EventRequest> eventRequests;
     JobHandle<GetMembersByRoomJob> allMembersJob;
     //! Map from megolm sessionId to set of eventIds
     std::unordered_map<QString, QSet<QString>> undecryptedEvents;
@@ -2337,55 +2339,59 @@ JobHandle<GetRoomEventsJob> Room::getPreviousContent(int limit, const QString& f
     return d->getPreviousContent(limit, filter);
 }
 
-QFuture<const RoomEvent*> Room::ensureEvent(const QString& eventId, std::chrono::seconds maxWaitTime)
+QFuture<const RoomEvent*> Room::ensureEvent(const QString& eventId, quint16 maxWaitSeconds)
 {
     if (auto eventIt = findInTimeline(eventId); eventIt != historyEdge())
         return makeReadyValueFuture(eventIt->event());
 
-    return d->eventPromises
-        .emplace_back(eventId, EventPromise{ {}, { maxWaitTime, Qt::VeryCoarseTimer } })
-        .second.future();
-}
-
-QStringList Room::requestedEvents() const
-{
-    auto requestedIdsView = std::views::keys(d->eventPromises);
-    QStringList evtIds(begin(requestedIdsView), end(requestedIdsView));
-    return evtIds; // TODO: use std::ranges::to when it's available from all toolchains
+    if (allHistoryLoaded())
+        return {};
+    // Request a small number of events (or whatever the ongoing request says, if there's any),
+    // to make sure checkForRequestedEvents() gets executed
+    getPreviousContent();
+    return d->eventRequests
+        .emplace_back(eventId,
+                      QDeadlineTimer{ std::chrono::seconds(maxWaitSeconds), Qt::VeryCoarseTimer })
+        .promise.future();
 }
 
 void Room::Private::checkForRequestedEvents(const rev_iter_t& from, bool allHistoryLoaded)
 {
     using namespace std::ranges;
-    std::erase_if(eventPromises, [this, from](auto& idAndPromise) {
-        auto&& [eventId, promise] = idAndPromise;
+    std::erase_if(eventRequests, [this, from](EventRequest& request) {
+        auto&& [eventId, deadline, promise] = request;
+        if (promise.isCanceled()) {
+            qCInfo(MESSAGES) << "The request to ensure event" << eventId << "has been cancelled";
+            return true;
+        }
         if (auto it = find(from, historyEdge(), eventId, &RoomEvent::id); it != historyEdge()) {
             promise.addResult(it->event());
             promise.finish();
             return true;
         }
-        if (promise.deadline.hasExpired()) {
+        if (deadline.hasExpired()) {
             qCWarning(MESSAGES) << "Timeout - giving up on obtaining event" << eventId;
             promise.future().cancel();
             return true;
         }
         return false;
     });
-    if (!eventPromises.empty()) {
+    if (!eventRequests.empty()) {
         if (allHistoryLoaded) {
-            for_each(views::values(eventPromises), [](auto& p) { p.future().cancel(); });
-            eventPromises.clear();
+            for_each(eventRequests, [](auto& r) { r.promise.future().cancel(); });
+            eventRequests.clear();
         }
         static constexpr auto EventsProgression = std::array{ 50, 100, 200, 500, 1000 };
         static_assert(is_sorted(EventsProgression));
         const auto thisMany = lastRequestedHistorySize >= EventsProgression.back()
                                   ? EventsProgression.back()
                                   : *upper_bound(EventsProgression, lastRequestedHistorySize);
+        auto requestedIdsView = std::views::transform(eventRequests, &EventRequest::eventId);
         qCDebug(MESSAGES) << "Requesting" << thisMany << "events, looking for" <<
 #if defined(__cpp_lib_ranges_join_with) && defined(__cpp_lib_ranges_to_container)
-            (views::keys(eventPromises) | views::join_with(u","_s) | to<QString>());
+            (requestedIdsView | views::join_with(u","_s) | to<QString>());
 #else
-            q->requestedEvents().join(u',');
+            QStringList(begin(requestedIdsView), end(requestedIdsView)).join(u',');
 #endif
         getPreviousContent(thisMany);
     }

Quotient/room.h

@@ -747,19 +747,19 @@ class QUOTIENT_API Room : public QObject {
 
     //! \brief Loads the message history until the specified event id is found
     //!
-    //! This is much heavier than getEventFromServer(); clients should use this sparingly.
-    //! One intended use case for that is loading the timeline until the last read event, assuming
-    //! that the last read event is not too far back and that the user will read or at least scroll
-    //! through the just loaded events anyway. This will not be necessary once we move to sliding
-    //! sync but sliding sync support is still a bit away in the future.
+    //! This is potentially heavy; clients should use this sparingly. One intended use case is
+    //! loading the timeline until the last read event, assuming that the last read event is
+    //! not too far back and that the user will read or at least scroll through the just loaded
+    //! events anyway. This will not be necessary once we move to sliding sync but sliding sync
+    //! support is still a bit away in the future.
+    //!
     //! Because the process is heavy (particularly on the homeserver), ensureEvent() will cancel
-    //! after \p maxWaitTime. It is not recommended to raise this too much.
+    //! after \p maxWaitSeconds. Clients may opt to reduce this number; it is not recommended
+    //! to increase it, as most users will give up waiting much earlier than even the default value.
     //! \return the future that resolves to the event with \p eventId, or self-cancels if the event
     //!         is not found
-    Q_INVOKABLE QFuture<const RoomEvent*> ensureEvent(
-        const QString& eventId, std::chrono::seconds maxWaitTime = std::chrono::seconds(60));
-
-    Q_INVOKABLE QStringList requestedEvents() const;
+    Q_INVOKABLE QFuture<const RoomEvent*> ensureEvent(const QString& eventId,
+                                                      quint16 maxWaitSeconds = 20);
 
 public Q_SLOTS:
     /** Check whether the room should be upgraded */