Remove autoOrientJPEG and consolidate downscaling logic

ts/linkPreviews/linkPreviewFetch.ts

@@ -609,12 +609,12 @@ export async function fetchLinkPreviewImage(
     const dataBlob = new Blob([data], {
       type: contentType,
     });
-    const { blob: xcodedDataBlob } = await scaleImageToLevel(
-      dataBlob,
+    const { blob: xcodedDataBlob } = await scaleImageToLevel({
+      fileOrBlobOrURL: dataBlob,
       contentType,
-      dataBlob.size,
-      false
-    );
+      size: dataBlob.size,
+      highQuality: false,
+    });
     const xcodedDataArrayBuffer = await blobToArrayBuffer(xcodedDataBlob);
 
     data = new Uint8Array(xcodedDataArrayBuffer);

ts/models/conversations.ts

@@ -161,6 +161,7 @@ import { deriveProfileKeyVersion } from '../util/zkgroup';
 import { incrementMessageCounter } from '../util/incrementMessageCounter';
 import OS from '../util/os/osMain';
 import { getMessageAuthorText } from '../util/getMessageAuthorText';
+import { downscaleOutgoingAttachment } from '../util/attachments';
 
 /* eslint-disable more/no-then */
 window.Whisper = window.Whisper || {};
@@ -3818,7 +3819,7 @@ export class ConversationModel extends window.Backbone
 
     // If there are link previews present in the message we shouldn't include
     // any attachments as well.
-    const attachmentsToSend = preview && preview.length ? [] : attachments;
+    let attachmentsToSend = preview && preview.length ? [] : attachments;
 
     if (preview && preview.length) {
       attachments.forEach(attachment => {
@@ -3828,6 +3829,33 @@ export class ConversationModel extends window.Backbone
       });
     }
 
+    /**
+     * At this point, all attachments have been processed and written to disk as draft
+     * attachments, via processAttachments. All transcodable images have been re-encoded
+     * via canvas to remove EXIF data. Images above the high-quality threshold size have
+     * been scaled to high-quality JPEGs.
+     *
+     * If we choose to send images in standard quality, we need to scale them down
+     * (potentially for the second time). When we do so, we also delete the current
+     * draft attachment on disk for cleanup.
+     *
+     * All draft attachments (with a path or just in-memory) will be written to disk for
+     * real in `upgradeMessageSchema`.
+     */
+    if (!sendHQImages) {
+      attachmentsToSend = await Promise.all(
+        attachmentsToSend.map(async attachment => {
+          const downscaledAttachment = await downscaleOutgoingAttachment(
+            attachment
+          );
+          if (downscaledAttachment !== attachment && attachment.path) {
+            drop(deleteAttachmentData(attachment.path));
+          }
+          return downscaledAttachment;
+        })
+      );
+    }
+
     // Here we move attachments to disk
     const attributes = await upgradeMessageSchema({
       id: generateGuid(),

ts/services/LinkPreview.ts

@@ -351,6 +351,7 @@ async function getPreview(
           type: fullSizeImage.contentType,
         }),
         fileName: title,
+        highQuality: true,
       });
 
       const data = await fileToBytes(withBlob.file);

ts/signal.ts

@@ -111,7 +111,7 @@ type MigrationsModuleType = {
   }>;
   upgradeMessageSchema: (
     attributes: MessageAttributesType,
-    options?: { maxVersion?: number; keepOnDisk?: boolean }
+    options?: { maxVersion?: number }
   ) => Promise<MessageAttributesType>;
   writeMessageAttachments: (
     message: MessageAttributesType
@@ -266,9 +266,9 @@ export function initializeMigrations({
       }),
     upgradeMessageSchema: (
       message: MessageAttributesType,
-      options: { maxVersion?: number; keepOnDisk?: boolean } = {}
+      options: { maxVersion?: number } = {}
     ) => {
-      const { maxVersion, keepOnDisk } = options;
+      const { maxVersion } = options;
 
       return MessageType.upgradeSchema(message, {
         deleteOnDisk,
@@ -283,7 +283,6 @@ export function initializeMigrations({
         writeNewAttachmentData,
         writeNewStickerData,
 
-        keepOnDisk,
         logger,
         maxVersion,
       });

ts/test-electron/util/scaleImageToLevel_test.ts

@@ -35,12 +35,12 @@ describe('scaleImageToLevel', () => {
       testCases.map(
         async ({ path, contentType, expectedWidth, expectedHeight }) => {
           const blob = await getBlob(path);
-          const scaled = await scaleImageToLevel(
-            blob,
+          const scaled = await scaleImageToLevel({
+            fileOrBlobOrURL: blob,
             contentType,
-            blob.size,
-            true
-          );
+            size: blob.size,
+            highQuality: true,
+          });
 
           const data = await loadImage(scaled.blob, { orientation: true });
           const { originalWidth: width, originalHeight: height } = data;
@@ -61,7 +61,12 @@ describe('scaleImageToLevel', () => {
       'Test setup failure: expected fixture to have EXIF data'
     );
 
-    const scaled = await scaleImageToLevel(original, IMAGE_JPEG, original.size);
+    const scaled = await scaleImageToLevel({
+      fileOrBlobOrURL: original,
+      contentType: IMAGE_JPEG,
+      size: original.size,
+      highQuality: true,
+    });
     assert.isUndefined(
       (await loadImage(scaled.blob, { meta: true, orientation: true })).exif
     );

ts/types/Message2.ts

@@ -5,7 +5,6 @@ import { isFunction, isObject, isString, omit } from 'lodash';
 
 import * as Contact from './EmbeddedContact';
 import type { AttachmentType, AttachmentWithHydratedData } from './Attachment';
-import { autoOrientJPEG } from '../util/attachments';
 import {
   captureDimensionsAndScreenshot,
   hasData,
@@ -52,7 +51,6 @@ export type ContextType = {
     height: number;
   }>;
   getRegionCode: () => string | undefined;
-  keepOnDisk?: boolean;
   logger: LoggerType;
   makeImageThumbnail: (params: {
     size: number;
@@ -369,37 +367,18 @@ export const _mapPreviewAttachments =
     return { ...message, preview };
   };
 
+const noopUpgrade = async (message: MessageAttributesType) => message;
+
 const toVersion0 = async (
   message: MessageAttributesType,
   context: ContextType
 ) => initializeSchemaVersion({ message, logger: context.logger });
+
 const toVersion1 = _withSchemaVersion({
   schemaVersion: 1,
-  upgrade: _mapAttachments(
-    async (
-      attachment: AttachmentType,
-      context,
-      options
-    ): Promise<AttachmentType> => {
-      const { deleteOnDisk, keepOnDisk } = context;
-      const rotatedAttachment = await autoOrientJPEG(
-        attachment,
-        context,
-        options
-      );
-
-      if (
-        !keepOnDisk &&
-        attachment !== rotatedAttachment &&
-        rotatedAttachment.data &&
-        attachment.path
-      ) {
-        await deleteOnDisk(attachment.path);
-      }
-
-      return rotatedAttachment;
-    }
-  ),
+  // NOOP: We no longer need to run autoOrientJPEG on incoming JPEGs since Chromium
+  // respects the EXIF orientation for us when displaying the image
+  upgrade: noopUpgrade,
 });
 const toVersion2 = _withSchemaVersion({
   schemaVersion: 2,
@@ -506,7 +485,6 @@ export const upgradeSchema = async (
     makeVideoScreenshot,
     writeNewStickerData,
     deleteOnDisk,
-    keepOnDisk,
     logger,
     maxVersion = CURRENT_SCHEMA_VERSION,
   }: ContextType
@@ -566,7 +544,6 @@ export const upgradeSchema = async (
       getImageDimensions,
       makeImageThumbnail,
       makeVideoScreenshot,
-      keepOnDisk,
       logger,
       getAbsoluteStickerPath,
       getRegionCode,
@@ -590,7 +567,6 @@ export const processNewAttachment = async (
     getImageDimensions,
     makeImageThumbnail,
     makeVideoScreenshot,
-    deleteOnDisk,
     logger,
   }: Pick<
     ContextType,
@@ -630,42 +606,16 @@ export const processNewAttachment = async (
     throw new TypeError('context.logger is required');
   }
 
-  const rotatedAttachment = await autoOrientJPEG(
-    attachment,
-    { logger },
-    {
-      isIncoming: true,
-    }
-  );
-
-  let onDiskAttachment = rotatedAttachment;
-
-  // If we rotated the attachment, then `data` will be the actual bytes of the attachment,
-  //   in memory. We want that updated attachment to go back to disk.
-  if (rotatedAttachment.data) {
-    onDiskAttachment = await migrateDataToFileSystem(rotatedAttachment, {
-      writeNewAttachmentData,
-      logger,
-    });
-
-    if (rotatedAttachment !== attachment && attachment.path) {
-      await deleteOnDisk(attachment.path);
-    }
-  }
-
-  const finalAttachment = await captureDimensionsAndScreenshot(
-    onDiskAttachment,
-    {
-      writeNewAttachmentData,
-      getAbsoluteAttachmentPath,
-      makeObjectUrl,
-      revokeObjectUrl,
-      getImageDimensions,
-      makeImageThumbnail,
-      makeVideoScreenshot,
-      logger,
-    }
-  );
+  const finalAttachment = await captureDimensionsAndScreenshot(attachment, {
+    writeNewAttachmentData,
+    getAbsoluteAttachmentPath,
+    makeObjectUrl,
+    revokeObjectUrl,
+    getImageDimensions,
+    makeImageThumbnail,
+    makeVideoScreenshot,
+    logger,
+  });
 
   return finalAttachment;
 };

ts/util/attachments.ts

@@ -3,6 +3,7 @@
 
 import { blobToArrayBuffer } from 'blob-util';
 
+import * as log from '../logging/log';
 import { scaleImageToLevel } from './scaleImageToLevel';
 import { dropNull } from './dropNull';
 import type {
@@ -10,49 +11,23 @@ import type {
   UploadedAttachmentType,
 } from '../types/Attachment';
 import { canBeTranscoded } from '../types/Attachment';
-import type { LoggerType } from '../types/Logging';
-import * as MIME from '../types/MIME';
 import * as Errors from '../types/errors';
 import * as Bytes from '../Bytes';
 
-// Upgrade steps NOTE: This step strips all EXIF metadata from JPEG images as part of
-// re-encoding the image:
-
-// When sending an image:
-// 1. During composition, images are passed through handleImageAttachment. If needed, this
-//    scales them down to high-quality (level 3).
-// 2. Draft images are then written to disk as a draft image (so there is a `path`)
-// 3. On send, the message schema is upgraded, triggering this function
-
-export async function autoOrientJPEG(
-  attachment: AttachmentType,
-  { logger }: { logger: LoggerType },
-  {
-    sendHQImages = false,
-    isIncoming = false,
-  }: {
-    sendHQImages?: boolean;
-    isIncoming?: boolean;
-  } = {}
-): Promise<AttachmentType> {
-  if (isIncoming && !MIME.isJPEG(attachment.contentType)) {
-    return attachment;
-  }
-
+// All outgoing images go through handleImageAttachment before being sent and thus have
+// already been scaled to high-quality level, stripped of exif data, and saved. This
+// should be called just before message send to downscale the attachment further if
+// needed.
+export const downscaleOutgoingAttachment = async (
+  attachment: AttachmentType
+): Promise<AttachmentType> => {
   if (!canBeTranscoded(attachment)) {
     return attachment;
   }
-  // If we haven't downloaded the attachment yet, we won't have the data.
-  // All images go through handleImageAttachment before being sent and thus have
-  // already been scaled to level, oriented, stripped of exif data, and saved
-  // in high quality format. If we want to send the image in HQ we can return
-  // the attachment as-is. Otherwise we'll have to further scale it down.
+
+  let scaleTarget: string | Blob;
   const { data, path, size } = attachment;
 
-  if (sendHQImages) {
-    return attachment;
-  }
-  let scaleTarget: string | Blob;
   if (data) {
     scaleTarget = new Blob([data], {
       type: attachment.contentType,
@@ -65,12 +40,12 @@ export async function autoOrientJPEG(
   }
 
   try {
-    const { blob: xcodedDataBlob } = await scaleImageToLevel(
-      scaleTarget,
-      attachment.contentType,
+    const { blob: xcodedDataBlob } = await scaleImageToLevel({
+      fileOrBlobOrURL: scaleTarget,
+      contentType: attachment.contentType,
       size,
-      isIncoming
-    );
+      highQuality: false,
+    });
     const xcodedDataArrayBuffer = await blobToArrayBuffer(xcodedDataBlob);
 
     // IMPORTANT: We overwrite the existing `data` `Uint8Array` losing the original
@@ -91,14 +66,14 @@ export async function autoOrientJPEG(
     return xcodedAttachment;
   } catch (error: unknown) {
     const errorString = Errors.toLogFormat(error);
-    logger.error(
-      'autoOrientJPEG: Failed to rotate/scale attachment',
+    log.error(
+      'downscaleOutgoingAttachment: Failed to scale attachment',
       errorString
     );
 
     return attachment;
   }
-}
+};
 
 export type CdnFieldsType = Pick<
   AttachmentType,

ts/util/handleEditMessage.ts

@@ -135,10 +135,7 @@ export async function handleEditMessage(
   }
 
   const upgradedEditedMessageData =
-    await window.Signal.Migrations.upgradeMessageSchema(
-      editAttributes.message,
-      { keepOnDisk: true }
-    );
+    await window.Signal.Migrations.upgradeMessageSchema(editAttributes.message);
 
   // Copies over the attachments from the main message if they're the same
   // and they have already been downloaded.

ts/util/handleImageAttachment.ts

@@ -46,6 +46,8 @@ export async function handleImageAttachment(
       : stringToMIMEType(file.type),
     fileName: file.name,
     file: processedFile,
+    // We always store draft attachments as HQ
+    highQuality: true,
   });
 
   const data = await blobToArrayBuffer(resizedBlob);
@@ -66,10 +68,12 @@ export async function autoScale({
   contentType,
   file,
   fileName,
+  highQuality,
 }: {
   contentType: MIMEType;
   file: File | Blob;
   fileName: string;
+  highQuality: boolean;
 }): Promise<{
   contentType: MIMEType;
   file: Blob;
@@ -79,12 +83,12 @@ export async function autoScale({
     return { contentType, file, fileName };
   }
 
-  const { blob, contentType: newContentType } = await scaleImageToLevel(
-    file,
+  const { blob, contentType: newContentType } = await scaleImageToLevel({
+    fileOrBlobOrURL: file,
     contentType,
-    file.size,
-    true
-  );
+    size: file.size,
+    highQuality,
+  });
 
   if (newContentType !== IMAGE_JPEG) {
     return {

ts/util/scaleImageToLevel.ts

@@ -108,12 +108,17 @@ async function getCanvasBlobAsJPEG(
   return canvasToBlob(canvas, IMAGE_JPEG, quality);
 }
 
-export async function scaleImageToLevel(
-  fileOrBlobOrURL: File | Blob | string,
-  contentType: MIMEType,
-  size: number,
-  sendAsHighQuality?: boolean
-): Promise<{
+export async function scaleImageToLevel({
+  fileOrBlobOrURL,
+  contentType,
+  size,
+  highQuality,
+}: {
+  fileOrBlobOrURL: File | Blob | string;
+  contentType: MIMEType;
+  size: number;
+  highQuality: boolean | null;
+}): Promise<{
   blob: Blob;
   contentType: MIMEType;
 }> {
@@ -134,16 +139,13 @@ export async function scaleImageToLevel(
     throw error;
   }
 
-  const level = sendAsHighQuality
-    ? MediaQualityLevels.Three
-    : getMediaQualityLevel();
+  const level = highQuality ? MediaQualityLevels.Three : getMediaQualityLevel();
   const {
     maxDimensions,
     quality,
     size: targetSize,
     thresholdSize,
   } = MEDIA_QUALITY_LEVEL_DATA.get(level) || DEFAULT_LEVEL_DATA;
-
   if (size <= thresholdSize) {
     // Always encode through canvas as a temporary fix for a library bug
     const blob: Blob = await canvasToBlob(data.image, contentType);