Skip to content

Commit

Permalink
Break DuckPlayer ref cycle (#3206)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations. Feel free to change it, although assigning a GitHub
reviewer and the items in bold are required.

⚠️ If you're an external contributor, please file an issue first before
working on a PR, as we can't guarantee that we will accept your changes
if they haven't been discussed ahead of time. Thanks!
-->

Task/Issue URL:
https://app.asana.com/0/414709148257752/1207998631270910/f

**Description**:
Breaks reference cycle between DuckPlayer and TabViewController, which
can contribute to containers not being properly cleaned up

**Steps to test this PR**:
1.  Fire up the app, navigate away
2. Check Memory Graph, confirm the duckPlayer property in
TabViewController is now a weak reference,
3. Fire button everything.  
4. There should not be leftovers.
  • Loading branch information
afterxleep authored Aug 8, 2024
1 parent 9b9ae1f commit dd4bfb9
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 8 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo/DuckPlayer/DuckNavigationHandling.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import WebKit

protocol DuckNavigationHandling {
protocol DuckNavigationHandling: AnyObject {
var referrer: DuckPlayerReferrer { get set }
var duckPlayer: DuckPlayerProtocol { get }
func handleNavigation(_ navigationAction: WKNavigationAction, webView: WKWebView)
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/DuckPlayer/DuckPlayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public enum DuckPlayerReferrer {
case youtube, other
}

protocol DuckPlayerProtocol {
protocol DuckPlayerProtocol: AnyObject {

var settings: DuckPlayerSettingsProtocol { get }
var hostView: UIViewController? { get }
Expand All @@ -103,7 +103,7 @@ final class DuckPlayer: DuckPlayerProtocol {
static let commonName = "Duck Player"

private(set) var settings: DuckPlayerSettingsProtocol
private(set) var hostView: UIViewController?
private(set) weak var hostView: UIViewController?

init(settings: DuckPlayerSettingsProtocol = DuckPlayerSettings()) {
self.settings = settings
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/DuckPlayer/DuckPlayerSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ enum DuckPlayerMode: Equatable, Codable, CustomStringConvertible, CaseIterable {
}
}

protocol DuckPlayerSettingsProtocol {
protocol DuckPlayerSettingsProtocol: AnyObject {

var duckPlayerSettingsPublisher: AnyPublisher<Void, Never> { get }
var mode: DuckPlayerMode { get }
Expand Down
10 changes: 6 additions & 4 deletions DuckDuckGo/TabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class TabViewController: UIViewController {
bookmarksDatabase: CoreDataDatabase,
historyManager: HistoryManaging,
syncService: DDGSyncing,
duckPlayer: DuckPlayerProtocol,
duckPlayer: DuckPlayerProtocol?,
privacyProDataReporter: PrivacyProDataReporting,
contextualOnboardingPresenter: ContextualOnboardingPresenting,
contextualOnboardingLogic: ContextualOnboardingLogic,
Expand Down Expand Up @@ -323,7 +323,7 @@ class TabViewController: UIViewController {

let historyManager: HistoryManaging
let historyCapture: HistoryCapture
var duckPlayer: DuckPlayerProtocol
weak var duckPlayer: DuckPlayerProtocol?
var duckPlayerNavigationHandler: DuckNavigationHandling?

let contextualOnboardingPresenter: ContextualOnboardingPresenting
Expand All @@ -336,7 +336,7 @@ class TabViewController: UIViewController {
bookmarksDatabase: CoreDataDatabase,
historyManager: HistoryManaging,
syncService: DDGSyncing,
duckPlayer: DuckPlayerProtocol,
duckPlayer: DuckPlayerProtocol?,
privacyProDataReporter: PrivacyProDataReporting,
contextualOnboardingPresenter: ContextualOnboardingPresenting,
contextualOnboardingLogic: ContextualOnboardingLogic,
Expand All @@ -348,7 +348,9 @@ class TabViewController: UIViewController {
self.historyCapture = HistoryCapture(historyManager: historyManager)
self.syncService = syncService
self.duckPlayer = duckPlayer
self.duckPlayerNavigationHandler = DuckPlayerNavigationHandler(duckPlayer: duckPlayer)
if let duckPlayer {
self.duckPlayerNavigationHandler = DuckPlayerNavigationHandler(duckPlayer: duckPlayer)
}
self.privacyProDataReporter = privacyProDataReporter
self.contextualOnboardingPresenter = contextualOnboardingPresenter
self.contextualOnboardingLogic = contextualOnboardingLogic
Expand Down

0 comments on commit dd4bfb9

Please sign in to comment.