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.

Sign up for GitHub

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

Jump to bottom

Remove NewTabPage retain cycles #3532

Merged
merged 3 commits into from
Nov 5, 2024
Merged

Remove NewTabPage retain cycles #3532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b6fbd4
Properly dismiss NewTabPageViewController preventing holding all inst…
dus7 Nov 4, 2024
50bf23e
Fix NewTabPageSettingsModel leaking via NTPSetting
dus7 Nov 4, 2024
51b5173
Fix issues with dismissing favicon fetcher tutorial
dus7 Nov 4, 2024
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
7 changes: 5 additions & 2 deletions DuckDuckGo/MainViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class MainViewController: UIViewController {

private lazy var featureFlagger = AppDependencyProvider.shared.featureFlagger
private lazy var faviconLoader: FavoritesFaviconLoading = FavoritesFaviconLoader()
private lazy var faviconsFetcherOnboarding = FaviconsFetcherOnboarding(syncService: syncService, syncBookmarksAdapter: syncDataProviders.bookmarksAdapter)

lazy var menuBookmarksViewModel: MenuBookmarksInteracting = {
let viewModel = MenuBookmarksViewModel(bookmarksDatabase: bookmarksDatabase, syncService: syncService)
Expand Down Expand Up @@ -795,8 +796,6 @@ class MainViewController: UIViewController {
let controller = NewTabPageViewController(tab: tabModel,
isNewTabPageCustomizationEnabled: homeTabManager.isNewTabPageSectionsEnabled,
interactionModel: favoritesViewModel,
syncService: syncService,
syncBookmarksAdapter: syncDataProviders.bookmarksAdapter,
homePageMessagesConfiguration: homePageConfiguration,
privacyProDataReporting: privacyProDataReporter,
variantManager: variantManager,
Expand Down Expand Up @@ -2172,6 +2171,10 @@ extension MainViewController: NewTabPageControllerDelegate {
func newTabPageDidDeleteFavorite(_ controller: NewTabPageViewController, favorite: BookmarkEntity) {
// no-op for now
}

func newTabPageDidRequestFaviconsFetcherOnboarding(_ controller: NewTabPageViewController) {
faviconsFetcherOnboarding.presentOnboardingIfNeeded(from: self)
}
}

extension MainViewController: NewTabPageControllerShortcutsDelegate {
Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/NewTabPageControllerDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ protocol NewTabPageControllerDelegate: AnyObject {
func newTabPageDidOpenFavoriteURL(_ controller: NewTabPageViewController, url: URL)
func newTabPageDidDeleteFavorite(_ controller: NewTabPageViewController, favorite: BookmarkEntity)
func newTabPageDidEditFavorite(_ controller: NewTabPageViewController, favorite: BookmarkEntity)
func newTabPageDidRequestFaviconsFetcherOnboarding(_ controller: NewTabPageViewController)
}

protocol NewTabPageControllerShortcutsDelegate: AnyObject {
Expand Down
14 changes: 7 additions & 7 deletions DuckDuckGo/NewTabPageSettingsModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ final class NewTabPageSettingsModel<SettingItem: NewTabPageSettingsStorageItem,
private func populateSettings() {
let allItemsOrder = settingsStorage.itemsOrder
filteredItemsOrder = allItemsOrder.filter(visibilityFilter)

itemsSettings = filteredItemsOrder.compactMap { item in
return NTPSetting(item: item,
isEnabled: Binding(get: {
self.settingsStorage.isEnabled(item)
}, set: { newValue in
self.onItemEnabled?(item, newValue)
self.settingsStorage.setItem(item, enabled: newValue)
self.updatePublishedValues()
isEnabled: Binding(get: { [weak self] in
self?.settingsStorage.isEnabled(item) ?? false
}, set: { [weak self] newValue in
self?.onItemEnabled?(item, newValue)
self?.settingsStorage.setItem(item, enabled: newValue)
self?.updatePublishedValues()
}))
}

Expand Down
16 changes: 6 additions & 10 deletions DuckDuckGo/NewTabPageViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,10 @@ import Core

final class NewTabPageViewController: UIHostingController<AnyView>, NewTabPage {

private let syncService: DDGSyncing
private let syncBookmarksAdapter: SyncBookmarksAdapter
private let variantManager: VariantManager
private let newTabDialogFactory: any NewTabDaxDialogProvider
private let newTabDialogTypeProvider: NewTabDialogSpecProvider

private(set) lazy var faviconsFetcherOnboarding = FaviconsFetcherOnboarding(syncService: syncService, syncBookmarksAdapter: syncBookmarksAdapter)

private let newTabPageViewModel: NewTabPageViewModel
private let messagesModel: NewTabPageMessagesModel
private let favoritesModel: FavoritesViewModel
Expand All @@ -53,8 +49,6 @@ final class NewTabPageViewController: UIHostingController<AnyView>, NewTabPage {
init(tab: Tab,
isNewTabPageCustomizationEnabled: Bool,
interactionModel: FavoritesListInteracting,
syncService: DDGSyncing,
syncBookmarksAdapter: SyncBookmarksAdapter,
homePageMessagesConfiguration: HomePageMessagesConfiguration,
privacyProDataReporting: PrivacyProDataReporting? = nil,
variantManager: VariantManager,
Expand All @@ -63,8 +57,6 @@ final class NewTabPageViewController: UIHostingController<AnyView>, NewTabPage {
faviconLoader: FavoritesFaviconLoading) {

self.associatedTab = tab
self.syncService = syncService
self.syncBookmarksAdapter = syncBookmarksAdapter
self.variantManager = variantManager
self.newTabDialogFactory = newTabDialogFactory
self.newTabDialogTypeProvider = newTabDialogTypeProvider
Expand Down Expand Up @@ -145,7 +137,8 @@ final class NewTabPageViewController: UIHostingController<AnyView>, NewTabPage {
private func assignFavoriteModelActions() {
favoritesModel.onFaviconMissing = { [weak self] in
guard let self else { return }
self.faviconsFetcherOnboarding.presentOnboardingIfNeeded(from: self)

delegate?.newTabPageDidRequestFaviconsFetcherOnboarding(self)
}

favoritesModel.onFavoriteURLSelected = { [weak self] url in
Expand Down Expand Up @@ -215,7 +208,10 @@ final class NewTabPageViewController: UIHostingController<AnyView>, NewTabPage {
}

func dismiss() {

delegate = nil
chromeDelegate = nil
removeFromParent()
view.removeFromSuperview()
}

func showNextDaxDialog() {
Expand Down
11 changes: 1 addition & 10 deletions DuckDuckGoTests/NewTabPageControllerDaxDialogTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,7 @@ final class NewTabPageControllerDaxDialogTests: XCTestCase {
variantManager = CapturingVariantManager()
dialogFactory = CapturingNewTabDaxDialogProvider()
specProvider = MockNewTabDialogSpecProvider()
let dataProviders = SyncDataProviders(
bookmarksDatabase: db,
secureVaultFactory: AutofillSecureVaultFactory,
secureVaultErrorReporter: SecureVaultReporter(),
settingHandlers: [],
favoritesDisplayModeStorage: MockFavoritesDisplayModeStoring(),
syncErrorHandler: SyncErrorHandler()
)

let remoteMessagingClient = RemoteMessagingClient(
bookmarksDatabase: db,
appSettings: AppSettingsMock(),
Expand All @@ -60,8 +53,6 @@ final class NewTabPageControllerDaxDialogTests: XCTestCase {
tab: Tab(),
isNewTabPageCustomizationEnabled: false,
interactionModel: MockFavoritesListInteracting(),
syncService: MockDDGSyncing(authState: .active, isSyncInProgress: false),
syncBookmarksAdapter: dataProviders.bookmarksAdapter,
homePageMessagesConfiguration: homePageConfiguration,
variantManager: variantManager,
newTabDialogFactory: dialogFactory,
Expand Down
Loading