From 82ce1ead86ae67bebb9ebb56136528a7f785d962 Mon Sep 17 00:00:00 2001 From: Pete Walters Date: Mon, 8 Jun 2026 17:45:33 -0500 Subject: [PATCH] Prioritize backup thumbnail downloads for the opened conversation --- .../Components/CVComponentQuotedReply.swift | 1 + ...onViewController+CVComponentDelegate.swift | 60 +++++++-- .../Downloads/AttachmentDownloadManager.swift | 8 +- .../AttachmentDownloadManagerImpl.swift | 114 +++++++++++------- .../AttachmentDownloadManagerMock.swift | 1 + 5 files changed, 130 insertions(+), 54 deletions(-) diff --git a/Signal/ConversationView/Components/CVComponentQuotedReply.swift b/Signal/ConversationView/Components/CVComponentQuotedReply.swift index 8caf8a7456..94db2afb81 100644 --- 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, ) } diff --git a/Signal/ConversationView/ConversationViewController+CVComponentDelegate.swift b/Signal/ConversationView/ConversationViewController+CVComponentDelegate.swift index 9a9ea63f0b..d54cdb1c6c 100644 --- 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, ) } diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManager.swift index 45d26edbc6..e445a45546 100644 --- 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( diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerImpl.swift index 08c4b7929f..171d98ca38 100644 --- 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 diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadManagerMock.swift index 8aae56729b..a6ca812bfe 100644 --- 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