diff --git a/Podfile.lock b/Podfile.lock index be1cfb7bb..d144bb215 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -134,7 +134,7 @@ EXTERNAL SOURCES: OpenSSL: :git: https://github.com/WhisperSystems/OpenSSL-Pod SignalServiceKit: - :path: "." + :path: . SocketRocket: :git: https://github.com/facebook/SocketRocket.git @@ -176,6 +176,6 @@ SPEC CHECKSUMS: YapDatabase: cd911121580ff16675f65ad742a9eb0ab4d9e266 YYImage: 1e1b62a9997399593e4b9c4ecfbbabbf1d3f3b54 -PODFILE CHECKSUM: '00831faaa7677029090c311c00ceadaa44f65c0f' +PODFILE CHECKSUM: 00831faaa7677029090c311c00ceadaa44f65c0f COCOAPODS: 1.2.1 diff --git a/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift b/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift index e7cc030c8..d805576a2 100644 --- a/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift +++ b/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift @@ -17,6 +17,10 @@ class GifPickerCell: UICollectionViewCell { } } + // Loading and playing GIFs is quite expensive (network, memory, cpu). + // Here's a bit of logic to not preload offscreen cells that are prefetched. + // + // TODO: Go a step farther and stop playback of cells that scroll offscreen. var shouldLoad = false { didSet { AssertIsOnMainThread() @@ -25,6 +29,8 @@ class GifPickerCell: UICollectionViewCell { } } + // We do "progressive" loading by loading stills (jpg or gif) and "full" gifs. + // This is critical on cellular connections. var stillAssetRequest: GiphyAssetRequest? var stillAsset: GiphyAsset? var fullAssetRequest: GiphyAssetRequest? @@ -85,6 +91,8 @@ class GifPickerCell: UICollectionViewCell { guard self.fullAsset == nil else { return } + // The Giphy API returns a slew of "renditions" for a given image. + // It's critical that we carefully "pick" the best rendition to use. guard let fullRendition = imageInfo.pickGifRendition() else { Logger.warn("\(TAG) could not pick gif rendition: \(imageInfo.giphyId)") clearAssetRequest() diff --git a/Signal/src/ViewControllers/GifPicker/GifPickerLayout.swift b/Signal/src/ViewControllers/GifPicker/GifPickerLayout.swift index 63402af98..a25fffd16 100644 --- a/Signal/src/ViewControllers/GifPicker/GifPickerLayout.swift +++ b/Signal/src/ViewControllers/GifPicker/GifPickerLayout.swift @@ -8,6 +8,7 @@ protocol GifPickerLayoutDelegate: class { func imageInfosForLayout() -> [GiphyImageInfo] } +// A Pinterest-style waterfall layout. class GifPickerLayout: UICollectionViewLayout { let TAG = "[GifPickerLayout]" @@ -69,16 +70,17 @@ class GifPickerLayout: UICollectionViewLayout { // due to rounding error. let totalHSpacing = totalViewWidth - ((2 * hMargin) + (columnCount * columnWidth)) + // columnXs are the left edge of each column. var columnXs = [UInt]() + // columnYs are the top edge of the next cell in each column. var columnYs = [UInt]() - // Note that columnIndex is a one-based index, not a zero-based index. - for columnIndex in 1...columnCount { - var columnX = hMargin + (columnWidth * (columnIndex - 1)) + for columnIndex in 0...columnCount-1 { + var columnX = hMargin + (columnWidth * columnIndex) if columnCount > 1 { // We want to unevenly distribute the hSpacing between the columns // so that the left and right margins are equal, which is non-trivial // due to rounding error. - columnX += ((totalHSpacing * (columnIndex - 1)) / (columnCount - 1)) + columnX += ((totalHSpacing * columnIndex) / (columnCount - 1)) } columnXs.append(columnX) columnYs.append(vMargin) @@ -86,6 +88,7 @@ class GifPickerLayout: UICollectionViewLayout { // Always layout all items. let imageInfos = delegate.imageInfosForLayout() + var contentBottom = vMargin for (cellIndex, imageInfo) in imageInfos.enumerated() { // Select a column by finding the "highest, leftmost" column. var column = 0 @@ -107,13 +110,11 @@ class GifPickerLayout: UICollectionViewLayout { itemAttributesMap[UInt(cellIndex)] = itemAttributes columnYs[column] = cellY + cellHeight + vSpacing + contentBottom = max(contentBottom, cellY + cellHeight) } - var contentHeight = vMargin - for columnY in columnYs { - contentHeight = max(contentHeight, columnY) - } - contentHeight += vMargin + // Add bottom margin. + let contentHeight = contentBottom + vMargin contentSize = CGSize(width:CGFloat(totalViewWidth), height:CGFloat(contentHeight)) } diff --git a/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift b/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift index 99859e17d..259663241 100644 --- a/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift +++ b/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift @@ -132,7 +132,6 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect return imageInfos.count } - // The cell that is returned must be retrieved from a call to -dequeueReusableCellWithReuseIdentifier:forIndexPath: public func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { let imageInfo = imageInfos[indexPath.row] diff --git a/Signal/src/environment/NotificationsManager.m b/Signal/src/environment/NotificationsManager.m index e16c926b2..df7ddfc7e 100644 --- a/Signal/src/environment/NotificationsManager.m +++ b/Signal/src/environment/NotificationsManager.m @@ -90,12 +90,16 @@ NSString *const kNotificationsManagerNewMesssageSoundName = @"NewMessage.aifc"; */ - (void)presentMissedCall:(SignalCall *)call callerName:(NSString *)callerName { + TSContactThread *thread = [TSContactThread getOrCreateThreadWithContactId:call.remotePhoneNumber]; + OWSAssert(thread != nil); + UILocalNotification *notification = [UILocalNotification new]; notification.category = PushManagerCategoriesMissedCall; NSString *localCallId = call.localId.UUIDString; notification.userInfo = @{ PushManagerUserInfoKeysLocalCallId : localCallId, - PushManagerUserInfoKeysCallBackSignalRecipientId : call.remotePhoneNumber + PushManagerUserInfoKeysCallBackSignalRecipientId : call.remotePhoneNumber, + Signal_Thread_UserInfo_Key : thread.uniqueId }; if ([self shouldPlaySoundForNotification]) { diff --git a/Signal/src/network/GifDownloader.swift b/Signal/src/network/GifDownloader.swift index 5c20f4ebd..1600f3141 100644 --- a/Signal/src/network/GifDownloader.swift +++ b/Signal/src/network/GifDownloader.swift @@ -9,14 +9,22 @@ enum GiphyRequestPriority { case low, high } +// Represents a request to download a GIF. +// +// Should be cancelled if no longer necessary. @objc class GiphyAssetRequest: NSObject { static let TAG = "[GiphyAssetRequest]" let rendition: GiphyRendition let priority: GiphyRequestPriority - let success: ((GiphyAsset) -> Void) - let failure: (() -> Void) + // Exactly one of success or failure should be called once, + // on the main thread _unless_ this request is cancelled before + // the request succeeds or fails. + private var success: ((GiphyAsset) -> Void) + private var failure: (() -> Void) + var wasCancelled = false + // This property is an internal implementation detail of the download process. var assetFilePath: String? init(rendition: GiphyRendition, @@ -31,10 +39,48 @@ enum GiphyRequestPriority { } public func cancel() { + AssertIsOnMainThread() + wasCancelled = true + + // Don't call the callbacks if the request is cancelled. + clearCallbacks() + } + + private func clearCallbacks() { + AssertIsOnMainThread() + + // Replace success and failure with no-ops. + success = { _ in + } + failure = { + } + } + + public func requestDidSucceed(asset: GiphyAsset) { + AssertIsOnMainThread() + + success(asset) + + // Only one of the callbacks should be called, and only once. + clearCallbacks() + } + + public func requestDidFail() { + AssertIsOnMainThread() + + failure() + + // Only one of the callbacks should be called, and only once. + clearCallbacks() } } +// Represents a downloaded gif asset. +// +// The blob on disk is cleaned up when this instance is deallocated, +// so consumers of this resource should retain a strong reference to +// this instance as long as they are using the asset. @objc class GiphyAsset: NSObject { static let TAG = "[GiphyAsset]" @@ -48,6 +94,7 @@ enum GiphyRequestPriority { } deinit { + // Clean up on the asset on disk. let filePathCopy = filePath DispatchQueue.global().async { do { @@ -60,6 +107,7 @@ enum GiphyRequestPriority { } } +// A simple LRU cache bounded by the number of entries. class LRUCache { private var cacheMap = [KeyType: ValueType]() @@ -102,6 +150,7 @@ class LRUCache { private var URLSessionTask_GiphyAssetRequest: UInt8 = 0 +// This extension is used to punch an asset request onto a download task. extension URLSessionTask { var assetRequest: GiphyAssetRequest { get { @@ -121,6 +170,7 @@ extension URLSessionTask { static let sharedInstance = GifDownloader() + // A private queue used for download task callbacks. private let operationQueue = OperationQueue() // Force usage as a singleton @@ -147,10 +197,19 @@ extension URLSessionTask { return session } + // 100 entries of which at least half will probably be stills. + // Actual animated GIFs will usually be less than 3 MB so the + // max size of the cache on disk should be ~150 MB. Bear in mind + // that assets are not always deleted on disk as soon as they are + // evacuated from the cache; if a cache consumer (e.g. view) is + // still using the asset, the asset won't be deleted on disk until + // it is no longer in use. private var assetMap = LRUCache(maxSize:100) - // TODO: We could use a proper queue. + // TODO: We could use a proper queue, e.g. implemented with a linked + // list. private var assetRequestQueue = [GiphyAssetRequest]() - private var isDownloading = false + private let kMaxAssetRequestCount = 3 + private var activeAssetRequests = Set() // The success and failure handlers are always called on main queue. // The success and failure handlers may be called synchronously on cache hit. @@ -165,69 +224,60 @@ extension URLSessionTask { return nil } - var hasRequestCompleted = false let assetRequest = GiphyAssetRequest(rendition:rendition, priority:priority, - success : { asset in - DispatchQueue.main.async { - // Ensure we call success or failure exactly once. - guard !hasRequestCompleted else { - return - } - hasRequestCompleted = true - - self.assetMap.set(key:rendition.url, value:asset) - self.isDownloading = false - self.downloadIfNecessary() - success(asset) - } - }, - failure : { - DispatchQueue.main.async { - // Ensure we call success or failure exactly once. - guard !hasRequestCompleted else { - return - } - hasRequestCompleted = true - - self.isDownloading = false - self.downloadIfNecessary() - failure() - } - }) + success : success, + failure : failure) assetRequestQueue.append(assetRequest) downloadIfNecessary() return assetRequest } + private func assetRequestDidSucceed(assetRequest: GiphyAssetRequest, asset: GiphyAsset) { + DispatchQueue.main.async { + self.assetMap.set(key:assetRequest.rendition.url, value:asset) + self.activeAssetRequests.remove(assetRequest) + assetRequest.requestDidSucceed(asset:asset) + self.downloadIfNecessary() + } + } + + private func assetRequestDidFail(assetRequest: GiphyAssetRequest) { + DispatchQueue.main.async { + self.activeAssetRequests.remove(assetRequest) + assetRequest.requestDidFail() + self.downloadIfNecessary() + } + } + private func downloadIfNecessary() { AssertIsOnMainThread() DispatchQueue.main.async { - guard !self.isDownloading else { + guard self.activeAssetRequests.count < self.kMaxAssetRequestCount else { return } guard let assetRequest = self.popNextAssetRequest() else { return } guard !assetRequest.wasCancelled else { - DispatchQueue.main.async { - self.downloadIfNecessary() - } + // Discard the cancelled asset request and try again. + self.downloadIfNecessary() return } - self.isDownloading = true + self.activeAssetRequests.insert(assetRequest) if let asset = self.assetMap.get(key:assetRequest.rendition.url) { // Deferred cache hit, avoids re-downloading assets already in the // asset cache. - assetRequest.success(asset) + + self.assetRequestDidSucceed(assetRequest : assetRequest, asset: asset) return } guard let downloadSession = self.giphyDownloadSession() else { Logger.error("\(GifDownloader.TAG) Couldn't create session manager.") - assetRequest.failure() + self.assetRequestDidFail(assetRequest:assetRequest) return } @@ -266,32 +316,32 @@ extension URLSessionTask { let assetRequest = task.assetRequest guard !assetRequest.wasCancelled else { task.cancel() - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } if let error = error { Logger.error("\(GifDownloader.TAG) download failed with error: \(error)") - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } guard let httpResponse = task.response as? HTTPURLResponse else { Logger.error("\(GifDownloader.TAG) missing or unexpected response: \(task.response)") - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } let statusCode = httpResponse.statusCode guard statusCode >= 200 && statusCode < 400 else { Logger.error("\(GifDownloader.TAG) response has invalid status code: \(statusCode)") - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } guard let assetFilePath = assetRequest.assetFilePath else { Logger.error("\(GifDownloader.TAG) task is missing asset file") - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } let asset = GiphyAsset(rendition: assetRequest.rendition, filePath : assetFilePath) - assetRequest.success(asset) + assetRequestDidSucceed(assetRequest : assetRequest, asset: asset) } // MARK: URLSessionDownloadDelegate @@ -300,7 +350,7 @@ extension URLSessionTask { let assetRequest = downloadTask.assetRequest guard !assetRequest.wasCancelled else { downloadTask.cancel() - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } @@ -321,7 +371,7 @@ extension URLSessionTask { let assetRequest = downloadTask.assetRequest guard !assetRequest.wasCancelled else { downloadTask.cancel() - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } } @@ -330,7 +380,7 @@ extension URLSessionTask { let assetRequest = downloadTask.assetRequest guard !assetRequest.wasCancelled else { downloadTask.cancel() - assetRequest.failure() + assetRequestDidFail(assetRequest:assetRequest) return } } diff --git a/Signal/src/network/PushManager.m b/Signal/src/network/PushManager.m index 479f2e817..bc75a3766 100644 --- a/Signal/src/network/PushManager.m +++ b/Signal/src/network/PushManager.m @@ -134,7 +134,7 @@ NSString *const Signal_Message_MarkAsRead_Identifier = @"Signal_Message_MarkAsRe - (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification { DDLogInfo(@"received: %s", __PRETTY_FUNCTION__); - NSString *threadId = notification.userInfo[Signal_Thread_UserInfo_Key]; + NSString *_Nullable threadId = notification.userInfo[Signal_Thread_UserInfo_Key]; if (threadId) { [Environment presentConversationForThreadId:threadId];