From ae5cee21670eb56e76b917078375a48130a1fcdc Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 26 Mar 2019 17:05:00 -0400 Subject: [PATCH 1/4] Clean up the link preview cache. --- .../ConversationView/ConversationInputToolbar.m | 4 ++++ .../ConversationView/ConversationViewController.m | 2 ++ Signal/src/ViewControllers/HomeView/HomeViewController.m | 2 ++ .../src/Messages/Interactions/OWSLinkPreview.swift | 7 +++++++ 4 files changed, 15 insertions(+) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m b/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m index fd34c1cd4..37b768297 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m @@ -878,6 +878,10 @@ const CGFloat kMaxTextViewHeight = 98; [self ensureShouldShowVoiceMemoButtonAnimated:YES doLayout:YES]; [self updateHeightWithTextView:textView]; [self updateInputLinkPreview]; + + if (textView.text.ows_stripped.length < 1) { + [OWSLinkPreview clearLinkPreviewCache]; + } } - (void)textViewDidChangeSelection:(UITextView *)textView diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 0002ac339..614d73f7c 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2752,6 +2752,8 @@ typedef enum : NSUInteger { AudioServicesPlaySystemSound(soundId); } [self.typingIndicators didSendOutgoingMessageInThread:self.thread]; + + [OWSLinkPreview clearLinkPreviewCache]; } #pragma mark UIDocumentMenuDelegate diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index 3806974b9..7ceb701b8 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -709,6 +709,8 @@ typedef NS_ENUM(NSInteger, HomeViewControllerSection) { [self.searchResultsController viewDidAppear:animated]; self.hasEverAppeared = YES; + + [OWSLinkPreview clearLinkPreviewCache]; } - (void)viewDidDisappear:(BOOL)animated diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index 9bb11b4de..86415b295 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -553,6 +553,13 @@ public class OWSLinkPreview: MTLModel { } } + @objc + public class func clearLinkPreviewCache() { + return serialQueue.async { + linkPreviewDraftCache.removeAllObjects() + } + } + @objc public class func tryToBuildPreviewInfoObjc(previewUrl: String?) -> AnyPromise { return AnyPromise(tryToBuildPreviewInfo(previewUrl: previewUrl)) From 04b60677a6000b2fbc7575d7b7f07ee511dfbefb Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 27 Mar 2019 09:00:29 -0400 Subject: [PATCH 2/4] Simplify link preview cache. --- Signal/src/views/LinkPreviewView.swift | 8 +- .../Interactions/OWSLinkPreview.swift | 78 ++++++++----------- 2 files changed, 38 insertions(+), 48 deletions(-) diff --git a/Signal/src/views/LinkPreviewView.swift b/Signal/src/views/LinkPreviewView.swift index 07d3037ac..bce6af478 100644 --- a/Signal/src/views/LinkPreviewView.swift +++ b/Signal/src/views/LinkPreviewView.swift @@ -103,7 +103,7 @@ public class LinkPreviewDraft: NSObject, LinkPreviewState { } public func imageState() -> LinkPreviewImageState { - if linkPreviewDraft.imageFilePath != nil { + if linkPreviewDraft.jpegImageData != nil { return .loaded } else { return .none @@ -113,11 +113,11 @@ public class LinkPreviewDraft: NSObject, LinkPreviewState { public func image() -> UIImage? { assert(imageState() == .loaded) - guard let imageFilepath = linkPreviewDraft.imageFilePath else { + guard let jpegImageData = linkPreviewDraft.jpegImageData else { return nil } - guard let image = UIImage(contentsOfFile: imageFilepath) else { - owsFailDebug("Could not load image: \(imageFilepath)") + guard let image = UIImage(data: jpegImageData) else { + owsFailDebug("Could not load image: \(jpegImageData.count)") return nil } return image diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index 86415b295..231b9995b 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -41,31 +41,22 @@ public class OWSLinkPreviewDraft: NSObject { public var title: String? @objc - public var imageFilePath: String? + public var jpegImageData: Data? - public init(urlString: String, title: String?, imageFilePath: String? = nil) { + public init(urlString: String, title: String?, jpegImageData: Data? = nil) { self.urlString = urlString self.title = title - self.imageFilePath = imageFilePath + self.jpegImageData = jpegImageData super.init() } - deinit { - // Eagerly clean up temp files. - if let imageFilePath = imageFilePath { - DispatchQueue.global().async { - OWSFileSystem.deleteFile(imageFilePath) - } - } - } - fileprivate func isValid() -> Bool { var hasTitle = false if let titleValue = title { hasTitle = titleValue.count > 0 } - let hasImage = imageFilePath != nil + let hasImage = jpegImageData != nil return hasTitle || hasImage } @@ -198,7 +189,7 @@ public class OWSLinkPreview: MTLModel { guard SSKPreferences.areLinkPreviewsEnabled else { throw LinkPreviewError.noPreview } - let imageAttachmentId = OWSLinkPreview.saveAttachmentIfPossible(inputFilePath: info.imageFilePath, + let imageAttachmentId = OWSLinkPreview.saveAttachmentIfPossible(jpegImageData: info.jpegImageData, transaction: transaction) let linkPreview = OWSLinkPreview(urlString: info.urlString, title: info.title, imageAttachmentId: imageAttachmentId) @@ -211,34 +202,32 @@ public class OWSLinkPreview: MTLModel { return linkPreview } - private class func saveAttachmentIfPossible(inputFilePath filePath: String?, + private class func saveAttachmentIfPossible(jpegImageData: Data?, transaction: YapDatabaseReadWriteTransaction) -> String? { - guard let filePath = filePath else { - return nil - } - guard let fileSize = OWSFileSystem.fileSize(ofPath: filePath) else { - owsFailDebug("Unknown file size for path: \(filePath)") - return nil - } - guard fileSize.uint32Value > 0 else { - owsFailDebug("Invalid file size for path: \(filePath)") + guard let jpegImageData = jpegImageData else { return nil } - let filename = (filePath as NSString).lastPathComponent - let fileExtension = (filename as NSString).pathExtension - guard fileExtension.count > 0 else { - owsFailDebug("Invalid file extension for path: \(filePath)") + let fileSize = jpegImageData.count + guard fileSize > 0 else { + owsFailDebug("Invalid file size for image data.") return nil } - guard let contentType = MIMETypeUtil.mimeType(forFileExtension: fileExtension) else { - owsFailDebug("Invalid content type for path: \(filePath)") + let fileExtension = "jpg" + let contentType = OWSMimeTypeImageJpeg + + let filePath = OWSFileSystem.temporaryFilePath(withFileExtension: fileExtension) + do { + try jpegImageData.write(to: NSURL.fileURL(withPath: filePath), options: .atomicWrite) + } catch let error as NSError { + owsFailDebug("file write failed: \(filePath), \(error)") return nil } + guard let dataSource = DataSourcePath.dataSource(withFilePath: filePath, shouldDeleteOnDeallocation: true) else { owsFailDebug("Could not create data source for path: \(filePath)") return nil } - let attachment = TSAttachmentStream(contentType: contentType, byteCount: fileSize.uint32Value, sourceFilename: nil, caption: nil, albumMessageId: nil) + let attachment = TSAttachmentStream(contentType: contentType, byteCount: UInt32(fileSize), sourceFilename: nil, caption: nil, albumMessageId: nil) guard attachment.write(dataSource) else { owsFailDebug("Could not write data source for path: \(filePath)") return nil @@ -528,16 +517,25 @@ public class OWSLinkPreview: MTLModel { // MARK: - Preview Construction // This cache should only be accessed on serialQueue. - private static var linkPreviewDraftCache: NSCache = NSCache() + // + // We should only main + private static var linkPreviewDraftCache: OWSLinkPreviewDraft? private class func cachedLinkPreview(forPreviewUrl previewUrl: String) -> OWSLinkPreviewDraft? { return serialQueue.sync { - return linkPreviewDraftCache.object(forKey: previewUrl as NSString) + guard let linkPreviewDraft = linkPreviewDraftCache, + linkPreviewDraft.urlString == previewUrl else { + Logger.verbose("----- Cache miss.") + return nil + } + Logger.verbose("----- Cache hit.") + return linkPreviewDraft } } private class func setCachedLinkPreview(_ linkPreviewDraft: OWSLinkPreviewDraft, forPreviewUrl previewUrl: String) { + assert(previewUrl == linkPreviewDraft.urlString) // Exit early if link previews are not enabled in order to avoid // tainting the cache. @@ -549,14 +547,14 @@ public class OWSLinkPreview: MTLModel { } serialQueue.sync { - linkPreviewDraftCache.setObject(linkPreviewDraft, forKey: previewUrl as NSString) + linkPreviewDraftCache = linkPreviewDraft } } @objc public class func clearLinkPreviewCache() { return serialQueue.async { - linkPreviewDraftCache.removeAllObjects() + linkPreviewDraftCache = nil } } @@ -766,15 +764,7 @@ public class OWSLinkPreview: MTLModel { return downloadImage(url: imageUrl, imageMimeType: imageMimeType) .map(on: DispatchQueue.global()) { (imageData: Data) -> OWSLinkPreviewDraft in // We always recompress images to Jpeg. - let imageFilePath = OWSFileSystem.temporaryFilePath(withFileExtension: "jpg") - do { - try imageData.write(to: NSURL.fileURL(withPath: imageFilePath), options: .atomicWrite) - } catch let error as NSError { - owsFailDebug("file write failed: \(imageFilePath), \(error)") - throw LinkPreviewError.assertionFailure - } - - let linkPreviewDraft = OWSLinkPreviewDraft(urlString: linkUrlString, title: title, imageFilePath: imageFilePath) + let linkPreviewDraft = OWSLinkPreviewDraft(urlString: linkUrlString, title: title, jpegImageData: imageData) return linkPreviewDraft } .recover(on: DispatchQueue.global()) { (_) -> Promise in From bb1921afe65b21b1ec119dae82c64df980f8910e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 27 Mar 2019 09:03:27 -0400 Subject: [PATCH 3/4] Simplify link preview cache. --- .../ConversationView/ConversationInputToolbar.m | 4 ---- .../ConversationView/ConversationViewController.m | 2 -- Signal/src/ViewControllers/HomeView/HomeViewController.m | 2 -- .../src/Messages/Interactions/OWSLinkPreview.swift | 7 ------- 4 files changed, 15 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m b/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m index 37b768297..fd34c1cd4 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m @@ -878,10 +878,6 @@ const CGFloat kMaxTextViewHeight = 98; [self ensureShouldShowVoiceMemoButtonAnimated:YES doLayout:YES]; [self updateHeightWithTextView:textView]; [self updateInputLinkPreview]; - - if (textView.text.ows_stripped.length < 1) { - [OWSLinkPreview clearLinkPreviewCache]; - } } - (void)textViewDidChangeSelection:(UITextView *)textView diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 614d73f7c..0002ac339 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2752,8 +2752,6 @@ typedef enum : NSUInteger { AudioServicesPlaySystemSound(soundId); } [self.typingIndicators didSendOutgoingMessageInThread:self.thread]; - - [OWSLinkPreview clearLinkPreviewCache]; } #pragma mark UIDocumentMenuDelegate diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index 7ceb701b8..3806974b9 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -709,8 +709,6 @@ typedef NS_ENUM(NSInteger, HomeViewControllerSection) { [self.searchResultsController viewDidAppear:animated]; self.hasEverAppeared = YES; - - [OWSLinkPreview clearLinkPreviewCache]; } - (void)viewDidDisappear:(BOOL)animated diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index 231b9995b..9f52e3ad4 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -551,13 +551,6 @@ public class OWSLinkPreview: MTLModel { } } - @objc - public class func clearLinkPreviewCache() { - return serialQueue.async { - linkPreviewDraftCache = nil - } - } - @objc public class func tryToBuildPreviewInfoObjc(previewUrl: String?) -> AnyPromise { return AnyPromise(tryToBuildPreviewInfo(previewUrl: previewUrl)) From 81e3dfc3cf6d08b93ec07882b63d35cfd7162bfe Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 27 Mar 2019 12:36:39 -0400 Subject: [PATCH 4/4] Respond to CR. --- SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index 9f52e3ad4..8b1eb18eb 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -518,7 +518,7 @@ public class OWSLinkPreview: MTLModel { // This cache should only be accessed on serialQueue. // - // We should only main + // We should only maintain a "cache" of the last known draft. private static var linkPreviewDraftCache: OWSLinkPreviewDraft? private class func cachedLinkPreview(forPreviewUrl previewUrl: String) -> OWSLinkPreviewDraft? {