Merge branch 'charlesmchen/convoThumbnails2' into release/2.29.2

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m

@@ -654,7 +654,6 @@ NS_ASSUME_NONNULL_BEGIN
 - (nullable id)tryToLoadCellMedia:(nullable id (^)(void))loadCellMediaBlock
                         mediaView:(UIView *)mediaView
                          cacheKey:(NSString *)cacheKey
-                  shouldSkipCache:(BOOL)shouldSkipCache
                      canLoadAsync:(BOOL)canLoadAsync
 {
     OWSAssert(self.attachmentStream);
@@ -674,9 +673,7 @@ NS_ASSUME_NONNULL_BEGIN
     cellMedia = loadCellMediaBlock();
     if (cellMedia) {
         DDLogVerbose(@"%@ cell media cache miss", self.logTag);
-        if (!shouldSkipCache) {
+        [self.cellMediaCache setObject:cellMedia forKey:cacheKey];
-            [self.cellMediaCache setObject:cellMedia forKey:cacheKey];
-        }
     } else if (!canLoadAsync) {
         DDLogError(@"%@ Failed to load cell media: %@", [self logTag], [self.attachmentStream originalMediaURL]);
         self.viewItem.didCellMediaFailToLoad = YES;
@@ -837,9 +834,8 @@ NS_ASSUME_NONNULL_BEGIN
     stillImageView.backgroundColor = [UIColor whiteColor];
     [self addAttachmentUploadViewIfNecessary];
 
-    __weak UIImageView *weakImageView = stillImageView;
-
     __weak OWSMessageBubbleView *weakSelf = self;
+    __weak UIImageView *weakImageView = stillImageView;
     self.loadCellContentBlock = ^{
         OWSMessageBubbleView *strongSelf = weakSelf;
         if (!strongSelf) {
@@ -849,16 +845,11 @@ NS_ASSUME_NONNULL_BEGIN
         if (stillImageView.image) {
             return;
         }
-        // Don't cache large still images.
-        //
-        // TODO: Don't use full size images in the message cells.
-        const NSUInteger kMaxCachableSize = 1024 * 1024;
-        BOOL shouldSkipCache =
-            [OWSFileSystem fileSizeOfPath:strongSelf.attachmentStream.originalFilePath].unsignedIntegerValue
-            < kMaxCachableSize;
         stillImageView.image = [strongSelf
             tryToLoadCellMedia:^{
                 OWSCAssert([strongSelf.attachmentStream isImage]);
+                OWSCAssert([strongSelf.attachmentStream isValidImage]);
+
                 return [strongSelf.attachmentStream
                     thumbnailImageMediumWithSuccess:^(UIImage *image) {
                         weakImageView.image = image;
@@ -869,7 +860,6 @@ NS_ASSUME_NONNULL_BEGIN
             }
                      mediaView:stillImageView
                       cacheKey:strongSelf.attachmentStream.uniqueId
-               shouldSkipCache:shouldSkipCache
                   canLoadAsync:YES];
     };
     self.unloadCellContentBlock = ^{
@@ -909,6 +899,7 @@ NS_ASSUME_NONNULL_BEGIN
         animatedImageView.image = [strongSelf
             tryToLoadCellMedia:^{
                 OWSCAssert([strongSelf.attachmentStream isAnimated]);
+                OWSCAssert([strongSelf.attachmentStream isValidImage]);
 
                 NSString *_Nullable filePath = [strongSelf.attachmentStream originalFilePath];
                 YYImage *_Nullable animatedImage = nil;
@@ -919,7 +910,6 @@ NS_ASSUME_NONNULL_BEGIN
             }
                      mediaView:animatedImageView
                       cacheKey:strongSelf.attachmentStream.uniqueId
-               shouldSkipCache:NO
                   canLoadAsync:NO];
     };
     self.unloadCellContentBlock = ^{
@@ -980,6 +970,7 @@ NS_ASSUME_NONNULL_BEGIN
     }];
 
     __weak OWSMessageBubbleView *weakSelf = self;
+    __weak UIImageView *weakImageView = stillImageView;
     self.loadCellContentBlock = ^{
         OWSMessageBubbleView *strongSelf = weakSelf;
         if (!strongSelf) {
@@ -992,13 +983,19 @@ NS_ASSUME_NONNULL_BEGIN
         stillImageView.image = [strongSelf
             tryToLoadCellMedia:^{
                 OWSCAssert([strongSelf.attachmentStream isVideo]);
+                OWSCAssert([strongSelf.attachmentStream isValidVideo]);
 
-                return strongSelf.attachmentStream.originalImage;
+                return [strongSelf.attachmentStream
+                    thumbnailImageMediumWithSuccess:^(UIImage *image) {
+                        weakImageView.image = image;
+                    }
+                    failure:^{
+                        DDLogError(@"Could not load thumbnail.");
+                    }];
             }
                      mediaView:stillImageView
                       cacheKey:strongSelf.attachmentStream.uniqueId
-               shouldSkipCache:NO
+                  canLoadAsync:YES];
-                  canLoadAsync:NO];
     };
     self.unloadCellContentBlock = ^{
         OWSMessageBubbleView *strongSelf = weakSelf;

Signal/src/ViewControllers/ConversationView/ConversationViewController.m

@@ -2220,7 +2220,7 @@ typedef enum : NSUInteger {
 
     NSFileManager *fileManager = [NSFileManager defaultManager];
     if (![fileManager fileExistsAtPath:attachmentStream.originalFilePath]) {
-        OWSFail(@"%@ Missing video file: %@", self.logTag, attachmentStream.originalFilePath);
+        OWSFail(@"%@ Missing audio file: %@", self.logTag, attachmentStream.originalFilePath);
     }
 
     [self dismissKeyBoard];

Signal/src/ViewControllers/MediaDetailViewController.m

@@ -35,11 +35,9 @@ NS_ASSUME_NONNULL_BEGIN
 @property (nonatomic) UIView *replacingView;
 @property (nonatomic) UIButton *shareButton;
 
-@property (nonatomic) NSData *fileData;
-
 @property (nonatomic) TSAttachmentStream *attachmentStream;
 @property (nonatomic, nullable) ConversationViewItem *viewItem;
-@property (nonatomic, readonly) UIImage *image;
+@property (nonatomic, nullable) UIImage *image;
 
 @property (nonatomic, nullable) OWSVideoPlayer *videoPlayer;
 @property (nonatomic, nullable) UIButton *playVideoButton;
@@ -55,6 +53,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
+#pragma mark -
+
 @implementation MediaDetailViewController
 
 - (void)dealloc
@@ -72,8 +72,18 @@ NS_ASSUME_NONNULL_BEGIN
 
     _galleryItemBox = galleryItemBox;
     _viewItem = viewItem;
+
     // We cache the image data in case the attachment stream is deleted.
-    _image = galleryItemBox.attachmentStream.originalImage;
+    __weak MediaDetailViewController *weakSelf = self;
+    _image = [galleryItemBox.attachmentStream
+        thumbnailImageLargeWithSuccess:^(UIImage *image) {
+            weakSelf.image = image;
+            [weakSelf updateContents];
+            [weakSelf updateMinZoomScale];
+        }
+        failure:^{
+            DDLogWarn(@"Could not load media.");
+        }];
 
     return self;
 }
@@ -83,22 +93,6 @@ NS_ASSUME_NONNULL_BEGIN
     return self.galleryItemBox.attachmentStream;
 }
 
-- (NSURL *_Nullable)attachmentUrl
-{
-    return self.attachmentStream.originalMediaURL;
-}
-
-- (NSData *)fileData
-{
-    if (!_fileData) {
-        NSURL *_Nullable url = self.attachmentUrl;
-        if (url) {
-            _fileData = [NSData dataWithContentsOfURL:url];
-        }
-    }
-    return _fileData;
-}
-
 - (BOOL)isAnimated
 {
     return self.attachmentStream.isAnimated;
@@ -115,7 +109,7 @@ NS_ASSUME_NONNULL_BEGIN
 
     self.view.backgroundColor = [UIColor clearColor];
 
-    [self createContents];
+    [self updateContents];
 }
 
 - (void)viewWillAppear:(BOOL)animated
@@ -134,6 +128,13 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)updateMinZoomScale
 {
+    if (!self.image) {
+        self.scrollView.minimumZoomScale = 1.f;
+        self.scrollView.maximumZoomScale = 1.f;
+        self.scrollView.zoomScale = 1.f;
+        return;
+    }
+
     CGSize viewSize = self.scrollView.bounds.size;
     UIImage *image = self.image;
     OWSAssert(image);
@@ -163,8 +164,13 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark - Initializers
 
-- (void)createContents
+- (void)updateContents
 {
+    [self.mediaView removeFromSuperview];
+    [self.scrollView removeFromSuperview];
+    [self.playVideoButton removeFromSuperview];
+    [self.videoProgressBar removeFromSuperview];
+
     UIScrollView *scrollView = [UIScrollView new];
     [self.view addSubview:scrollView];
     self.scrollView = scrollView;
@@ -184,19 +190,28 @@ NS_ASSUME_NONNULL_BEGIN
 
     if (self.isAnimated) {
         if (self.attachmentStream.isValidImage) {
-            YYImage *animatedGif = [YYImage imageWithData:self.fileData];
+            YYImage *animatedGif = [YYImage imageWithContentsOfFile:self.attachmentStream.originalFilePath];
             YYAnimatedImageView *animatedView = [YYAnimatedImageView new];
             animatedView.image = animatedGif;
             self.mediaView = animatedView;
         } else {
-            self.mediaView = [UIImageView new];
+            self.mediaView = [UIView new];
+            self.mediaView.backgroundColor = Theme.offBackgroundColor;
         }
+    } else if (!self.image) {
+        // Still loading thumbnail.
+        self.mediaView = [UIView new];
+        self.mediaView.backgroundColor = Theme.offBackgroundColor;
     } else if (self.isVideo) {
-        self.mediaView = [self buildVideoPlayerView];
+        if (self.attachmentStream.isValidVideo) {
+            self.mediaView = [self buildVideoPlayerView];
+        } else {
+            self.mediaView = [UIView new];
+            self.mediaView.backgroundColor = Theme.offBackgroundColor;
+        }
     } else {
         // Present the static image using standard UIImageView
         UIImageView *imageView = [[UIImageView alloc] initWithImage:self.image];
-
         self.mediaView = imageView;
     }
 
@@ -260,12 +275,14 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (UIView *)buildVideoPlayerView
 {
+    NSURL *_Nullable attachmentUrl = self.attachmentStream.originalMediaURL;
+
     NSFileManager *fileManager = [NSFileManager defaultManager];
-    if (![fileManager fileExistsAtPath:[self.attachmentUrl path]]) {
+    if (![fileManager fileExistsAtPath:[attachmentUrl path]]) {
         OWSFail(@"%@ Missing video file", self.logTag);
     }
 
-    OWSVideoPlayer *player = [[OWSVideoPlayer alloc] initWithUrl:self.attachmentUrl];
+    OWSVideoPlayer *player = [[OWSVideoPlayer alloc] initWithUrl:attachmentUrl];
     [player seekToTime:kCMTimeZero];
     player.delegate = self;
     self.videoPlayer = player;

Signal/src/ViewControllers/MediaGalleryViewController.swift

@@ -8,7 +8,7 @@ public enum GalleryDirection {
     case before, after, around
 }
 
-public struct MediaGalleryItem: Equatable, Hashable {
+public class MediaGalleryItem: Equatable, Hashable {
     let logTag = "[MediaGalleryItem]"
 
     let message: TSMessage
@@ -38,15 +38,6 @@ public struct MediaGalleryItem: Equatable, Hashable {
         return attachmentStream.thumbnailImageSmall(success: async, failure: {})
     }
 
-    var fullSizedImage: UIImage {
-        guard let image = attachmentStream.originalImage else {
-            owsFail("\(logTag) in \(#function) unexpectedly unable to build attachment image")
-            return UIImage()
-        }
-
-        return image
-    }
-
     // MARK: Equatable
 
     public static func == (lhs: MediaGalleryItem, rhs: MediaGalleryItem) -> Bool {
@@ -304,7 +295,16 @@ class MediaGalleryViewController: OWSNavigationController, MediaGalleryDataSourc
 
         // loadView hasn't necessarily been called yet.
         self.loadViewIfNeeded()
-        self.presentationView.image = initialDetailItem.fullSizedImage
+
+        self.presentationView.image = initialDetailItem.attachmentStream.thumbnailImageLarge(success: { [weak self] (image) in
+            guard let strongSelf = self else {
+                return
+            }
+            strongSelf.presentationView.image = image
+            }, failure: {
+                Logger.warn("Could not load presentation image.")
+        })
+
         self.applyInitialMediaViewConstraints()
 
         // Restore presentationView.alpha in case a previous dismiss left us in a bad state.
@@ -481,7 +481,14 @@ class MediaGalleryViewController: OWSNavigationController, MediaGalleryDataSourc
         // it sits on the screen in the conversation view.
         let changedItems = currentItem != self.initialDetailItem
         if changedItems {
-            self.presentationView.image = currentItem.fullSizedImage
+            self.presentationView.image = currentItem.attachmentStream.thumbnailImageLarge(success: { [weak self] (image) in
+                guard let strongSelf = self else {
+                    return
+                }
+                strongSelf.presentationView.image = image
+                }, failure: {
+                    Logger.warn("Could not load presentation image.")
+            })
             self.applyOffscreenMediaViewConstraints()
         } else {
             self.applyInitialMediaViewConstraints()

SignalMessaging/ViewModels/OWSQuotedReplyModel.m

@@ -88,7 +88,7 @@ NS_ASSUME_NONNULL_BEGIN
         TSAttachmentStream *attachmentStream;
         if ([attachment isKindOfClass:[TSAttachmentStream class]]) {
             attachmentStream = (TSAttachmentStream *)attachment;
-            thumbnailImage = attachmentStream.originalImage;
+            thumbnailImage = attachmentStream.thumbnailImageSmallSync;
         }
     } else if (attachmentInfo.thumbnailAttachmentPointerId) {
         // download failed, or hasn't completed yet.

SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift

@@ -85,8 +85,7 @@ private struct OWSThumbnailRequest {
         return attachment.isImage || attachment.isAnimated || attachment.isVideo
     }
 
-    // completion will only be called on success.
+    // success and failure will be called async _off_ the main thread.
-    // completion will be called async on the main thread.
     @objc
     public func ensureThumbnail(forAttachment attachment: TSAttachmentStream,
                                                      thumbnailDimensionPoints: UInt,
@@ -114,13 +113,13 @@ private struct OWSThumbnailRequest {
 
         do {
             let loadedThumbnail = try process(thumbnailRequest: thumbnailRequest)
-            DispatchQueue.main.async {
+            DispatchQueue.global().async {
                 thumbnailRequest.success(loadedThumbnail)
             }
         } catch {
             Logger.error("Could not create thumbnail: \(error)")
 
-            DispatchQueue.main.async {
+            DispatchQueue.global().async {
                 thumbnailRequest.failure(error)
             }
         }
@@ -146,6 +145,9 @@ private struct OWSThumbnailRequest {
             }
             return OWSLoadedThumbnail(image: image, filePath: thumbnailPath)
         }
+
+        Logger.verbose("Creating thumbnail of size: \(thumbnailRequest.thumbnailDimensionPoints)")
+
         let thumbnailDirPath = (thumbnailPath as NSString).deletingLastPathComponent
         guard OWSFileSystem.ensureDirectoryExists(thumbnailDirPath) else {
             throw OWSThumbnailError.failure(description: "Could not create attachment's thumbnail directory.")

SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m

@@ -7,6 +7,7 @@
 #import "NSData+Image.h"
 #import "OWSFileSystem.h"
 #import "TSAttachmentPointer.h"
+#import "Threading.h"
 #import <AVFoundation/AVFoundation.h>
 #import <SignalServiceKit/SignalServiceKit-Swift.h>
 #import <YapDatabase/YapDatabase.h>
@@ -20,7 +21,7 @@ const NSUInteger ThumbnailDimensionPointsLarge()
 {
     CGSize screenSizePoints = UIScreen.mainScreen.bounds.size;
     const CGFloat kMinZoomFactor = 2.f;
-    return MAX(screenSizePoints.width * kMinZoomFactor, screenSizePoints.height * kMinZoomFactor);
+    return MAX(screenSizePoints.width, screenSizePoints.height) * kMinZoomFactor;
 }
 
 typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
@@ -41,6 +42,9 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
 // Optional property.  Only set for attachments which need "lazy backup restore."
 @property (nonatomic, nullable) NSString *lazyRestoreFragmentId;
 
+@property (atomic, nullable) NSNumber *isValidImageCached;
+@property (atomic, nullable) NSNumber *isValidVideoCached;
+
 @end
 
 #pragma mark -
@@ -347,14 +351,33 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
 {
     OWSAssert(self.isImage || self.isAnimated);
 
-    return [NSData ows_isValidImageAtPath:self.originalFilePath mimeType:self.contentType];
+    if (self.lazyRestoreFragment) {
+        return NO;
+    }
+
+    @synchronized(self) {
+        if (!self.isValidImageCached) {
+            self.isValidImageCached =
+                @([NSData ows_isValidImageAtPath:self.originalFilePath mimeType:self.contentType]);
+        }
+        return self.isValidImageCached.boolValue;
+    }
 }
 
 - (BOOL)isValidVideo
 {
     OWSAssert(self.isVideo);
 
-    return [OWSMediaUtils isValidVideoWithPath:self.originalFilePath];
+    if (self.lazyRestoreFragment) {
+        return NO;
+    }
+
+    @synchronized(self) {
+        if (!self.isValidVideoCached) {
+            self.isValidVideoCached = @([OWSMediaUtils isValidVideoWithPath:self.originalFilePath]);
+        }
+        return self.isValidVideoCached.boolValue;
+    }
 }
 
 #pragma mark -
@@ -672,10 +695,16 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
 {
     OWSLoadedThumbnail *_Nullable loadedThumbnail;
     loadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:thumbnailDimensionPoints
-                                                                success:^(OWSLoadedThumbnail *loadedThumbnail) {
+        success:^(OWSLoadedThumbnail *loadedThumbnail) {
-                                                                    success(loadedThumbnail.image);
+            DispatchMainThreadSafe(^{
-                                                                }
+                success(loadedThumbnail.image);
-                                                                failure:failure];
+            });
+        }
+        failure:^{
+            DispatchMainThreadSafe(^{
+                failure();
+            });
+        }];
     return loadedThumbnail.image;
 }
 
@@ -715,13 +744,14 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
 
 - (nullable OWSLoadedThumbnail *)loadedThumbnailSmallSync
 {
-    __block dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
+    dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
 
-    __block OWSLoadedThumbnail *_Nullable loadedThumbnail = nil;
+    __block OWSLoadedThumbnail *_Nullable asyncLoadedThumbnail = nil;
-    loadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall
+    OWSLoadedThumbnail *_Nullable syncLoadedThumbnail = nil;
+    syncLoadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall
         success:^(OWSLoadedThumbnail *asyncLoadedThumbnail) {
             @synchronized(self) {
-                loadedThumbnail = asyncLoadedThumbnail;
+                asyncLoadedThumbnail = asyncLoadedThumbnail;
             }
             dispatch_semaphore_signal(semaphore);
         }
@@ -729,22 +759,36 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail);
             dispatch_semaphore_signal(semaphore);
         }];
 
+    if (syncLoadedThumbnail) {
+        return syncLoadedThumbnail;
+    }
+
     // Wait up to N seconds.
     dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC)));
     @synchronized(self) {
-        return loadedThumbnail;
+        return asyncLoadedThumbnail;
     }
 }
 
 - (nullable UIImage *)thumbnailImageSmallSync
 {
-    return [self loadedThumbnailSmallSync].image;
+    OWSLoadedThumbnail *_Nullable loadedThumbnail = [self loadedThumbnailSmallSync];
+    if (!loadedThumbnail) {
+        DDLogInfo(@"Couldn't load small thumbnail sync.");
+        return nil;
+    }
+    return loadedThumbnail.image;
 }
 
 - (nullable NSData *)thumbnailDataSmallSync
 {
+    OWSLoadedThumbnail *_Nullable loadedThumbnail = [self loadedThumbnailSmallSync];
+    if (!loadedThumbnail) {
+        DDLogInfo(@"Couldn't load small thumbnail sync.");
+        return nil;
+    }
     NSError *error;
-    NSData *_Nullable data = [[self loadedThumbnailSmallSync] dataAndReturnError:&error];
+    NSData *_Nullable data = [loadedThumbnail dataAndReturnError:&error];
     if (error || !data) {
         OWSFail(@"Couldn't load thumbnail data: %@", error);
         return nil;

SignalServiceKit/src/Util/NSData+Image.m

@@ -136,7 +136,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) {
     CGFloat bytesPerPixel = kWorseCastComponentsPerPixel * depthBytes;
 
     const CGFloat kExpectedBytePerPixel = 4;
-    const CGFloat kMaxValidImageDimension = 4 * 1024;
+    const CGFloat kMaxValidImageDimension = 8 * 1024;
     CGFloat kMaxBytes = kMaxValidImageDimension * kMaxValidImageDimension * kExpectedBytePerPixel;
     CGFloat actualBytes = width * height * bytesPerPixel;
     if (actualBytes > kMaxBytes) {