From e0d48a03552a070c514b4d67a2271eb54927f24a Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Thu, 30 Jan 2025 17:27:23 -0500 Subject: [PATCH 1/6] Try to improve viewDidLoad --- .../Views/BrowserViewController.swift | 101 +++++++++--------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index 3054cd8c89c4..f95ed8b509d5 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -307,6 +307,7 @@ class BrowserViewController: UIViewController, let navigationViewProvider = ContextualHintViewProvider(forHintType: .navigation, with: profile) self.navigationContextHintVC = ContextualHintViewController(with: navigationViewProvider, windowUUID: windowUUID) + self.searchTelemetry = SearchTelemetry(tabManager: tabManager) super.init(nibName: nil, bundle: nil) didInit() @@ -746,61 +747,65 @@ class BrowserViewController: UIViewController, override func viewDidLoad() { super.viewDidLoad() - KeyboardHelper.defaultHelper.addDelegate(self) - trackTelemetry() - setupNotifications() - addSubviews() - listenForThemeChange(view) - setupAccessibleActions() - - clipboardBarDisplayHandler = ClipboardBarDisplayHandler(prefs: profile.prefs, tabManager: tabManager) - clipboardBarDisplayHandler?.delegate = self - - navigationToolbarContainer.toolbarDelegate = self - - scrollController.header = header - scrollController.overKeyboardContainer = overKeyboardContainer - scrollController.bottomContainer = bottomContainer - - updateToolbarStateForTraitCollection(traitCollection) - - setupConstraints() - // Setup UIDropInteraction to handle dragging and dropping - // links into the view from other apps. - let dropInteraction = UIDropInteraction(delegate: self) - view.addInteraction(dropInteraction) + setupEssentialUI() + subscribeToRedux() - searchTelemetry = SearchTelemetry(tabManager: tabManager) + // Non-UI tasks only + DispatchQueue.global(qos: .background).async { + self.trackTelemetry() + self.setupNotifications() + SearchBarSettingsViewModel.recordLocationTelemetry(for: self.isBottomSearchBar ? .bottom : .top) - // Awesomebar Location Telemetry - SearchBarSettingsViewModel.recordLocationTelemetry(for: isBottomSearchBar ? .bottom : .top) + // Feature flag for credit card until we fully enable this feature + let autofillCreditCardStatus = self.featureFlags.isFeatureEnabled( + .creditCardAutofillStatus, checking: .buildOnly) + // We need to update autofill status on sync manager as there could be delay from nimbus + // in getting the value. When the delay happens the credit cards might not sync + // as the default value is false + self.profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) - overlayManager.setURLBar(urlBarView: urlBarView) + self.creditCardInitialSetupTelemetry() - // Update theme of already existing views - let theme = currentTheme() - header.applyTheme(theme: theme) - overKeyboardContainer.applyTheme(theme: theme) - bottomContainer.applyTheme(theme: theme) - bottomContentStackView.applyTheme(theme: theme) - statusBarOverlay.hasTopTabs = ToolbarHelper().shouldShowTopTabs(for: traitCollection) - statusBarOverlay.applyTheme(theme: theme) - - // Feature flag for credit card until we fully enable this feature - let autofillCreditCardStatus = featureFlags.isFeatureEnabled( - .creditCardAutofillStatus, checking: .buildOnly) - // We need to update autofill status on sync manager as there could be delay from nimbus - // in getting the value. When the delay happens the credit cards might not sync - // as the default value is false - profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) - // Credit card initial setup telemetry - creditCardInitialSetupTelemetry() + FakespotUtils().addSettingTelemetry() + } + } - // Send settings telemetry for Fakespot - FakespotUtils().addSettingTelemetry() + private func setupEssentialUI() { + addSubviews() + setupConstraints() - subscribeToRedux() + DispatchQueue.main.async { + self.overlayManager.setURLBar(urlBarView: self.urlBarView) + self.updateToolbarStateForTraitCollection(self.traitCollection) + + // Update theme of already existing views + let theme = self.currentTheme() + self.header.applyTheme(theme: theme) + self.overKeyboardContainer.applyTheme(theme: theme) + self.bottomContainer.applyTheme(theme: theme) + self.bottomContentStackView.applyTheme(theme: theme) + self.statusBarOverlay.hasTopTabs = ToolbarHelper().shouldShowTopTabs(for: self.traitCollection) + self.statusBarOverlay.applyTheme(theme: theme) + + KeyboardHelper.defaultHelper.addDelegate(self) + self.listenForThemeChange(self.view) + self.setupAccessibleActions() + + self.clipboardBarDisplayHandler = ClipboardBarDisplayHandler(prefs: self.profile.prefs, + tabManager: self.tabManager) + self.clipboardBarDisplayHandler?.delegate = self + + self.navigationToolbarContainer.toolbarDelegate = self + self.scrollController.header = self.header + self.scrollController.overKeyboardContainer = self.overKeyboardContainer + self.scrollController.bottomContainer = self.bottomContainer + + // Setup UIDropInteraction to handle dragging and dropping + // links into the view from other apps. + let dropInteraction = UIDropInteraction(delegate: self) + self.view.addInteraction(dropInteraction) + } } private func setupAccessibleActions() { From ccbc1d2037b35ed13f0c97672b426cb23f5ea288 Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Thu, 30 Jan 2025 17:45:59 -0500 Subject: [PATCH 2/6] Remove UISearchController init from startpath, wasn't even needed + fix some stuff that needs to be on the main thread --- .../Views/BrowserViewController.swift | 28 +++++++++++-------- .../PasswordManagement/LoginDataSource.swift | 2 +- .../PasswordManagerViewModel.swift | 2 +- .../Telemetry/AppStartupTelemetry.swift | 3 +- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index f95ed8b509d5..5bf81ee7535e 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -751,9 +751,13 @@ class BrowserViewController: UIViewController, setupEssentialUI() subscribeToRedux() + DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 2.0) { + // App startup telemetry accesses RustLogins to queryLogins, shouldn't be on the app startup critical path + self.trackStartupTelemetry() + } + // Non-UI tasks only DispatchQueue.global(qos: .background).async { - self.trackTelemetry() self.setupNotifications() SearchBarSettingsViewModel.recordLocationTelemetry(for: self.isBottomSearchBar ? .bottom : .top) @@ -4363,7 +4367,7 @@ extension BrowserViewController: DevicePickerViewControllerDelegate, Instruction } extension BrowserViewController { - func trackTelemetry() { + func trackStartupTelemetry() { trackAccessibility() trackNotificationPermission() appStartupTelemetry.sendStartupTelemetry() @@ -4402,15 +4406,17 @@ extension BrowserViewController { extras: [Key.isInvertColorsEnabled.rawValue: UIAccessibility.isInvertColorsEnabled.description] ) - let a11yEnabled = UIApplication.shared.preferredContentSizeCategory.isAccessibilityCategory.description - let a11yCategory = UIApplication.shared.preferredContentSizeCategory.rawValue.description - TelemetryWrapper.recordEvent( - category: .action, - method: .dynamicTextSize, - object: .app, - extras: [Key.isAccessibilitySizeEnabled.rawValue: a11yEnabled, - Key.preferredContentSizeCategory.rawValue: a11yCategory] - ) + ensureMainThread { + let a11yEnabled = UIApplication.shared.preferredContentSizeCategory.isAccessibilityCategory.description + let a11yCategory = UIApplication.shared.preferredContentSizeCategory.rawValue.description + TelemetryWrapper.recordEvent( + category: .action, + method: .dynamicTextSize, + object: .app, + extras: [Key.isAccessibilitySizeEnabled.rawValue: a11yEnabled, + Key.preferredContentSizeCategory.rawValue: a11yCategory] + ) + } } func trackNotificationPermission() { diff --git a/firefox-ios/Client/Frontend/PasswordManagement/LoginDataSource.swift b/firefox-ios/Client/Frontend/PasswordManagement/LoginDataSource.swift index 5089f7e8dcb2..c87bc9de3263 100644 --- a/firefox-ios/Client/Frontend/PasswordManagement/LoginDataSource.swift +++ b/firefox-ios/Client/Frontend/PasswordManagement/LoginDataSource.swift @@ -8,7 +8,7 @@ import Shared /// Data source for handling LoginData objects from a Cursor class LoginDataSource: NSObject, UITableViewDataSource { // in case there are no items to run cellForRowAt on, use an empty state view - private let emptyStateView = NoLoginsView() + private lazy var emptyStateView = NoLoginsView() var viewModel: PasswordManagerViewModel let boolSettings: (BoolSetting, BoolSetting) diff --git a/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift b/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift index ee91c99cd982..5478ea0b3b2f 100644 --- a/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift +++ b/firefox-ios/Client/Frontend/PasswordManagement/PasswordManagerViewModel.swift @@ -46,7 +46,7 @@ final class PasswordManagerViewModel { var hasLoadedBreaches = false var theme: Theme - init(profile: Profile, searchController: UISearchController, theme: Theme, loginProvider: LoginProvider) { + init(profile: Profile, searchController: UISearchController?, theme: Theme, loginProvider: LoginProvider) { self.profile = profile self.searchController = searchController self.theme = theme diff --git a/firefox-ios/Client/Telemetry/AppStartupTelemetry.swift b/firefox-ios/Client/Telemetry/AppStartupTelemetry.swift index 5dd25a41c9b2..05081dfa845d 100644 --- a/firefox-ios/Client/Telemetry/AppStartupTelemetry.swift +++ b/firefox-ios/Client/Telemetry/AppStartupTelemetry.swift @@ -37,10 +37,9 @@ final class AppStartupTelemetry { // MARK: Logins func queryLogins() { - let searchController = UISearchController() let loginsViewModel = PasswordManagerViewModel( profile: profile, - searchController: searchController, + searchController: nil, theme: LightTheme(), loginProvider: profile.logins ) From 9b70418eb80b7d78e3771b2ca94f9241f006e78b Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Thu, 30 Jan 2025 18:00:35 -0500 Subject: [PATCH 3/6] Remove redundant call to updateToolbarStateForTraitCollection Move telemetry in the telemetry tracking class --- .../Views/BrowserViewController.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index 5bf81ee7535e..2daddbc3d297 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -768,10 +768,6 @@ class BrowserViewController: UIViewController, // in getting the value. When the delay happens the credit cards might not sync // as the default value is false self.profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) - - self.creditCardInitialSetupTelemetry() - - FakespotUtils().addSettingTelemetry() } } @@ -781,7 +777,7 @@ class BrowserViewController: UIViewController, DispatchQueue.main.async { self.overlayManager.setURLBar(urlBarView: self.urlBarView) - self.updateToolbarStateForTraitCollection(self.traitCollection) +// self.updateToolbarStateForTraitCollection(self.traitCollection) // Update theme of already existing views let theme = self.currentTheme() @@ -4371,6 +4367,8 @@ extension BrowserViewController { trackAccessibility() trackNotificationPermission() appStartupTelemetry.sendStartupTelemetry() + creditCardInitialSetupTelemetry() + FakespotUtils().addSettingTelemetry() } func trackAccessibility() { From c98e85183e791f38f0a2b0b55fa8a75badb28fcd Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Fri, 31 Jan 2025 14:59:47 -0500 Subject: [PATCH 4/6] Update - Move Search bar telemetry to the trackStartUpTelemetry function - Move updateCreditCardAutofillStatus into main.async - Remove commented code - Move setupNotifications back to setupEssentialUI since we dont want race conditions --- .../Views/BrowserViewController.swift | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index 2daddbc3d297..e0918b6a7170 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -751,33 +751,19 @@ class BrowserViewController: UIViewController, setupEssentialUI() subscribeToRedux() - DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 2.0) { + DispatchQueue.global(qos: .background).async { // App startup telemetry accesses RustLogins to queryLogins, shouldn't be on the app startup critical path self.trackStartupTelemetry() } - - // Non-UI tasks only - DispatchQueue.global(qos: .background).async { - self.setupNotifications() - SearchBarSettingsViewModel.recordLocationTelemetry(for: self.isBottomSearchBar ? .bottom : .top) - - // Feature flag for credit card until we fully enable this feature - let autofillCreditCardStatus = self.featureFlags.isFeatureEnabled( - .creditCardAutofillStatus, checking: .buildOnly) - // We need to update autofill status on sync manager as there could be delay from nimbus - // in getting the value. When the delay happens the credit cards might not sync - // as the default value is false - self.profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) - } } private func setupEssentialUI() { addSubviews() setupConstraints() + setupNotifications() DispatchQueue.main.async { self.overlayManager.setURLBar(urlBarView: self.urlBarView) -// self.updateToolbarStateForTraitCollection(self.traitCollection) // Update theme of already existing views let theme = self.currentTheme() @@ -805,6 +791,14 @@ class BrowserViewController: UIViewController, // links into the view from other apps. let dropInteraction = UIDropInteraction(delegate: self) self.view.addInteraction(dropInteraction) + + // Feature flag for credit card until we fully enable this feature + let autofillCreditCardStatus = self.featureFlags.isFeatureEnabled( + .creditCardAutofillStatus, checking: .buildOnly) + // We need to update autofill status on sync manager as there could be delay from nimbus + // in getting the value. When the delay happens the credit cards might not sync + // as the default value is false + self.profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) } } @@ -4364,6 +4358,8 @@ extension BrowserViewController: DevicePickerViewControllerDelegate, Instruction extension BrowserViewController { func trackStartupTelemetry() { + let toolbarLocation: SearchBarPosition = self.isBottomSearchBar ? .bottom : .top + SearchBarSettingsViewModel.recordLocationTelemetry(for: toolbarLocation) trackAccessibility() trackNotificationPermission() appStartupTelemetry.sendStartupTelemetry() From b172a8da1c985cf0f0fea53709d7ae9dd97abd46 Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Fri, 31 Jan 2025 15:59:23 -0500 Subject: [PATCH 5/6] Create setupNonEssentialUI function --- .../Views/BrowserViewController.swift | 76 ++++++++++--------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index e0918b6a7170..953c5a4a7b30 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -763,45 +763,49 @@ class BrowserViewController: UIViewController, setupNotifications() DispatchQueue.main.async { - self.overlayManager.setURLBar(urlBarView: self.urlBarView) - - // Update theme of already existing views - let theme = self.currentTheme() - self.header.applyTheme(theme: theme) - self.overKeyboardContainer.applyTheme(theme: theme) - self.bottomContainer.applyTheme(theme: theme) - self.bottomContentStackView.applyTheme(theme: theme) - self.statusBarOverlay.hasTopTabs = ToolbarHelper().shouldShowTopTabs(for: self.traitCollection) - self.statusBarOverlay.applyTheme(theme: theme) - - KeyboardHelper.defaultHelper.addDelegate(self) - self.listenForThemeChange(self.view) - self.setupAccessibleActions() - - self.clipboardBarDisplayHandler = ClipboardBarDisplayHandler(prefs: self.profile.prefs, - tabManager: self.tabManager) - self.clipboardBarDisplayHandler?.delegate = self - - self.navigationToolbarContainer.toolbarDelegate = self - self.scrollController.header = self.header - self.scrollController.overKeyboardContainer = self.overKeyboardContainer - self.scrollController.bottomContainer = self.bottomContainer - - // Setup UIDropInteraction to handle dragging and dropping - // links into the view from other apps. - let dropInteraction = UIDropInteraction(delegate: self) - self.view.addInteraction(dropInteraction) - - // Feature flag for credit card until we fully enable this feature - let autofillCreditCardStatus = self.featureFlags.isFeatureEnabled( - .creditCardAutofillStatus, checking: .buildOnly) - // We need to update autofill status on sync manager as there could be delay from nimbus - // in getting the value. When the delay happens the credit cards might not sync - // as the default value is false - self.profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) + self.setupNonEssentialUI() } } + private func setupNonEssentialUI() { + overlayManager.setURLBar(urlBarView: urlBarView) + + // Update theme of already existing views + let theme = currentTheme() + header.applyTheme(theme: theme) + overKeyboardContainer.applyTheme(theme: theme) + bottomContainer.applyTheme(theme: theme) + bottomContentStackView.applyTheme(theme: theme) + statusBarOverlay.hasTopTabs = ToolbarHelper().shouldShowTopTabs(for: traitCollection) + statusBarOverlay.applyTheme(theme: theme) + + KeyboardHelper.defaultHelper.addDelegate(self) + listenForThemeChange(view) + setupAccessibleActions() + + clipboardBarDisplayHandler = ClipboardBarDisplayHandler(prefs: profile.prefs, + tabManager: tabManager) + clipboardBarDisplayHandler?.delegate = self + + navigationToolbarContainer.toolbarDelegate = self + scrollController.header = header + scrollController.overKeyboardContainer = overKeyboardContainer + scrollController.bottomContainer = bottomContainer + + // Setup UIDropInteraction to handle dragging and dropping + // links into the view from other apps. + let dropInteraction = UIDropInteraction(delegate: self) + view.addInteraction(dropInteraction) + + // Feature flag for credit card until we fully enable this feature + let autofillCreditCardStatus = featureFlags.isFeatureEnabled( + .creditCardAutofillStatus, checking: .buildOnly) + // We need to update autofill status on sync manager as there could be delay from nimbus + // in getting the value. When the delay happens the credit cards might not sync + // as the default value is false + profile.syncManager?.updateCreditCardAutofillStatus(value: autofillCreditCardStatus) + } + private func setupAccessibleActions() { // UIAccessibilityCustomAction subclass holding an AccessibleAction instance does not work, // thus unable to generate AccessibleActions and UIAccessibilityCustomActions "on-demand" and need From 1a33cf9bd278c9ab6fef1e91b42aa8ebf4ec180d Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Tue, 4 Feb 2025 11:34:51 -0500 Subject: [PATCH 6/6] Remove non essential UI + change to Task(priority:) for background telemetry queue --- .../Views/BrowserViewController.swift | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index 953c5a4a7b30..17f42fdeaffb 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -751,7 +751,7 @@ class BrowserViewController: UIViewController, setupEssentialUI() subscribeToRedux() - DispatchQueue.global(qos: .background).async { + Task(priority: .background) { // App startup telemetry accesses RustLogins to queryLogins, shouldn't be on the app startup critical path self.trackStartupTelemetry() } @@ -762,12 +762,6 @@ class BrowserViewController: UIViewController, setupConstraints() setupNotifications() - DispatchQueue.main.async { - self.setupNonEssentialUI() - } - } - - private func setupNonEssentialUI() { overlayManager.setURLBar(urlBarView: urlBarView) // Update theme of already existing views @@ -784,7 +778,7 @@ class BrowserViewController: UIViewController, setupAccessibleActions() clipboardBarDisplayHandler = ClipboardBarDisplayHandler(prefs: profile.prefs, - tabManager: tabManager) + tabManager: tabManager) clipboardBarDisplayHandler?.delegate = self navigationToolbarContainer.toolbarDelegate = self