Prioritize backup thumbnail downloads for the opened conversation

2026-06-08 17:45:33 -05:00 · 2026-06-08 17:45:33 -05:00 · 82ce1ead86
commit 82ce1ead86
parent d10259eae1
5 changed files with 130 additions and 54 deletions
--- a/Signal/ConversationView/Components/CVComponentQuotedReply.swift
+++ b/Signal/ConversationView/Components/CVComponentQuotedReply.swift
@ -150,6 +150,7 @@ private class CVQuotedMessageViewAdapter: CVQuotedMessageViewDelegate {
            DependenciesBridge.shared.attachmentDownloadManager.enqueueDownloadOfAttachmentsForMessage(
                message,
                priority: .userInitiated,
+                useThumbnails: false,
                tx: tx,
            )
        }
--- a/Signal/ConversationView/ConversationViewController+CVComponentDelegate.swift
+++ b/Signal/ConversationView/ConversationViewController+CVComponentDelegate.swift
@ -181,7 +181,7 @@ extension ConversationViewController: CVComponentDelegate {

        // If any of the failed or pending downloads were enqueued by a Backup
        // restore, immediately attempt to download those attachments.
-        Task {
+        Task.detached {
            let attachmentDownloadManager = DependenciesBridge.shared.attachmentDownloadManager
            let attachmentStore = DependenciesBridge.shared.attachmentStore
            let backupAttachmentDownloadStore = DependenciesBridge.shared.backupAttachmentDownloadStore
@ -192,17 +192,22 @@ extension ConversationViewController: CVComponentDelegate {
                return
            }

-            let messageHasAnyEnqueuedBackupDownloads = db.read { tx in
+            enum DownloadTypeToEnqueue {
+                case thumbnail
+                case fullsize
+            }
+
+            let messageTypeToDownload: DownloadTypeToEnqueue? = db.read { tx in
                let referencedAttachments = attachmentStore.fetchReferencedAttachmentsOwnedByMessage(
                    messageRowId: messageRowId,
                    tx: tx,
                )

-                return referencedAttachments.contains { referencedAttachment in
+                let downloadTypes: [DownloadTypeToEnqueue] = referencedAttachments.compactMap { referencedAttachment in
                    // We only auto-download on appear if we've got a cdn number to try.
                    // The user can still manual download if there isn't one (using fallback cdn).
                    guard referencedAttachment.attachment.mediaTierInfo?.cdnNumber != nil else {
-                        return false
+                        return nil
                    }
                    // Otherwise use presence in the backup download queue to indicate
                    // downloadability; this just functionally bumps the priority so the
@ -213,22 +218,60 @@ extension ConversationViewController: CVComponentDelegate {
                        tx: tx,
                    )
                    switch enqueuedDownload?.state {
-                    case nil, .done, .ineligible:
-                        return false
+                    case .ineligible:
+                        if referencedAttachment.attachment.localRelativeFilePathThumbnail != nil {
+                            return nil
+                        }
+                        let enqueuedThumbnail = backupAttachmentDownloadStore.getEnqueuedDownload(
+                            attachmentRowId: referencedAttachment.attachment.id,
+                            thumbnail: true,
+                            tx: tx,
+                        )
+                        switch enqueuedThumbnail?.state {
+                        case .ready:
+                            return .thumbnail
+                        case .done, .ineligible, nil:
+                            // There is already a thumbnail, or never will be a thumbnail to display here.
+                            // Either way, no need to re-enqueue the thumbnail
+                            return nil
+                        }
+                    case nil, .done:
+                        return nil
                    case .ready:
-                        return true
+                        return .fullsize
                    }
                }
+
+                if downloadTypes.contains(.fullsize) {
+                    return .fullsize
+                } else if downloadTypes.contains(.thumbnail) {
+                    return .thumbnail
+                } else {
+                    return nil
+                }
            }

-            if messageHasAnyEnqueuedBackupDownloads {
+            switch messageTypeToDownload {
+            case .fullsize:
                await db.awaitableWrite { tx in
                    attachmentDownloadManager.enqueueDownloadOfAttachmentsForMessage(
                        message,
                        priority: .default,
+                        useThumbnails: false,
                        tx: tx,
                    )
                }
+            case .thumbnail:
+                await db.awaitableWrite { tx in
+                    attachmentDownloadManager.enqueueDownloadOfAttachmentsForMessage(
+                        message,
+                        priority: .default,
+                        useThumbnails: true,
+                        tx: tx,
+                    )
+                }
+            case .none:
+                break
            }
        }
    }
@ -242,6 +285,7 @@ extension ConversationViewController: CVComponentDelegate {
            attachmentDownloadManager.enqueueDownloadOfAttachmentsForMessage(
                message,
                priority: .userInitiated,
+                useThumbnails: false,
                tx: tx,
            )
        }
--- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift
+++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift
@ -127,6 +127,7 @@ public protocol AttachmentDownloadManager {
    func enqueueDownloadOfAttachmentsForMessage(
        _ message: TSMessage,
        priority: AttachmentDownloadPriority,
+        useThumbnails: Bool,
        tx: DBWriteTransaction,
    )

@ -179,7 +180,12 @@ extension AttachmentDownloadManager {
        _ message: TSMessage,
        tx: DBWriteTransaction,
    ) {
-        enqueueDownloadOfAttachmentsForMessage(message, priority: .default, tx: tx)
+        enqueueDownloadOfAttachmentsForMessage(
+            message,
+            priority: .default,
+            useThumbnails: false,
+            tx: tx,
+        )
    }

    public func enqueueDownloadOfAttachmentsForStoryMessage(
--- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift
+++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift
@ -227,6 +227,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
    public func enqueueDownloadOfAttachmentsForMessage(
        _ message: TSMessage,
        priority: AttachmentDownloadPriority,
+        useThumbnails: Bool,
        tx: DBWriteTransaction,
    ) {
        guard let messageRowId = message.sqliteRowId else {
@ -254,7 +255,12 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
            }
        }

-        enqueueDownloadOfReferencedAttachments(referencedAttachments, priority: priority, tx: tx)
+        enqueueDownloadOfReferencedAttachments(
+            referencedAttachments,
+            priority: priority,
+            useThumbnails: useThumbnails,
+            tx: tx,
+        )
    }

    public func enqueueDownloadOfAttachmentsForStoryMessage(
@ -270,7 +276,12 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
            storyMessageRowId: storyMessageRowId,
            tx: tx,
        )
-        enqueueDownloadOfReferencedAttachments(referencedAttachments, priority: priority, tx: tx)
+        enqueueDownloadOfReferencedAttachments(
+            referencedAttachments,
+            priority: priority,
+            useThumbnails: false,
+            tx: tx,
+        )
    }

    public func downloadReferencedAttachment(
@ -287,6 +298,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
            try _enqueueDownloadOfReferencedAttachment(
                referencedAttachment: referencedAttachment,
                priority: priority,
+                isThumbnail: false,
                tx: tx,
            )
        }
@ -306,6 +318,7 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
        _ = try _enqueueDownloadOfReferencedAttachment(
            referencedAttachment: referencedAttachment,
            priority: priority,
+            isThumbnail: false,
            tx: tx,
        )
    }
@ -313,51 +326,14 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
    private func _enqueueDownloadOfReferencedAttachment(
        referencedAttachment: ReferencedAttachment,
        priority: AttachmentDownloadPriority,
+        isThumbnail: Bool,
        tx: DBWriteTransaction,
    ) throws(AttachmentDownloads.Error) -> QueuedAttachmentDownloadRecord.SourceType {
-        let backupPlan = backupSettingsStore.backupPlan(tx: tx)
-        let isEligibleToDownloadFromMediaTier: Bool
-        switch backupPlan {
-        case .disabled, .disabling:
-            isEligibleToDownloadFromMediaTier = false
-        case .free:
-            // We still might attempt media tier downloads
-            // while currently free tier.
-            isEligibleToDownloadFromMediaTier = true
-        case .paid, .paidExpiringSoon, .paidAsTester:
-            isEligibleToDownloadFromMediaTier = true
+        let sourceToUse: QueuedAttachmentDownloadRecord.SourceType = if isThumbnail {
+            .mediaTierThumbnail
+        } else {
+            fullsizeSourceToUse(referencedAttachment, tx: tx)
        }
-
-        let sourceToUse: QueuedAttachmentDownloadRecord.SourceType = {
-            // We only download from the latest transit tier info.
-            let transitTierInfo = referencedAttachment.attachment.latestTransitTierInfo
-            let mediaTierInfo = referencedAttachment.attachment.mediaTierInfo
-            guard
-                let transitTierInfo,
-                let mediaTierInfo
-            else {
-                // If we don't have both there's nothing to decide
-                return mediaTierInfo == nil ? .transitTier : .mediaTierFullsize
-            }
-            if
-                isEligibleToDownloadFromMediaTier,
-                mediaTierInfo.lastDownloadAttemptTimestamp == nil
-            {
-                // If we've never tried media tier, always try that first.
-                return .mediaTierFullsize
-            } else
-            if transitTierInfo.lastDownloadAttemptTimestamp == nil {
-                // If we tried media tier and failed, try transit tier
-                // next time.
-                return .transitTier
-            } else {
-                // If both have failed fall back to default.
-                return isEligibleToDownloadFromMediaTier
-                    ? .mediaTierFullsize
-                    : .transitTier
-            }
-        }()
-
        let downloadability = downloadabilityChecker.downloadability(
            of: referencedAttachment.reference,
            priority: priority,
@ -396,17 +372,65 @@ public class AttachmentDownloadManagerImpl: AttachmentDownloadManager {
        }
    }

+    private func fullsizeSourceToUse(
+        _ referencedAttachment: ReferencedAttachment,
+        tx: DBReadTransaction,
+    ) -> QueuedAttachmentDownloadRecord.SourceType {
+        let backupPlan = backupSettingsStore.backupPlan(tx: tx)
+        let isEligibleToDownloadFromMediaTier: Bool
+        switch backupPlan {
+        case .disabled, .disabling:
+            isEligibleToDownloadFromMediaTier = false
+        case .free:
+            // We still might attempt media tier downloads
+            // while currently free tier.
+            isEligibleToDownloadFromMediaTier = true
+        case .paid, .paidExpiringSoon, .paidAsTester:
+            isEligibleToDownloadFromMediaTier = true
+        }
+
+        // We only download from the latest transit tier info.
+        let transitTierInfo = referencedAttachment.attachment.latestTransitTierInfo
+        let mediaTierInfo = referencedAttachment.attachment.mediaTierInfo
+        guard
+            let transitTierInfo,
+            let mediaTierInfo
+        else {
+            // If we don't have both there's nothing to decide
+            return mediaTierInfo == nil ? .transitTier : .mediaTierFullsize
+        }
+        if
+            isEligibleToDownloadFromMediaTier,
+            mediaTierInfo.lastDownloadAttemptTimestamp == nil
+        {
+            // If we've never tried media tier, always try that first.
+            return .mediaTierFullsize
+        } else
+        if transitTierInfo.lastDownloadAttemptTimestamp == nil {
+            // If we tried media tier and failed, try transit tier
+            // next time.
+            return .transitTier
+        } else {
+            // If both have failed fall back to default.
+            return isEligibleToDownloadFromMediaTier
+                ? .mediaTierFullsize
+                : .transitTier
+        }
+    }
+
    private func enqueueDownloadOfReferencedAttachments(
        _ referencedAttachments: [ReferencedAttachment],
        priority: AttachmentDownloadPriority,
+        useThumbnails: Bool,
        tx: DBWriteTransaction,
    ) {
        var didEnqueueAnyDownloads = false
        referencedAttachments.forEach { referencedAttachment in
            do throws(AttachmentDownloads.Error) {
-                try enqueueDownloadOfReferencedAttachment(
+                _ = try _enqueueDownloadOfReferencedAttachment(
                    referencedAttachment: referencedAttachment,
                    priority: priority,
+                    isThumbnail: useThumbnails,
                    tx: tx,
                )
                didEnqueueAnyDownloads = true
--- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift
+++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift
@ -48,6 +48,7 @@ open class AttachmentDownloadManagerMock: AttachmentDownloadManager {
    open func enqueueDownloadOfAttachmentsForMessage(
        _ message: TSMessage,
        priority: AttachmentDownloadPriority,
+        useThumbnails: Bool,
        tx: DBWriteTransaction,
    ) {
        // Do nothing