Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ditto instance retain cycle in PresenceViewer #5

Merged
merged 1 commit into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 53 additions & 26 deletions Swift/Sources/DittoPresenceViewer/DittoPresenceView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ public typealias PlatformViewController = NSViewController
let ditto: Ditto

func showPresence() {
let presenceView = DittoPresenceView(ditto: ditto)
// maybe add it to an existing view
self.view.addSubview(presenceView)
let presenceView = DittoPresenceView(ditto: ditto)
// maybe add it to an existing view
self.view.addSubview(presenceView)

// or add it as a view controller
let viewController = DittoPresenceView(ditto: ditto).viewController
present(viewController: viewController, animated: true)
// or add it as a view controller
let viewController = DittoPresenceView(ditto: ditto).viewController
present(viewController: viewController, animated: true)
}
```
*/
Expand All @@ -55,12 +55,26 @@ public class DittoPresenceView: PlatformView {
The `DittoPresenceView` will not display any presence information
until the `ditto` property has been set.
*/
public var ditto: Ditto? {
public weak var ditto: Ditto? {
willSet {
if (ditto !== newValue) {
self.stopObservingPresence()
}
}

didSet {
observePresence()
if (oldValue !== ditto) {
self.startObservingPresence()
}
}
}

deinit {
stopObservingPresence()
webView.removeFromSuperview()
_vc = nil
}

/**
Returns a `UIViewController` containing this view.
*/
Expand All @@ -73,7 +87,17 @@ public class DittoPresenceView: PlatformView {
// The inversion will likely trip up more experienced native iOS
// developers, but they're also the group likely to be designing their
// own `UIViewControllers` which renders this a non-issue in most cases.
return _vc
//
// Also note that we can't hold the view controller strongly here because
// this would lead to a retain cycle. Therefore, we'll hold it weakly
// and create a new one whenever the previous one is dealloced:
if let vc = self._vc {
return vc
}

let vc = DittoPresenceViewController(view: self)
self._vc = vc
return vc
}

// MARK: Private Properties
Expand All @@ -82,22 +106,20 @@ public class DittoPresenceView: PlatformView {

private var webView = VisJSWebView()

private lazy var _vc: PlatformViewController! = DittoPresenceViewController(view: self)
private weak var _vc: DittoPresenceViewController?

// MARK: Initializer

/**
Initializes a new `DittoPresenceView`.

- Parameter ditto: A reference to the `Ditto` which you would like
to visualize presence status for.
to visualize presence status for.
*/
public convenience init(ditto: Ditto) {
self.init(frame: .zero)
self.ditto = ditto

setup()
observePresence()
startObservingPresence()
}

public override init(frame: CGRect) {
Expand All @@ -110,9 +132,11 @@ public class DittoPresenceView: PlatformView {
setup()
}


// MARK: Private Functions

private func setup() {

#if canImport(UIKit)
backgroundColor = .clear
#endif
Expand All @@ -127,9 +151,9 @@ public class DittoPresenceView: PlatformView {
])
}

private func observePresence() {
private func startObservingPresence() {
if let ditto = ditto {
peersObserver = ditto.presence.observe { presenceGraph in
self.peersObserver = ditto.presence.observe { presenceGraph in
let encoder = JSONEncoder()
encoder.dataEncodingStrategy = .custom({ data, enc in
var container = enc.unkeyedContainer()
Expand All @@ -144,16 +168,19 @@ public class DittoPresenceView: PlatformView {
}
}
}

// // Comment out the ditto observer above and toggle following to test presence with
// // fake data. Several different mock drivers exist:
// // - runFrequentConnectionChanges()
// // - runFrequentRSSIChanges()
// // - runLargeMesh()
// // - runMassiveMesh()
// MockData.runLargeMesh() { [weak self] json in
// self?.webView.updateNetwork(json: json, completionHandler: nil)
// }
// // Comment out the ditto observer above and toggle following to test presence with
// // fake data. Several different mock drivers exist:
// // - runFrequentConnectionChanges()
// // - runFrequentRSSIChanges()
// // - runLargeMesh()
// // - runMassiveMesh()
// MockData.runLargeMesh() { [weak self] json in
// self?.webView.updateNetwork(json: json, completionHandler: nil)
// }
}

private func stopObservingPresence() {
peersObserver?.stop()
peersObserver = nil
}
}
36 changes: 16 additions & 20 deletions Swift/Sources/DittoPresenceViewer/PresenceView.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//
// PresenceView.swift
//
//
//
// Created by Ben Chatelain on 9/23/22.
//
Expand All @@ -20,47 +20,43 @@ import AppKit
#endif

#if canImport(WebKit)
@available(macOS 10.15, *)
public struct PresenceView: View {
public var ditto: Ditto
private var ditto: Ditto

public init(ditto: Ditto) {
self.ditto = ditto
}

public var body: some View {
PresenceView(ditto: ditto)
DittoPresenceViewRepresentable(ditto: ditto)
}
}
#endif

// MARK: - UIViewRepresentable
#if os(iOS)
extension PresenceView: UIViewRepresentable {
public typealias Body = Never
public typealias UIViewType = UIView
private struct DittoPresenceViewRepresentable: UIViewRepresentable {
let ditto: Ditto

public func makeUIView(context: Self.Context) -> Self.UIViewType {
return DittoPresenceView(ditto: self.ditto)
func makeUIView(context: Context) -> UIView {
return DittoPresenceView(ditto: ditto)
}

public func updateUIView(_: Self.UIViewType, context: Self.Context) {
return
func updateUIView(_ uiView: UIView, context: Context) {
}
}
#endif

// MARK: - NSViewRepresentable
#if os(macOS)
extension PresenceView: NSViewRepresentable {
public typealias Body = Never
public typealias NSViewType = NSView
#elseif os(macOS)
private struct DittoPresenceViewRepresentable: NSViewRepresentable {
let ditto: Ditto

public func makeNSView(context: Self.Context) -> Self.NSViewType {
return DittoPresenceView(ditto: self.ditto)
func makeNSView(context: Context) -> NSView {
return DittoPresenceView(ditto: ditto)
}

public func updateNSView(_: Self.NSViewType, context: Self.Context) {
return
func updateNSView(_ nsView: NSView, context: Context) {
}
}
#endif
#endif