Skip to content

Commit

Permalink
Fix memory leak in PageboyViewController.Transition (#183)
Browse files Browse the repository at this point in the history 
* Clean up self references
  • Loading branch information
@msaps
msaps authored Oct 18, 2018
1 parent ce1178b commit 66ac273
Show file tree
Hide file tree
Showing 18 changed files with 285 additions and 361 deletions.
12 changes: 0 additions & 12 deletions Sources/Pageboy.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@
46ADAACD208F7EB200974529 /* PageboyViewController+ScrollDetection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAC6208F7EB100974529 /* PageboyViewController+ScrollDetection.swift */; };
46ADAACE208F7EB200974529 /* PageboyViewController+Management.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAC7208F7EB200974529 /* PageboyViewController+Management.swift */; };
46ADAACF208F7EB200974529 /* PageboyViewController+Management.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAC7208F7EB200974529 /* PageboyViewController+Management.swift */; };
46ADAAD2208F7ED600974529 /* IndexedMap.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAD0208F7ED600974529 /* IndexedMap.swift */; };
46ADAAD3208F7ED600974529 /* IndexedMap.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAD0208F7ED600974529 /* IndexedMap.swift */; };
46ADAAD4208F7ED600974529 /* WeakWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAD1208F7ED600974529 /* WeakWrapper.swift */; };
46ADAAD5208F7ED600974529 /* WeakWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46ADAAD1208F7ED600974529 /* WeakWrapper.swift */; };
D623B1C11E1D0C6B00527F3D /* Pageboy.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D623B1B71E1D0C6A00527F3D /* Pageboy.framework */; };
D623B1C61E1D0C6B00527F3D /* PageboyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D623B1C51E1D0C6B00527F3D /* PageboyTests.swift */; };
D623B1C81E1D0C6B00527F3D /* Pageboy.h in Headers */ = {isa = PBXBuildFile; fileRef = D623B1BA1E1D0C6A00527F3D /* Pageboy.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -150,8 +146,6 @@
46ADAAC4208F7EB100974529 /* PageboyViewController+Updating.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "PageboyViewController+Updating.swift"; sourceTree = "<group>"; };
46ADAAC6208F7EB100974529 /* PageboyViewController+ScrollDetection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "PageboyViewController+ScrollDetection.swift"; sourceTree = "<group>"; };
46ADAAC7208F7EB200974529 /* PageboyViewController+Management.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "PageboyViewController+Management.swift"; sourceTree = "<group>"; };
46ADAAD0208F7ED600974529 /* IndexedMap.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IndexedMap.swift; sourceTree = "<group>"; };
46ADAAD1208F7ED600974529 /* WeakWrapper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WeakWrapper.swift; sourceTree = "<group>"; };
D623B1B71E1D0C6A00527F3D /* Pageboy.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pageboy.framework; sourceTree = BUILT_PRODUCTS_DIR; };
D623B1BA1E1D0C6A00527F3D /* Pageboy.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Pageboy.h; sourceTree = "<group>"; };
D623B1BB1E1D0C6A00527F3D /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
Expand Down Expand Up @@ -374,8 +368,6 @@
isa = PBXGroup;
children = (
462A65D82000FCAA0051C79C /* Extensions */,
46ADAAD0208F7ED600974529 /* IndexedMap.swift */,
46ADAAD1208F7ED600974529 /* WeakWrapper.swift */,
468B6FBF20EF66A30038E26C /* ScrollObservationService.swift */,
);
path = Utilities;
Expand Down Expand Up @@ -664,10 +656,8 @@
D623B1D21E1D2DF200527F3D /* PageboyViewController.swift in Sources */,
46ADAAC0208F7E8500974529 /* TransitionOperation+Action.swift in Sources */,
46ADAACC208F7EB200974529 /* PageboyViewController+ScrollDetection.swift in Sources */,
46ADAAD2208F7ED600974529 /* IndexedMap.swift in Sources */,
46ADAABE208F7E8500974529 /* PageboyViewController+Transitioning.swift in Sources */,
464ADF52209717EF00929AFB /* NavigationDirection.swift in Sources */,
46ADAAD4208F7ED600974529 /* WeakWrapper.swift in Sources */,
46ADAACE208F7EB200974529 /* PageboyViewController+Management.swift in Sources */,
464ADF5F20975E5000929AFB /* UIViewController+Pageboy.swift in Sources */,
46ADAAB6208F7E1500974529 /* PageboyAutoScroller.swift in Sources */,
Expand Down Expand Up @@ -713,10 +703,8 @@
466A76B61FB38B32000B5C1C /* PageboyViewController.swift in Sources */,
46ADAAC1208F7E8500974529 /* TransitionOperation+Action.swift in Sources */,
46ADAACD208F7EB200974529 /* PageboyViewController+ScrollDetection.swift in Sources */,
46ADAAD3208F7ED600974529 /* IndexedMap.swift in Sources */,
46ADAABF208F7E8500974529 /* PageboyViewController+Transitioning.swift in Sources */,
464ADF53209717EF00929AFB /* NavigationDirection.swift in Sources */,
46ADAAD5208F7ED600974529 /* WeakWrapper.swift in Sources */,
46ADAACF208F7EB200974529 /* PageboyViewController+Management.swift in Sources */,
464ADF6020975E5000929AFB /* UIViewController+Pageboy.swift in Sources */,
46ADAAB7208F7E1500974529 /* PageboyAutoScroller.swift in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion Sources/Pageboy/AutoScrolling/PageboyViewController+AutoScrolling.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ import UIKit
extension PageboyViewController: PageboyAutoScrollerHandler {

func autoScroller(didRequestAutoScroll autoScroller: PageboyAutoScroller, animated: Bool) {
self.scrollToPage(.next, animated: animated)
scrollToPage(.next, animated: animated)
}
}
2 changes: 1 addition & 1 deletion Sources/Pageboy/Components/ParentMatchedScrollView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import UIKit

/// UIScrollView which has a content view that matches a parent view's height & width.
internal class ParentMatchedScrollView: UIScrollView {
internal final class ParentMatchedScrollView: UIScrollView {

// MARK: Properties

Expand Down
16 changes: 8 additions & 8 deletions Sources/Pageboy/Model/NavigationDirection.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// Copyright © 2018 UI At Six. All rights reserved.
//

import Foundation
import UIKit

public extension PageboyViewController {

Expand All @@ -33,25 +33,25 @@ internal extension PageboyViewController.NavigationDirection {
}

internal extension NavigationDirection {

static func forPage(_ page: Int,
previousPage: Int) -> NavigationDirection {
return self.forPosition(CGFloat(page), previous: CGFloat(previousPage))
return forPosition(CGFloat(page), previous: CGFloat(previousPage))
}

static func forPosition(_ position: CGFloat,
previous previousPosition: CGFloat) -> NavigationDirection {
if position == previousPosition {
return .neutral
}
return position > previousPosition ? .forward : .reverse
}

static func forPageScroll(to newPage: Page,
at index: Int,
in pageViewController: PageboyViewController) -> NavigationDirection {
var direction = NavigationDirection.forPage(index, previousPage: pageViewController.currentIndex ?? index)

if pageViewController.isInfiniteScrollEnabled {
switch newPage {
case .next:
Expand All @@ -62,13 +62,13 @@ internal extension NavigationDirection {
break
}
}

return direction
}
}

internal extension NavigationDirection {

func layoutNormalized(isRtL: Bool) -> NavigationDirection {
guard isRtL else {
return self
Expand Down
10 changes: 5 additions & 5 deletions Sources/Pageboy/Model/Page.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal extension Page {
static func indexValue(for page: Page,
in pageViewController: PageboyViewController) -> PageIndex {
switch page {

case .next:
guard let currentIndex = pageViewController.currentIndex else {
return 0
Expand All @@ -61,7 +61,7 @@ internal extension Page {
proposedIndex = 0
}
return proposedIndex

case .previous:
guard let currentIndex = pageViewController.currentIndex else {
return 0
Expand All @@ -71,13 +71,13 @@ internal extension Page {
proposedIndex = (pageViewController.pageCount ?? 1) - 1
}
return proposedIndex

case .first:
return 0

case .last:
return (pageViewController.pageCount ?? 1) - 1

case .at(let index):
return index
}
Expand Down
65 changes: 33 additions & 32 deletions Sources/Pageboy/PageboyViewController+Management.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,48 @@ public extension PageboyViewController {
/// This reloads the dataSource entirely, calling viewControllers(forPageboyViewController:)
/// and defaultPageIndex(forPageboyViewController:).
public func reloadData() {
self.reloadData(reloadViewControllers: true)
reloadData(reloadViewControllers: true)
}

/// Reload the pages in the PageboyViewController
///
/// - Parameter reloadViewControllers: Reload the view controller data source.
internal func reloadData(reloadViewControllers: Bool) {

if reloadViewControllers {
viewControllerMap.clear()
viewControllerMap.removeAll()
}
let viewControllerCount = dataSource?.numberOfViewControllers(in: self) ?? 0
self.viewControllerCount = viewControllerCount

let newViewControllerCount = dataSource?.numberOfViewControllers(in: self) ?? 0
viewControllerCount = newViewControllerCount

let defaultPage = self.dataSource?.defaultPage(for: self) ?? .first
let defaultIndex = defaultPage.indexValue(in: self)
guard defaultIndex < viewControllerCount,
let viewController = viewController(at: defaultIndex) else {

guard defaultIndex < newViewControllerCount,
let viewController = fetchViewController(at: defaultIndex) else {
return
}

updateViewControllers(to: [viewController], animated: false, async: false, force: false) { _ in
self.currentIndex = defaultIndex
self.delegate?.pageboyViewController(self,
didReloadWith: viewController,
currentPageIndex: defaultIndex)
updateViewControllers(to: [viewController], animated: false, async: false, force: false) { [weak self] _ in
self?.currentIndex = defaultIndex
if let self = self {
self.delegate?.pageboyViewController(self,
didReloadWith: viewController,
currentPageIndex: defaultIndex)
}
}
}

/// Reload the currently active page into the page view controller if possible.
internal func reloadCurrentPageSoftly() {
guard let currentIndex = self.currentIndex else {
guard let currentIndex = currentIndex else {
return
}
guard let currentViewController = viewController(at: currentIndex) else {
guard let currentViewController = fetchViewController(at: currentIndex) else {
return
}

updateViewControllers(to: [currentViewController],
animated: false,
async: false,
Expand All @@ -74,17 +76,17 @@ internal extension PageboyViewController {
async: Bool,
force: Bool,
completion: TransitionOperation.Completion?) {
guard let pageViewController = self.pageViewController else {
guard let pageViewController = pageViewController else {
return
}
if isUpdatingViewControllers && !force {
return
}


targetIndex = toIndex
isUpdatingViewControllers = true

let isUsingCustomTransition = transition != nil
if isUsingCustomTransition {
performTransition(from: fromIndex,
Expand Down Expand Up @@ -129,11 +131,10 @@ internal extension PageboyViewController {
///
/// - Parameter index: Index of the view controller to load.
/// - Returns: View controller if it exists.
func viewController(at index: PageIndex) -> UIViewController? {
func fetchViewController(at index: PageIndex) -> UIViewController? {
let viewController = dataSource?.viewController(for: self, at: index)
if let viewController = viewController {
let wrapper = WeakWrapper<UIViewController>(with: viewController)
viewControllerMap.set(object: wrapper, for: index)
viewControllerMap[viewController] = index

childScrollObserver.register(viewController: viewController, for: index)
}
Expand Down Expand Up @@ -224,31 +225,31 @@ extension PageboyViewController: UIPageViewControllerDataSource {

public func pageViewController(_ pageViewController: UIPageViewController,
viewControllerBefore viewController: UIViewController) -> UIViewController? {
guard let viewControllerCount = self.viewControllerCount else {
guard let viewControllerCount = viewControllerCount else {
return nil
}

if let index = currentIndex {
if index != 0 {
return self.viewController(at: index - 1)
return fetchViewController(at: index - 1)
} else if isInfiniteScrollEnabled {
return self.viewController(at: viewControllerCount - 1)
return fetchViewController(at: viewControllerCount - 1)
}
}
return nil
}

public func pageViewController(_ pageViewController: UIPageViewController,
viewControllerAfter viewController: UIViewController) -> UIViewController? {
guard let viewControllerCount = self.viewControllerCount else {
guard let viewControllerCount = viewControllerCount else {
return nil
}

if let index = currentIndex {
if index != viewControllerCount - 1 {
return self.viewController(at: index + 1)
return fetchViewController(at: index + 1)
} else if isInfiniteScrollEnabled {
return self.viewController(at: 0)
return fetchViewController(at: 0)
}
}
return nil
Expand Down
Loading

0 comments on commit 66ac273

Please sign in to comment.