Respond to CR.

SignalServiceKit/src/Contacts/Threads/TSGroupThread.m

@@ -191,7 +191,7 @@ NSString *const TSGroupThread_NotificationKey_UniqueId = @"TSGroupThread_Notific
     OWSAssert(attachmentStream);
     OWSAssert(transaction);
 
-    self.groupModel.groupImage = [attachmentStream originalImage];
+    self.groupModel.groupImage = [attachmentStream thumbnailImageSmallSync];
     [self saveWithTransaction:transaction];
 
     [transaction addCompletionQueue:nil

SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift

@@ -26,26 +26,14 @@ public enum OWSMediaError: Error {
         guard let originalImage = UIImage(contentsOfFile: path) else {
             throw OWSMediaError.failure(description: "Could not load original image.")
         }
-        // We use UIGraphicsBeginImageContextWithOptions() to scale.
-        // Core Image would provide better quality (e.g. Lanczos) but
-        // at a perf cost we don't want to pay.  We could also use
-        // CoreGraphics directly, but I'm not sure there's any benefit.
         guard let thumbnailImage = originalImage.resized(withMaxDimensionPoints: maxDimension) else {
             throw OWSMediaError.failure(description: "Could not thumbnail image.")
         }
         return thumbnailImage
     }
 
-    private static let kMaxVideoStillSize: CGFloat = 1024
+    @objc public class func thumbnail(forVideoAtPath path: String, maxDimension: CGFloat) throws -> UIImage {
-
+        let maxSize = CGSize(width: maxDimension, height: maxDimension)
-    @objc public class func thumbnail(forVideoAtPath path: String) throws -> UIImage {
-        return try thumbnail(forVideoAtPath: path, maxSize: CGSize(width: kMaxVideoStillSize, height: kMaxVideoStillSize))
-    }
-
-    @objc public class func thumbnail(forVideoAtPath path: String, maxSize: CGSize) throws -> UIImage {
-        var maxSize = maxSize
-        maxSize.width = min(maxSize.width, kMaxVideoStillSize)
-        maxSize.height = min(maxSize.height, kMaxVideoStillSize)
 
         guard FileManager.default.fileExists(atPath: path) else {
             throw OWSMediaError.failure(description: "Media file missing.")

SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift

@@ -7,36 +7,42 @@ import AVFoundation
 
 public enum OWSThumbnailError: Error {
     case failure(description: String)
+    case assertionFailure(description: String)
+    case externalError(description: String, underlyingError:Error)
 }
 
 @objc public class OWSLoadedThumbnail: NSObject {
     public typealias DataSourceBlock = () throws -> Data
 
-    @objc public let image: UIImage
+    @objc
+    public let image: UIImage
     let dataSourceBlock: DataSourceBlock
 
-    @objc public init(image: UIImage, filePath: String) {
+    @objc
+    public init(image: UIImage, filePath: String) {
         self.image = image
         self.dataSourceBlock = {
             return try Data(contentsOf: URL(fileURLWithPath: filePath))
         }
     }
 
-    @objc public init(image: UIImage, data: Data) {
+    @objc
+    public init(image: UIImage, data: Data) {
         self.image = image
         self.dataSourceBlock = {
             return data
         }
     }
 
-    @objc public func data() throws -> Data {
+    @objc
+    public func data() throws -> Data {
         return try dataSourceBlock()
     }
 }
 
 private struct OWSThumbnailRequest {
     public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void
-    public typealias FailureBlock = () -> Void
+    public typealias FailureBlock = (Error) -> Void
 
     let attachment: TSAttachmentStream
     let thumbnailDimensionPoints: UInt
@@ -59,12 +65,10 @@ private struct OWSThumbnailRequest {
     public static let shared = OWSThumbnailService()
 
     public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void
-    public typealias FailureBlock = () -> Void
+    public typealias FailureBlock = (Error) -> Void
 
     private let serialQueue = DispatchQueue(label: "OWSThumbnailService")
 
-    private let dbConnection: YapDatabaseConnection
-
     // This property should only be accessed on the serialQueue.
     //
     // We want to process requests in _reverse_ order in which they
@@ -72,9 +76,6 @@ private struct OWSThumbnailRequest {
     private var thumbnailRequestStack = [OWSThumbnailRequest]()
 
     private override init() {
-
-        dbConnection = OWSPrimaryStorage.shared().newDatabaseConnection()
-
         super.init()
 
         SwiftSingletons.register(self)
@@ -86,7 +87,8 @@ private struct OWSThumbnailRequest {
 
     // completion will only be called on success.
     // completion will be called async on the main thread.
-    @objc public func ensureThumbnail(forAttachment attachment: TSAttachmentStream,
+    @objc
+    public func ensureThumbnail(forAttachment attachment: TSAttachmentStream,
                                                      thumbnailDimensionPoints: UInt,
                                                      success: @escaping SuccessBlock,
                                                      failure: @escaping FailureBlock) {
@@ -106,10 +108,9 @@ private struct OWSThumbnailRequest {
 
     // This should only be called on the serialQueue.
     private func processNextRequestSync() {
-        guard !thumbnailRequestStack.isEmpty else {
+        guard let thumbnailRequest = thumbnailRequestStack.popLast() else {
             return
         }
-        let thumbnailRequest = thumbnailRequestStack.removeLast()
 
         do {
             let loadedThumbnail = try process(thumbnailRequest: thumbnailRequest)
@@ -120,7 +121,7 @@ private struct OWSThumbnailRequest {
             Logger.error("Could not create thumbnail: \(error)")
 
             DispatchQueue.main.async {
-                thumbnailRequest.failure()
+                thumbnailRequest.failure(error)
             }
         }
     }
@@ -146,8 +147,8 @@ private struct OWSThumbnailRequest {
             return OWSLoadedThumbnail(image: image, filePath: thumbnailPath)
         }
         let thumbnailDirPath = (thumbnailPath as NSString).deletingLastPathComponent
-        if !FileManager.default.fileExists(atPath: thumbnailDirPath) {
+        guard OWSFileSystem.ensureDirectoryExists(thumbnailDirPath) else {
-            try FileManager.default.createDirectory(atPath: thumbnailDirPath, withIntermediateDirectories: true, attributes: nil)
+            throw OWSThumbnailError.failure(description: "Could not create attachment's thumbnail directory.")
         }
         guard let originalFilePath = attachment.originalFilePath else {
             throw OWSThumbnailError.failure(description: "Missing original file path.")
@@ -157,18 +158,17 @@ private struct OWSThumbnailRequest {
         if attachment.isImage || attachment.isAnimated {
             thumbnailImage = try OWSMediaUtils.thumbnail(forImageAtPath: originalFilePath, maxDimension: maxDimension)
         } else if attachment.isVideo {
-            let maxSize = CGSize(width: maxDimension, height: maxDimension)
+            thumbnailImage = try OWSMediaUtils.thumbnail(forVideoAtPath: originalFilePath, maxDimension: maxDimension)
-            thumbnailImage = try OWSMediaUtils.thumbnail(forVideoAtPath: originalFilePath, maxSize: maxSize)
         } else {
-            throw OWSThumbnailError.failure(description: "Invalid attachment type.")
+            throw OWSThumbnailError.assertionFailure(description: "Invalid attachment type.")
         }
         guard let thumbnailData = UIImageJPEGRepresentation(thumbnailImage, 0.85) else {
             throw OWSThumbnailError.failure(description: "Could not convert thumbnail to JPEG.")
         }
         do {
-            try thumbnailData.write(to: NSURL.fileURL(withPath: thumbnailPath), options: .atomicWrite)
+            try thumbnailData.write(to: URL(fileURLWithPath: thumbnailPath), options: .atomicWrite)
         } catch let error as NSError {
-            throw OWSThumbnailError.failure(description: "File write failed: \(thumbnailPath), \(error)")
+            throw OWSThumbnailError.externalError(description: "File write failed: \(thumbnailPath), \(error)", underlyingError: error)
         }
         return OWSLoadedThumbnail(image: thumbnailImage, data: thumbnailData)
     }

SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h

@@ -82,8 +82,10 @@ typedef void (^OWSThumbnailFailure)(void);
 #pragma mark - Thumbnails
 
 // On cache hit, the thumbnail will be returned synchronously and completion will never be invoked.
-// On cache miss, nil will be returned and the completion will be invoked async on main if
+// On cache miss, nil will be returned and success will be invoked if thumbnail can be generated;
-// thumbnail can be generated.
+// otherwise failure will be invoked.
+//
+// success and failure are invoked async on main.
 - (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint
                                          success:(OWSThumbnailSuccess)success
                                          failure:(OWSThumbnailFailure)failure;

SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m

@@ -265,8 +265,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
         return nil;
     }
 
+    // Thumbnails are written to the caches directory, so that iOS can
+    // remove them if necessary.
     NSString *dirName = [NSString stringWithFormat:@"%@-thumbnails", self.uniqueId];
-    return [[[self class] attachmentsFolder] stringByAppendingPathComponent:dirName];
+    return [OWSFileSystem.cachesDirectoryPath stringByAppendingPathComponent:dirName];
 }
 
 - (NSString *)pathForThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints
@@ -403,7 +405,9 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
 - (nullable UIImage *)videoStillImage
 {
     NSError *error;
-    UIImage *_Nullable image = [OWSMediaUtils thumbnailForVideoAtPath:self.originalFilePath error:&error];
+    UIImage *_Nullable image = [OWSMediaUtils thumbnailForVideoAtPath:self.originalFilePath
+                                                         maxDimension:ThumbnailDimensionPointsLarge()
+                                                                error:&error];
     if (error || !image) {
         DDLogError(@"Could not create video still: %@.", error);
         return nil;
@@ -702,7 +706,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
     [OWSThumbnailService.shared ensureThumbnailForAttachment:self
                                     thumbnailDimensionPoints:thumbnailDimensionPoints
                                                      success:success
-                                                     failure:failure];
+                                                     failure:^(NSError *error) {
+                                                         DDLogError(@"Failed to create thumbnail: %@", error);
+                                                         failure();
+                                                     }];
     return nil;
 }
 

SignalServiceKit/src/Util/NSData+Image.m

@@ -132,12 +132,12 @@ typedef NS_ENUM(NSInteger, ImageFormat) {
     }
 
     // We only support (A)RGB and (A)Grayscale, so worst case is 4.
-    CGFloat kWorseCastComponentsPerPixel = 4;
+    const CGFloat kWorseCastComponentsPerPixel = 4;
     CGFloat bytesPerPixel = kWorseCastComponentsPerPixel * depthBytes;
 
-    CGFloat kMaxDimension = 4 * 1024;
+    const CGFloat kExpectedBytePerPixel = 4;
-    CGFloat kExpectedBytePerPixel = 4;
+    const CGFloat kMaxValidImageDimension = 4 * 1024;
-    CGFloat kMaxBytes = kMaxDimension * kMaxDimension * kExpectedBytePerPixel;
+    CGFloat kMaxBytes = kMaxValidImageDimension * kMaxValidImageDimension * kExpectedBytePerPixel;
     CGFloat actualBytes = width * height * bytesPerPixel;
     if (actualBytes > kMaxBytes) {
         DDLogWarn(@"invalid dimensions width: %f, height %f, bytesPerPixel: %f", width, height, bytesPerPixel);

SignalServiceKit/src/Util/UIImage+OWS.m

@@ -42,6 +42,13 @@
         DDLogError(@"Invalid original size: %@", NSStringFromCGSize(originalSize));
         return nil;
     }
+
+    CGFloat maxOriginalDimensionPoints = MAX(originalSize.width, originalSize.height);
+    if (maxOriginalDimension < maxDimensionPoints) {
+        // Don't bother scaling an image that is already smaller than the max dimension.
+        return self;
+    }
+
     CGSize thumbnailSize = CGSizeZero;
     if (originalSize.width > originalSize.height) {
         thumbnailSize.width = maxDimensionPoints;