diff --git a/ts/jobs/AttachmentDownloadManager.ts b/ts/jobs/AttachmentDownloadManager.ts index f4a87dfb05ea..311281249a77 100644 --- a/ts/jobs/AttachmentDownloadManager.ts +++ b/ts/jobs/AttachmentDownloadManager.ts @@ -243,7 +243,7 @@ export class AttachmentDownloadManager { return { ...attachment, - pending: true, + pending: !this.shouldHoldOffOnStartingQueuedJobs(), }; } @@ -300,13 +300,6 @@ export class AttachmentDownloadManager { return; } - if (this.isInCall()) { - log.info( - 'AttachmentDownloadManager/_maybeStartJobs: holding off on starting new jobs; in call' - ); - return; - } - const numJobsToStart = this.getMaximumNumberOfJobsToStart(); if (numJobsToStart <= 0) { @@ -323,6 +316,17 @@ export class AttachmentDownloadManager { timestamp: Date.now(), }); + if (nextJobs.length === 0) { + return; + } + + if (this.shouldHoldOffOnStartingQueuedJobs()) { + log.info( + `AttachmentDownloadManager/_maybeStartJobs: holding off on starting ${nextJobs.length} new job(s)` + ); + return; + } + // TODO (DESKTOP-6913): if a prioritized job is selected, we will to update the // in-memory job with that information so we can handle it differently, including // e.g. downloading a thumbnail before the full-size version @@ -395,6 +399,7 @@ export class AttachmentDownloadManager { lastAttemptTimestamp: now, }); } + private getActiveJobCount(): number { return this.activeJobs.size; } @@ -428,7 +433,7 @@ export class AttachmentDownloadManager { if (this.isJobRunning(job)) { const jobIdForLogging = getJobIdForLogging(job); log.warn( - `attachmentDownloads/_addRunningJob: job ${jobIdForLogging} is already running` + `AttachmentDownloadManager/addRunningJob: job ${jobIdForLogging} is already running` ); } this.activeJobs.set(this.getJobId(job), { @@ -452,6 +457,10 @@ export class AttachmentDownloadManager { return `${messageId}.${attachmentType}.${digest}`; } + private shouldHoldOffOnStartingQueuedJobs(): boolean { + return this.isInCall(); + } + // Static methods static get instance(): AttachmentDownloadManager { if (!AttachmentDownloadManager._instance) { @@ -488,7 +497,7 @@ async function runDownloadAttachmentJob( isLastAttempt: boolean ): Promise { const jobIdForLogging = getJobIdForLogging(job); - const logId = `attachment_downloads/runDownloadAttachmentJob/${jobIdForLogging}`; + const logId = `AttachmentDownloadManager/runDownloadAttachmentJob/${jobIdForLogging}`; const message = await __DEPRECATED$getMessageById(job.messageId); @@ -511,6 +520,7 @@ async function runDownloadAttachmentJob( await addAttachmentToMessage( message, _markAttachmentAsTooBig(job.attachment), + logId, { type: job.attachmentType } ); return { status: 'finished' }; @@ -520,6 +530,7 @@ async function runDownloadAttachmentJob( await addAttachmentToMessage( message, _markAttachmentAsPermanentlyErrored(job.attachment), + logId, { type: job.attachmentType } ); @@ -530,6 +541,7 @@ async function runDownloadAttachmentJob( await addAttachmentToMessage( message, _markAttachmentAsTransientlyErrored(job.attachment), + logId, { type: job.attachmentType } ); return { status: 'finished' }; @@ -542,6 +554,7 @@ async function runDownloadAttachmentJob( ...job.attachment, pending: false, }, + logId, { type: job.attachmentType } ); return { status: 'retry' }; @@ -561,7 +574,7 @@ async function runDownloadAttachmentJobInner( const { messageId, attachment, attachmentType: type } = job; const jobIdForLogging = getJobIdForLogging(job); - const logId = `attachment_downloads/_runDownloadJobInner(${jobIdForLogging})`; + const logId = `AttachmentDownloadManager/runDownloadJobInner(${jobIdForLogging})`; if (!job || !attachment || !messageId) { throw new Error(`${logId}: Key information required for job was missing.`); @@ -590,6 +603,7 @@ async function runDownloadAttachmentJobInner( await addAttachmentToMessage( message, { ...attachment, pending: true }, + logId, { type } ); @@ -601,6 +615,7 @@ async function runDownloadAttachmentJobInner( await addAttachmentToMessage( message, omit(upgradedAttachment, ['error', 'pending']), + logId, { type, } diff --git a/ts/messageModifiers/AttachmentDownloads.ts b/ts/messageModifiers/AttachmentDownloads.ts index 53c5ba105725..fa9ff3203675 100644 --- a/ts/messageModifiers/AttachmentDownloads.ts +++ b/ts/messageModifiers/AttachmentDownloads.ts @@ -11,13 +11,14 @@ import { getAttachmentSignature, isDownloaded } from '../types/Attachment'; export async function addAttachmentToMessage( message: MessageModel | null | undefined, attachment: AttachmentType, + jobLogId: string, { type }: { type: AttachmentDownloadJobTypeType } ): Promise { if (!message) { return; } - const logPrefix = `${message.idForLogging()} (type: ${type})`; + const logPrefix = `${jobLogId}/addAttachmentToMessage`; const attachmentSignature = getAttachmentSignature(attachment); if (type === 'long-message') { diff --git a/ts/models/messages.ts b/ts/models/messages.ts index ea56e038569b..a747898ecaf9 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -47,7 +47,6 @@ import { isAciString } from '../util/isAciString'; import * as reactionUtil from '../reactions/util'; import * as Errors from '../types/errors'; import type { AttachmentType } from '../types/Attachment'; -import { isImage, isVideo } from '../types/Attachment'; import * as MIME from '../types/MIME'; import { ReadStatus } from '../messages/MessageReadStatus'; import type { SendStateByConversationId } from '../messages/MessageSendState'; @@ -96,7 +95,6 @@ import { isTitleTransitionNotification, } from '../state/selectors/message'; import type { ReactionAttributesType } from '../messageModifiers/Reactions'; -import { isInCall } from '../state/selectors/calling'; import { ReactionSource } from '../reactions/ReactionSource'; import * as LinkPreview from '../types/LinkPreview'; import { SignalService as Proto } from '../protobuf'; @@ -2371,23 +2369,18 @@ export class MessageModel extends window.Backbone.Model { // Only queue attachments for downloads if this is a story (with additional logic), or // if it's either an outgoing message or we've accepted the conversation - let shouldDownloadNow = false; - const attachments = this.get('attachments') || []; - const reduxState = window.reduxStore.getState(); - + let shouldQueueForDownload = false; if (isStory(this.attributes)) { - shouldDownloadNow = await shouldDownloadStory(conversation.attributes); + shouldQueueForDownload = await shouldDownloadStory( + conversation.attributes + ); } else { - const isVisualMediaAndUserInCall = - isInCall(reduxState) && (isImage(attachments) || isVideo(attachments)); - - shouldDownloadNow = + shouldQueueForDownload = this.hasAttachmentDownloads() && - (conversation.getAccepted() || isOutgoing(this.attributes)) && - !isVisualMediaAndUserInCall; + (conversation.getAccepted() || isOutgoing(this.attributes)); } - if (shouldDownloadNow) { + if (shouldQueueForDownload) { if (shouldUseAttachmentDownloadQueue()) { addToAttachmentDownloadQueue(idLog, this); } else {