diff --git a/Signal/src/ViewControllers/MediaDetailViewController.h b/Signal/src/ViewControllers/MediaDetailViewController.h index 37aca540c..18df8e748 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.h +++ b/Signal/src/ViewControllers/MediaDetailViewController.h @@ -7,9 +7,8 @@ NS_ASSUME_NONNULL_BEGIN @class ConversationViewItem; +@class GalleryItemBox; @class MediaDetailViewController; -@class SignalAttachment; -@class TSAttachmentStream; @protocol MediaDetailViewControllerDelegate @@ -22,14 +21,11 @@ NS_ASSUME_NONNULL_BEGIN @interface MediaDetailViewController : OWSViewController @property (nonatomic, weak) id delegate; +@property (nonatomic, readonly) GalleryItemBox *galleryItemBox; // If viewItem is non-null, long press will show a menu controller. -- (instancetype)initWithAttachmentStream:(TSAttachmentStream *)attachmentStream - viewItem:(ConversationViewItem *_Nullable)viewItem; - -- (void)presentFromViewController:(UIViewController *)viewController - replacingView:(UIView *)view NS_SWIFT_NAME(present(fromViewController:replacingView:)); - +- (instancetype)initWithGalleryItemBox:(GalleryItemBox *)galleryItemBox + viewItem:(ConversationViewItem *_Nullable)viewItem; #pragma mark - Actions - (void)didPressShare:(id)sender; diff --git a/Signal/src/ViewControllers/MediaDetailViewController.m b/Signal/src/ViewControllers/MediaDetailViewController.m index 2bfaac8fd..8f196f1b0 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.m +++ b/Signal/src/ViewControllers/MediaDetailViewController.m @@ -58,10 +58,9 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic) UIView *replacingView; @property (nonatomic) UIButton *shareButton; -@property (nonatomic) CGRect originRect; @property (nonatomic) NSData *fileData; -@property (nonatomic, nullable) TSAttachmentStream *attachmentStream; +@property (nonatomic) TSAttachmentStream *attachmentStream; @property (nonatomic, nullable) ConversationViewItem *viewItem; @property (nonatomic, nullable) OWSVideoPlayer *videoPlayer; @@ -80,20 +79,25 @@ NS_ASSUME_NONNULL_BEGIN @implementation MediaDetailViewController -- (instancetype)initWithAttachmentStream:(TSAttachmentStream *)attachmentStream - viewItem:(ConversationViewItem *_Nullable)viewItem +- (instancetype)initWithGalleryItemBox:(GalleryItemBox *)galleryItemBox + viewItem:(ConversationViewItem *_Nullable)viewItem { self = [super initWithNibName:nil bundle:nil]; if (!self) { return self; } - self.attachmentStream = attachmentStream; - self.viewItem = viewItem; + _galleryItemBox = galleryItemBox; + _viewItem = viewItem; return self; } +- (TSAttachmentStream *)attachmentStream +{ + return self.galleryItemBox.attachmentStream; +} + - (NSURL *_Nullable)attachmentUrl { return self.attachmentStream.mediaURL; diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index f871db288..562948a8c 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -8,7 +8,7 @@ public enum GalleryDirection { case before, after, around } -public struct MediaGalleryItem: Equatable { +public struct MediaGalleryItem: Equatable, Hashable { let logTag = "[MediaGalleryItem]" let message: TSMessage @@ -48,6 +48,13 @@ public struct MediaGalleryItem: Equatable { public static func == (lhs: MediaGalleryItem, rhs: MediaGalleryItem) -> Bool { return lhs.message.uniqueId == rhs.message.uniqueId } + + // MARK: Hashable + + public var hashValue: Int { + return message.hashValue + } + } public struct GalleryDate: Hashable, Comparable, Equatable { @@ -226,8 +233,8 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource // MARK: Present/Dismiss - private var currentPage: MediaGalleryPage? { - return self.pageViewController!.currentPage + private var currentItem: MediaGalleryItem { + return self.pageViewController!.currentItem } private var replacingView: UIView? @@ -327,15 +334,7 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource self.view.isUserInteractionEnabled = true - guard let currentPage = self.currentPage else { - owsFail("\(self.logTag) in \(#function) currentPage was unexpectedly nil") - self.dismissSelf(animated: false, completion: nil) - return - } - - if currentPage.isVideo { - currentPage.viewController.playVideo() - } + pageViewController.wasPresented() }) } } @@ -391,12 +390,6 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource self.view.isUserInteractionEnabled = false UIApplication.shared.isStatusBarHidden = false - guard let currentPage = self.currentPage else { - owsFail("\(logTag) in \(#function) currentItem was unexpectedly nil") - self.presentingViewController?.dismiss(animated: false, completion: completion) - return - } - guard let detailView = pageViewController?.view else { owsFail("\(logTag) in \(#function) detailView was unexpectedly nil") self.presentingViewController?.dismiss(animated: false, completion: completion) @@ -407,9 +400,9 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource // Move the presentationView back to it's initial position, i.e. where // it sits on the screen in the conversation view. - let changedItems = currentPage.galleryItem != self.initialDetailItem + let changedItems = currentItem != self.initialDetailItem if changedItems { - self.presentationView.image = currentPage.image + self.presentationView.image = currentItem.fullSizedImage self.applyOffscreenMediaViewConstraints() } else { self.applyInitialMediaViewConstraints() diff --git a/Signal/src/ViewControllers/MediaPageViewController.swift b/Signal/src/ViewControllers/MediaPageViewController.swift index c115972a5..29e584338 100644 --- a/Signal/src/ViewControllers/MediaPageViewController.swift +++ b/Signal/src/ViewControllers/MediaPageViewController.swift @@ -4,32 +4,30 @@ import UIKit -public struct MediaGalleryPage: Equatable { - - public let viewController: MediaDetailViewController - public let galleryItem: MediaGalleryItem - - public var message: TSMessage { - return galleryItem.message +// Objc wrapper for the MediaGalleryItem struct +@objc +public class GalleryItemBox: NSObject { + public let value: MediaGalleryItem + + init(_ value: MediaGalleryItem) { + self.value = value } - + public var attachmentStream: TSAttachmentStream { - return galleryItem.attachmentStream - } - - public var isVideo: Bool { - return galleryItem.isVideo + return value.attachmentStream } +} - public var image: UIImage { - // TODO cache this - return galleryItem.fullSizedImage +fileprivate class Box { + var value: A + init(_ val: A) { + self.value = val } +} - // MARK: Equatable - - public static func == (lhs: MediaGalleryPage, rhs: MediaGalleryPage) -> Bool { - return lhs.galleryItem == rhs.galleryItem +fileprivate extension MediaDetailViewController { + fileprivate var galleryItem: MediaGalleryItem { + return self.galleryItemBox.value } } @@ -37,26 +35,24 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou private weak var mediaGalleryDataSource: MediaGalleryDataSource? - private var cachedPages: [MediaGalleryPage] = [] - private var initialPage: MediaGalleryPage! + private var cachedPages: [MediaGalleryItem: MediaDetailViewController] = [:] + private var initialPage: MediaDetailViewController! - public var currentPage: MediaGalleryPage! { - return cachedPages.first { $0.viewController == viewControllers?.first } + private var currentViewController: MediaDetailViewController { + return viewControllers!.first as! MediaDetailViewController } public var currentItem: MediaGalleryItem! { get { - return currentPage.galleryItem + return currentViewController.galleryItemBox.value } set { - // FIXME cache separate from ordering so we don't have to clear cache guard let galleryPage = self.buildGalleryPage(galleryItem: newValue) else { - owsFail("unexpetedly unable to build initial gallery item") + owsFail("unexpetedly unable to build new gallery page") return } - self.cachedPages = [galleryPage] - self.setViewControllers([galleryPage.viewController], direction: .forward, animated: false, completion: nil) + self.setViewControllers([galleryPage], direction: .forward, animated: false, completion: nil) } } @@ -88,8 +84,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou return } self.initialPage = initialPage - cachedPages = [initialPage] - self.setViewControllers([initialPage.viewController], direction: .forward, animated: false, completion: nil) + self.setViewControllers([initialPage], direction: .forward, animated: false, completion: nil) } @available(*, unavailable, message: "Unimplemented") @@ -175,8 +170,22 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou view.addGestureRecognizer(verticalSwipe) } + override func didReceiveMemoryWarning() { + Logger.info("\(logTag) in \(#function)") + super.didReceiveMemoryWarning() + + self.cachedPages = [:] + } + // MARK: View Helpers + public func wasPresented() { + let currentViewController = self.currentViewController + + if currentViewController.galleryItem.isVideo { + currentViewController.playVideo() + } + } @objc public func didPressAllMediaButton(sender: Any) { Logger.debug("\(logTag) in \(#function)") @@ -218,7 +227,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou self.view.backgroundColor = shouldHideToolbars ? UIColor.black : UIColor.white UIView.animate(withDuration: 0.1) { - self.currentPage.viewController.setShouldHideToolbars(self.shouldHideToolbars) + self.currentViewController.setShouldHideToolbars(self.shouldHideToolbars) self.footerBar.alpha = self.shouldHideToolbars ? 0 : 1 } } @@ -237,7 +246,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target:nil, action:nil) ] - if (self.currentPage.isVideo) { + if (self.currentItem.isVideo) { toolbarItems += [ isPlayingVideo ? self.videoPauseBarButton : self.videoPlayBarButton, UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target:nil, action:nil) @@ -301,13 +310,13 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou assert(pendingViewControllers.count == 1) pendingViewControllers.forEach { viewController in - guard let pendingPage = self.cachedPages.first(where: { $0.viewController == viewController}) else { + guard let pendingPage = viewController as? MediaDetailViewController else { owsFail("\(logTag) in \(#function) unexpected mediaDetailViewController: \(viewController)") return } // Ensure upcoming page respects current toolbar status - pendingPage.viewController.setShouldHideToolbars(self.shouldHideToolbars) + pendingPage.setShouldHideToolbars(self.shouldHideToolbars) } } @@ -316,16 +325,17 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou assert(previousViewControllers.count == 1) previousViewControllers.forEach { viewController in - guard let previousPage = self.cachedPages.first(where: { $0.viewController == viewController}) else { + guard let previousPage = viewController as? MediaDetailViewController else { owsFail("\(logTag) in \(#function) unexpected mediaDetailViewController: \(viewController)") return } // Do any cleanup for the no-longer visible view controller if transitionCompleted { - previousPage.viewController.zoomOut(animated: false) - if previousPage.isVideo { - previousPage.viewController.stopVideo() + previousPage.zoomOut(animated: false) + + if previousPage.galleryItem.isVideo { + previousPage.stopVideo() } updateFooterBarButtonItems(isPlayingVideo: false) } @@ -337,66 +347,62 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou public func pageViewController(_ pageViewController: UIPageViewController, viewControllerBefore viewController: UIViewController) -> UIViewController? { Logger.debug("\(logTag) in \(#function)") - guard let currentIndex = cachedPages.index(where: { $0.viewController == viewController }) else { - owsFail("\(self.logTag) unknown view controller. \(viewController)") + guard let previousDetailViewController = viewController as? MediaDetailViewController else { + owsFail("\(logTag) in \(#function) unexpected viewController: \(viewController)") return nil } - let currentPage = cachedPages[currentIndex] - - let newIndex = currentIndex - 1 - if let cachedPage = cachedPages[safe: newIndex] { - return cachedPage.viewController - } guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") return nil } - guard let previousItem: MediaGalleryItem = mediaGalleryDataSource.galleryItem(before: currentPage.galleryItem) else { + let previousItem = previousDetailViewController.galleryItem + guard let nextItem: MediaGalleryItem = mediaGalleryDataSource.galleryItem(before: previousItem) else { return nil } - guard let previousPage: MediaGalleryPage = buildGalleryPage(galleryItem: previousItem) else { + guard let nextPage: MediaDetailViewController = buildGalleryPage(galleryItem: nextItem) else { return nil } - cachedPages.insert(previousPage, at: currentIndex) - return previousPage.viewController + return nextPage } public func pageViewController(_ pageViewController: UIPageViewController, viewControllerAfter viewController: UIViewController) -> UIViewController? { Logger.debug("\(logTag) in \(#function)") - guard let currentIndex = cachedPages.index(where: { $0.viewController == viewController }) else { - owsFail("\(self.logTag) unknown view controller. \(viewController)") + guard let previousDetailViewController = viewController as? MediaDetailViewController else { + owsFail("\(logTag) in \(#function) unexpected viewController: \(viewController)") return nil } - let currentPage = cachedPages[currentIndex] - - let newIndex = currentIndex + 1 - if let cachedPage = cachedPages[safe: newIndex] { - return cachedPage.viewController - } guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") return nil } - guard let nextItem: MediaGalleryItem = mediaGalleryDataSource.galleryItem(after: currentPage.galleryItem) else { + let previousItem = previousDetailViewController.galleryItem + guard let nextItem = mediaGalleryDataSource.galleryItem(after: previousItem) else { + // no more pages return nil } - guard let nextPage: MediaGalleryPage = buildGalleryPage(galleryItem: nextItem) else { + guard let nextPage: MediaDetailViewController = buildGalleryPage(galleryItem: nextItem) else { return nil } - cachedPages.insert(nextPage, at: newIndex) - return nextPage.viewController + return nextPage } - private func buildGalleryPage(galleryItem: MediaGalleryItem) -> MediaGalleryPage? { + private func buildGalleryPage(galleryItem: MediaGalleryItem) -> MediaDetailViewController? { + + if let cachedPage = cachedPages[galleryItem] { + Logger.debug("\(logTag) in \(#function) cache hit.") + return cachedPage + } + + Logger.debug("\(logTag) in \(#function) cache miss.") var fetchedItem: ConversationViewItem? = nil self.uiDatabaseConnection.read { transaction in let message = galleryItem.message @@ -405,21 +411,23 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou } guard let viewItem = fetchedItem else { - owsFail("viewItem stream unexpectedly nil") + owsFail("viewItem was unexpectedly nil") return nil } - let viewController = MediaDetailViewController(attachmentStream: galleryItem.attachmentStream, viewItem: viewItem) + let viewController = MediaDetailViewController(galleryItemBox: GalleryItemBox(galleryItem), viewItem: viewItem) viewController.delegate = self - return MediaGalleryPage(viewController: viewController, galleryItem: galleryItem) + cachedPages[galleryItem] = viewController + return viewController } // MARK: MediaDetailViewControllerDelegate public func dismissSelf(animated isAnimated: Bool, completion: (() -> Void)? = nil) { // Swapping mediaView for presentationView will be perceptible if we're not zoomed out all the way. - currentPage.viewController.zoomOut(animated: true) + // currentVC + currentViewController.zoomOut(animated: true) guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") @@ -432,7 +440,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou } public func mediaDetailViewController(_ mediaDetailViewController: MediaDetailViewController, isPlayingVideo: Bool) { - guard mediaDetailViewController == currentPage.viewController else { + guard mediaDetailViewController == currentViewController else { Logger.verbose("\(logTag) in \(#function) ignoring stale delegate.") return } diff --git a/SignalMessaging/utils/Bench.swift b/SignalMessaging/utils/Bench.swift index c1a7f17a8..e11e4622b 100644 --- a/SignalMessaging/utils/Bench.swift +++ b/SignalMessaging/utils/Bench.swift @@ -9,7 +9,7 @@ public func BenchAsync(title: String, block: (() -> Void) -> Void) { block { let timeElapsed = CFAbsoluteTimeGetCurrent() - startTime - print("[Bench] title: \(title), duration: \(timeElapsed)") + Logger.debug("[Bench] title: \(title), duration: \(timeElapsed)") } }