From fb4182c41f1d1c8ff044c2128510bfc0f588a18e Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 19 Mar 2018 13:44:59 -0400 Subject: [PATCH] Ensure gallery is GC'd // FREEBIE --- .../MediaGalleryViewController.swift | 5 ++- .../MediaPageViewController.swift | 28 +++++++++++-- .../MediaTileViewController.swift | 41 ++++++++++++++----- 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index dfd3764a9..f871db288 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -158,7 +158,6 @@ protocol MediaGalleryDataSource: class { func galleryItem(before currentItem: MediaGalleryItem) -> MediaGalleryItem? func galleryItem(after currentItem: MediaGalleryItem) -> MediaGalleryItem? - // TODO this doesn't seem very "data-source" func showAllMedia(focusedItem: MediaGalleryItem) func dismissSelf(animated isAnimated: Bool, completion: (() -> Void)?) } @@ -177,6 +176,10 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource // we start with a small range size for quick loading. private let fetchRangeSize: UInt = 10 + deinit { + Logger.debug("\(logTag) deinit") + } + convenience init(thread: TSThread, uiDatabaseConnection: YapDatabaseConnection) { self.init(thread: thread, uiDatabaseConnection: uiDatabaseConnection, includeGallery: true) } diff --git a/Signal/src/ViewControllers/MediaPageViewController.swift b/Signal/src/ViewControllers/MediaPageViewController.swift index 2ba54941b..c115972a5 100644 --- a/Signal/src/ViewControllers/MediaPageViewController.swift +++ b/Signal/src/ViewControllers/MediaPageViewController.swift @@ -35,7 +35,7 @@ public struct MediaGalleryPage: Equatable { class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSource, UIPageViewControllerDelegate, MediaDetailViewControllerDelegate { - let mediaGalleryDataSource: MediaGalleryDataSource + private weak var mediaGalleryDataSource: MediaGalleryDataSource? private var cachedPages: [MediaGalleryPage] = [] private var initialPage: MediaGalleryPage! @@ -181,7 +181,11 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou public func didPressAllMediaButton(sender: Any) { Logger.debug("\(logTag) in \(#function)") - self.mediaGalleryDataSource.showAllMedia(focusedItem: currentItem) + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + return + } + mediaGalleryDataSource.showAllMedia(focusedItem: currentItem) } @objc @@ -344,6 +348,11 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou 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 { return nil } @@ -370,6 +379,11 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou 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 { return nil } @@ -406,7 +420,15 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou 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) - self.mediaGalleryDataSource.dismissSelf(animated: isAnimated, completion: completion) + + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + self.presentingViewController?.dismiss(animated: true) + + return + } + + mediaGalleryDataSource.dismissSelf(animated: isAnimated, completion: completion) } public func mediaDetailViewController(_ mediaDetailViewController: MediaDetailViewController, isPlayingVideo: Bool) { diff --git a/Signal/src/ViewControllers/MediaTileViewController.swift b/Signal/src/ViewControllers/MediaTileViewController.swift index c5ee4fc53..7310356fe 100644 --- a/Signal/src/ViewControllers/MediaTileViewController.swift +++ b/Signal/src/ViewControllers/MediaTileViewController.swift @@ -10,13 +10,20 @@ public protocol MediaTileViewControllerDelegate: class { public class MediaTileViewController: UICollectionViewController, MediaGalleryCellDelegate { - // TODO weak? - private var mediaGalleryDataSource: MediaGalleryDataSource + private weak var mediaGalleryDataSource: MediaGalleryDataSource? private var galleryItems: [GalleryDate: [MediaGalleryItem]] { + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + return [:] + } return mediaGalleryDataSource.sections } private var galleryDates: [GalleryDate] { + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + return [] + } return mediaGalleryDataSource.sectionDates } public var focusedItem: MediaGalleryItem? @@ -25,6 +32,10 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe public weak var delegate: MediaTileViewControllerDelegate? + deinit { + Logger.debug("\(logTag) deinit") + } + init(mediaGalleryDataSource: MediaGalleryDataSource, uiDatabaseConnection: YapDatabaseConnection) { self.mediaGalleryDataSource = mediaGalleryDataSource @@ -250,10 +261,18 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe switch section { case kLoadOlderSectionIdx: // Show "loading older..." iff there is still older data to be fetched - return self.mediaGalleryDataSource.hasFetchedOldest ? CGSize.zero : CGSize(width: 0, height: 100) + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + return CGSize.zero + } + return mediaGalleryDataSource.hasFetchedOldest ? CGSize.zero : CGSize(width: 0, height: 100) case kLoadNewerSectionIdx: // Show "loading newer..." iff there is still more recent data to be fetched - return self.mediaGalleryDataSource.hasFetchedMostRecent ? CGSize.zero : CGSize(width: 0, height: 100) + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + return CGSize.zero + } + return mediaGalleryDataSource.hasFetchedMostRecent ? CGSize.zero : CGSize(width: 0, height: 100) default: return CGSize(width: 0, height: kHeaderHeight) } @@ -305,6 +324,11 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe return } + guard let mediaGalleryDataSource = self.mediaGalleryDataSource else { + owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil") + return + } + let contentOffsetY = collectionView.contentOffset.y let oldContentHeight = collectionView.contentSize.height @@ -326,7 +350,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe let scrollDistanceToBottom = oldContentHeight - contentOffsetY collectionView.performBatchUpdates({ - self.mediaGalleryDataSource.ensureGalleryItemsLoaded(.before, item: oldestLoadedItem, amount: self.kMediaTileViewLoadBatchSize) { addedSections, addedItems in + mediaGalleryDataSource.ensureGalleryItemsLoaded(.before, item: oldestLoadedItem, amount: self.kMediaTileViewLoadBatchSize) { addedSections, addedItems in Logger.debug("\(self.logTag) in \(#function) insertingSections: \(addedSections) items: \(addedItems)") collectionView.insertSections(addedSections) @@ -357,13 +381,8 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe isFetchingMoreData = true collectionView.performBatchUpdates({ - self.mediaGalleryDataSource.ensureGalleryItemsLoaded(.after, item: mostRecentLoadedItem, amount: self.kMediaTileViewLoadBatchSize) { addedSections, addedItems in - guard let collectionView = self.collectionView else { - Logger.debug("\(self.logTag) in \(#function) collectionView was unexpectedly nil") - return - } + mediaGalleryDataSource.ensureGalleryItemsLoaded(.after, item: mostRecentLoadedItem, amount: self.kMediaTileViewLoadBatchSize) { addedSections, addedItems in Logger.debug("\(self.logTag) in \(#function) insertingSections: \(addedSections), items: \(addedItems)") - collectionView.insertSections(addedSections) collectionView.insertItems(at: addedItems) }