diff --git a/firefox-ios/Client/Application/AppDelegate.swift b/firefox-ios/Client/Application/AppDelegate.swift index 4881c50c0e1ea..8a3342e2d8436 100644 --- a/firefox-ios/Client/Application/AppDelegate.swift +++ b/firefox-ios/Client/Application/AppDelegate.swift @@ -244,7 +244,7 @@ class AppDelegate: UIResponder, logger.log("Received memory warning", level: .info, category: .lifecycle) Task { for uuid in windowManager.allWindowUUIDs(includingReserved: false) { - await windowManager.tabManager(for: uuid).offloadBackgroundWebViews() + await windowManager.tabManager(for: uuid)?.offloadBackgroundWebViews() } } } diff --git a/firefox-ios/Client/Application/SceneDelegate.swift b/firefox-ios/Client/Application/SceneDelegate.swift index 50b0b476d4f7c..0f9edf8105ad0 100644 --- a/firefox-ios/Client/Application/SceneDelegate.swift +++ b/firefox-ios/Client/Application/SceneDelegate.swift @@ -176,7 +176,7 @@ class SceneDelegate: UIResponder, // If so, we want to be sure that we select the tab in the correct iPad window. if shouldRerouteIncomingURLToSpecificWindow(url), let tabUUID = URLScanner(url: url)?.value(query: "uuid"), - let targetWindow = (AppContainer.shared.resolve() as WindowManager).window(for: tabUUID), + let targetWindow = (AppContainer.shared.resolve() as WindowManager).windowUUID(forTab: tabUUID), targetWindow != sceneCoordinator?.windowUUID { DefaultApplicationHelper().open(url, inWindow: targetWindow) } else { diff --git a/firefox-ios/Client/Application/WindowManager+DebugUtilities.swift b/firefox-ios/Client/Application/WindowManager+DebugUtilities.swift index dcbebb14a264a..9f9046e6c2843 100644 --- a/firefox-ios/Client/Application/WindowManager+DebugUtilities.swift +++ b/firefox-ios/Client/Application/WindowManager+DebugUtilities.swift @@ -15,7 +15,7 @@ extension WindowManagerImplementation { var result = "----------- Window Debug Info ------------\n" result.append("Open windows (\(windows.count)) & normal tabs (via TabManager):\n") for (idx, (uuid, _)) in windows.enumerated() { - let tabMgr = tabManager(for: uuid) + guard let tabMgr = tabManager(for: uuid) else { continue } let window = windows[uuid]?.sceneCoordinator?.window let frame = window?.frame ?? .zero result.append(" \(idx + 1): \(short(uuid)) (\(tabMgr.normalTabs.count) tabs) (frame: \(frame.debugDescription))\n") diff --git a/firefox-ios/Client/Application/WindowManager.swift b/firefox-ios/Client/Application/WindowManager.swift index b822e7ffea1e3..de0f01669d05f 100644 --- a/firefox-ios/Client/Application/WindowManager.swift +++ b/firefox-ios/Client/Application/WindowManager.swift @@ -30,7 +30,7 @@ protocol WindowManager { func newBrowserWindowConfigured(_ windowInfo: AppWindowInfo, uuid: WindowUUID) /// Convenience. Returns the TabManager for a specific window. - func tabManager(for windowUUID: WindowUUID) -> TabManager + func tabManager(for windowUUID: WindowUUID) -> TabManager? /// Convenience. Returns all TabManagers for all open windows. func allWindowTabManagers() -> [TabManager] @@ -73,11 +73,7 @@ protocol WindowManager { /// - Parameter tab: the UUID of the tab. /// - Returns: the UUID of the window hosting it (if available and open). @MainActor - func window(for tab: TabUUID) -> WindowUUID? - - /// Convenience. Provides opportunity for safety checks or window validation. - @MainActor - func windowExists(uuid: WindowUUID) -> Bool + func windowUUID(forTab tab: TabUUID) -> WindowUUID? } /// Captures state and coordinator references specific to one particular app window. @@ -135,25 +131,17 @@ final class WindowManagerImplementation: WindowManager { clearReservedUUID(uuid) } - func tabManager(for windowUUID: WindowUUID) -> TabManager { - func unsafeAnyTabManager() -> TabManager { - // This is unsafe, but is the best fallback we have to try to handle non-fatally (but may crash anyway) - if let tabManager = windows.first?.value.tabManager { - logger.log("Unsafe tab manager with windowUUID: \(tabManager.windowUUID)", level: .fatal, category: .window) - } - return windows.first!.value.tabManager! - } - + func tabManager(for windowUUID: WindowUUID) -> TabManager? { guard let window = window(for: windowUUID) else { - assertionFailure("No window for UUID: \(windowUUID). This is a client error.") + assertionFailure("No window for UUID: \(windowUUID). This is a client error. It will return nil in production but querying a non-existent window is always indicative of a bug.") logger.log("No window for UUID: \(windowUUID)", level: .fatal, category: .window) - return unsafeAnyTabManager() + return nil } guard let manager = window.tabManager else { - assertionFailure("Window alive, but no TabManager for UUID: \(windowUUID). This is a client error.") + assertionFailure("Valid window but no TabManager for UUID: \(windowUUID). This is a client error. It will return nil in production but is indicative of a bug.") logger.log("Window alive, but no TabManager for UUID: \(windowUUID)", level: .fatal, category: .window) - return unsafeAnyTabManager() + return nil } return manager @@ -285,15 +273,10 @@ final class WindowManagerImplementation: WindowManager { } } - func window(for tab: TabUUID) -> WindowUUID? { + func windowUUID(forTab tab: TabUUID) -> WindowUUID? { return allWindowTabManagers().first(where: { $0.tabs.contains(where: { $0.tabUUID == tab }) })?.windowUUID } - func windowExists(uuid: WindowUUID) -> Bool { - guard uuid != .unavailable else { return false } - return windows[uuid] != nil - } - // MARK: - Internal Utilities private func clearReservedUUID(_ uuid: WindowUUID) { diff --git a/firefox-ios/Client/Frontend/Browser/StartAtHome/StartAtHomeMiddleware.swift b/firefox-ios/Client/Frontend/Browser/StartAtHome/StartAtHomeMiddleware.swift index 7b90b3a4b9be1..0cf4189fa0199 100644 --- a/firefox-ios/Client/Frontend/Browser/StartAtHome/StartAtHomeMiddleware.swift +++ b/firefox-ios/Client/Frontend/Browser/StartAtHome/StartAtHomeMiddleware.swift @@ -45,7 +45,7 @@ final class StartAtHomeMiddleware { /// - Returns: `true` if a homepage tab was selected and displayed, `false` otherwise. @MainActor private func startAtHomeCheck(windowUUID: WindowUUID) -> Bool { - let tabManager = windowManager.tabManager(for: windowUUID) + guard let tabManager = windowManager.tabManager(for: windowUUID) else { return false} let startAtHomeManager = StartAtHomeHelper( prefs: prefs, isRestoringTabs: !tabManager.tabRestoreHasFinished diff --git a/firefox-ios/Client/Frontend/Browser/Tabs/Middleware/TabManagerMiddleware.swift b/firefox-ios/Client/Frontend/Browser/Tabs/Middleware/TabManagerMiddleware.swift index f607ba3990b10..3aa3e598272dc 100644 --- a/firefox-ios/Client/Frontend/Browser/Tabs/Middleware/TabManagerMiddleware.swift +++ b/firefox-ios/Client/Frontend/Browser/Tabs/Middleware/TabManagerMiddleware.swift @@ -78,7 +78,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark private func resolveShortcutsLibraryActions(action: ShortcutsLibraryAction, state: AppState) { switch action.actionType { case ShortcutsLibraryActionType.switchTabToastButtonTapped: - tabManager(for: action.windowUUID).selectTab(action.tab) + tabManager(for: action.windowUUID)?.selectTab(action.tab) default: break } @@ -91,8 +91,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark return } - let manager = tabManager(for: action.windowUUID) - manager.tabDidSetScreenshot(action.tab) + tabManager(for: action.windowUUID)?.tabDidSetScreenshot(action.tab) guard let tabsState = state.componentState(TabsPanelState.self, for: .tabsPanel, @@ -148,7 +147,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark // Short-term fix to avoid potential crashes where actions are processed // after the window scene has been torn down [FXIOS-13809] let windowManager: WindowManager = AppContainer.shared.resolve() - guard windowManager.windowExists(uuid: action.windowUUID) else { + guard windowManager.windows.keys.contains(action.windowUUID) else { logger.log("Window does not exist (\(action.windowUUID.uuidString.prefix(4))) for resolveTabTrayActions()", level: .warning, category: .tabs) @@ -251,8 +250,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark } private func tabTrayDidLoad(for windowUUID: WindowUUID, panelType: TabTrayPanelType?) { - let tabManager = tabManager(for: windowUUID) - let isPrivateModeActive = tabManager.selectedTab?.isPrivate ?? false + let isPrivateModeActive = tabManager(for: windowUUID)?.selectedTab?.isPrivate ?? false // If no panelType is provided then fallback to whichever tab is currently selected let panelType = panelType ?? (isPrivateModeActive ? .privateTabs : .tabs) @@ -264,20 +262,20 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark } private func normalTabsCountText(for windowUUID: WindowUUID) -> String { - let tabManager = tabManager(for: windowUUID) + guard let tabManager = tabManager(for: windowUUID) else { return "" } return (tabManager.normalTabs.count < 100) ? tabManager.normalTabs.count.description : "\u{221E}" } private func normalTabsCountTextForTabTray(for windowUUID: WindowUUID) -> String { - return tabManager(for: windowUUID).normalTabs.count.description + return tabManager(for: windowUUID)?.normalTabs.count.description ?? "" } private func privateTabsCountTextForTabTray(for windowUUID: WindowUUID) -> String { - return tabManager(for: windowUUID).privateTabs.count.description + return tabManager(for: windowUUID)?.privateTabs.count.description ?? "" } private func shouldEnableDeleteTabsButton(for windowUUID: WindowUUID, isPrivateMode: Bool) -> Bool { - let tabManager = tabManager(for: windowUUID) + guard let tabManager = tabManager(for: windowUUID) else { return false } let tabsCount = !isPrivateMode ? tabManager.normalTabs.count : tabManager.privateTabs.count return tabsCount > 0 ? true : false } @@ -334,7 +332,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark /// - Returns: Array of TabModel used to configure collection view private func refreshTabs(for isPrivateMode: Bool, uuid: WindowUUID) -> [TabModel] { var tabs = [TabModel]() - let tabManager = tabManager(for: uuid) + guard let tabManager = tabManager(for: uuid) else { return [] } let selectedTab = tabManager.selectedTab let tabManagerTabs = isPrivateMode ? tabManager.privateTabs : tabManager.normalTabs tabManagerTabs.forEach { tab in @@ -361,7 +359,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark MainActor.assertIsolated("Expected to be called only on main actor.") // TODO: Legacy class has a guard to cancel adding new tab if dragging was enabled, // check if change is still needed - let tabManager = tabManager(for: uuid) + guard let tabManager = tabManager(for: uuid) else { return } let tab = tabManager.addTab(urlRequest, isPrivate: isPrivate) tabManager.selectTab(tab) @@ -390,9 +388,9 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark method: .drop, object: .tab, value: .tabTray) - tabManager.reorderTabs(isPrivate: moveTabData.isPrivate, - fromIndex: moveTabData.originIndex, - toIndex: moveTabData.destinationIndex) + tabManager?.reorderTabs(isPrivate: moveTabData.isPrivate, + fromIndex: moveTabData.originIndex, + toIndex: moveTabData.destinationIndex) let model = getTabsDisplayModel(for: moveTabData.isPrivate, uuid: uuid) let action = TabPanelMiddlewareAction(tabDisplayModel: model, @@ -409,7 +407,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark /// - Returns: If is the last tab to be closed used to trigger dismissTabTray action private func closeTab(with tabUUID: TabUUID, uuid: WindowUUID, isPrivate: Bool) -> Bool { tabsPanelTelemetry.tabClosed(mode: isPrivate ? .private : .normal) - let tabManager = tabManager(for: uuid) + guard let tabManager = tabManager(for: uuid) else { return false } // In non-private mode, if: // A) the last normal active tab is closed, or // B) the last of ALL normal tabs are closed (i.e. all tabs are inactive and closed at once), @@ -424,10 +422,11 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark /// Close tab and trigger refresh /// - Parameter tabUUID: UUID of the tab to be closed/removed private func closeTabFromTabPanel(with tabUUID: TabUUID, uuid: WindowUUID, isPrivate: Bool) { + guard let tabManager = tabManager(for: uuid) else { return } let shouldDismiss = closeTab(with: tabUUID, uuid: uuid, isPrivate: isPrivate) triggerRefresh(uuid: uuid, isPrivate: isPrivate) - if isPrivate && tabManager(for: uuid).privateTabs.isEmpty { + if isPrivate && tabManager.privateTabs.isEmpty { let didLoadAction = TabPanelViewAction(panelType: isPrivate ? .privateTabs : .tabs, windowUUID: uuid, actionType: TabPanelViewActionType.tabPanelDidLoad) @@ -472,7 +471,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark guard let tabsState = state.componentState(TabsPanelState.self, for: .tabsPanel, window: uuid) else { return } tabsPanelTelemetry.closeAllTabsSheetOptionSelected(option: .all, mode: tabsState.isPrivateMode ? .private : .normal) - tabManager.removeAllTabs(isPrivateMode: tabsState.isPrivateMode) + tabManager?.removeAllTabs(isPrivateMode: tabsState.isPrivateMode) triggerRefresh(uuid: uuid, isPrivate: tabsState.isPrivateMode) @@ -487,7 +486,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark private func deleteNormalTabsOlderThan(period: TabsDeletionPeriod, uuid: WindowUUID) { tabsPanelTelemetry.deleteNormalTabsSheetOptionSelected(period: period) let tabManager = tabManager(for: uuid) - tabManager.removeNormalTabsOlderThan(period: period, currentDate: .now) + tabManager?.removeNormalTabsOlderThan(period: period, currentDate: .now) // We are not closing the tab tray, so we need to refresh the tabs on screen let model = getTabsDisplayModel(for: false, uuid: uuid) @@ -502,8 +501,8 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark /// - Parameter uuid: The window identifier. private func addNewNormalTabIfSelectedIsPrivate(uuid: WindowUUID) { let tabManager = tabManager(for: uuid) - if let selectedTab = tabManager.selectedTab, selectedTab.isPrivate { - tabManager.addTab(nil, isPrivate: false) + if let selectedTab = tabManager?.selectedTab, selectedTab.isPrivate { + tabManager?.addTab(nil, isPrivate: false) } } @@ -518,9 +517,9 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark selectedTabIndex: Int? ) { let tabManager = tabManager(for: uuid) - guard let tab = tabManager.getTabForUUID(uuid: tabUUID) else { return } + guard let tab = tabManager?.getTabForUUID(uuid: tabUUID) else { return } - tabManager.selectTab(tab) + tabManager?.selectTab(tab) tabsPanelTelemetry.tabSelected(at: selectedTabIndex, mode: panelType.modeForTelemetry) @@ -529,21 +528,21 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark store.dispatch(action) } - private func tabManager(for uuid: WindowUUID) -> TabManager { - guard uuid != .unavailable else { + private func tabManager(for uuid: WindowUUID) -> TabManager? { + guard uuid != .unavailable, let tabManager = windowManager.tabManager(for: uuid) else { assertionFailure() logger.log("Unexpected or unavailable window UUID for requested TabManager.", level: .fatal, category: .tabs) - return windowManager.allWindowTabManagers().first! + return nil } - return windowManager.tabManager(for: uuid) + return tabManager } // MARK: - Tab Peek private func didLoadTabPeek(tabID: TabUUID, uuid: WindowUUID) { let tabManager = tabManager(for: uuid) - let tab = tabManager.getTabForUUID(uuid: tabID) + let tab = tabManager?.getTabForUUID(uuid: tabID) let urlString = tab?.url?.absoluteString ?? "" getIsBookmarked(url: urlString, dataQueue: .main) { isBookmarked in @@ -583,7 +582,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark private func copyURL(tabID: TabUUID, uuid: WindowUUID) { let tabManager = tabManager(for: uuid) - UIPasteboard.general.url = tabManager.getTabForUUID(uuid: tabID)?.canonicalURL + UIPasteboard.general.url = tabManager?.getTabForUUID(uuid: tabID)?.canonicalURL } private func tabPeekCloseTab(with tabID: TabUUID, uuid: WindowUUID, isPrivate: Bool) { @@ -635,7 +634,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark } private func changeUserAgent(forWindow windowUUID: WindowUUID) { - guard let selectedTab = tabManager(for: windowUUID).selectedTab else { return } + guard let selectedTab = tabManager(for: windowUUID)?.selectedTab else { return } if let url = selectedTab.url { // When the user changes user agent do the new request using the original URL @@ -657,7 +656,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark } private func provideTabInfo(forWindow windowUUID: WindowUUID, accountData: AccountData) { - guard let selectedTab = tabManager(for: windowUUID).selectedTab else { + guard let selectedTab = tabManager(for: windowUUID)?.selectedTab else { logger.log( "Attempted to get `selectedTab` but it was `nil` when in shouldn't be", level: .fatal, @@ -901,7 +900,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark } private func provideTabInfoForSiteProtectionsHeader(forWindow windowUUID: WindowUUID) { - guard let selectedTab = tabManager(for: windowUUID).selectedTab else { + guard let selectedTab = tabManager(for: windowUUID)?.selectedTab else { logger.log( "Attempted to get `selectedTab` but it was `nil` when in shouldn't be", level: .fatal, @@ -949,7 +948,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark case JumpBackInActionType.tapOnCell: guard let jumpBackInAction = action as? JumpBackInAction, let tab = jumpBackInAction.tab else { return } - tabManager(for: action.windowUUID).selectTab(tab) + tabManager(for: action.windowUUID)?.selectTab(tab) default: break } @@ -958,7 +957,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark // MARK: - Tab Manager Helper functions private func createShareItem(with tabID: TabUUID, and uuid: WindowUUID) -> ShareItem? { let tabManager = tabManager(for: uuid) - guard let tab = tabManager.getTabForUUID(uuid: tabID), + guard let tab = tabManager?.getTabForUUID(uuid: tabID), let url = tab.url?.absoluteString, !url.isEmpty else { return nil } @@ -987,7 +986,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark func removeBookmark(with tabID: TabUUID, uuid: WindowUUID) { let tabManager = tabManager(for: uuid) - guard let tab = tabManager.getTabForUUID(uuid: tabID), + guard let tab = tabManager?.getTabForUUID(uuid: tabID), let url = tab.url?.absoluteString, !url.isEmpty else { return } @@ -1004,7 +1003,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark private func addToShortcuts(with tabID: TabUUID?, uuid: WindowUUID) { let tabManager = tabManager(for: uuid) guard let tabID = tabID, - let tab = tabManager.getTabForUUID(uuid: tabID), + let tab = tabManager?.getTabForUUID(uuid: tabID), let url = tab.url?.displayURL?.absoluteString else { return } @@ -1016,7 +1015,7 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark private func removeFromShortcuts(with tabID: TabUUID?, uuid: WindowUUID) { let tabManager = tabManager(for: uuid) guard let tabID = tabID, - let tab = tabManager.getTabForUUID(uuid: tabID), + let tab = tabManager?.getTabForUUID(uuid: tabID), let url = tab.url?.displayURL?.absoluteString else { return } @@ -1026,13 +1025,13 @@ final class TabManagerMiddleware: FeatureFlaggable, CanRemoveQuickActionBookmark } private func preserveTabs(uuid: WindowUUID) { - let tabManager = tabManager(for: uuid) - tabManager.preserveTabs() + tabManager(for: uuid)?.preserveTabs() } /// Sends out updated recent tabs which is currently used for the homepage jumpBackIn section private func dispatchRecentlyAccessedTabs(action: Action) { - let recentTabs = self.tabManager(for: action.windowUUID).recentlyAccessedNormalTabs + guard let tabManager = tabManager(for: action.windowUUID) else { return } + let recentTabs = tabManager.recentlyAccessedNormalTabs store.dispatch( TabManagerAction( recentTabs: recentTabs, diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift index c5d5c79e681fe..4bc6bd42cc9bf 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift @@ -298,7 +298,7 @@ final class ToolbarMiddleware { case .summarizer: Task { @MainActor in - guard let webView = windowManager.tabManager(for: action.windowUUID).selectedTab?.webView else { return } + guard let webView = windowManager.tabManager(for: action.windowUUID)?.selectedTab?.webView else { return } let summarizerConfig = await summarizerConfigFactory.makeConfiguration(from: webView) let action = GeneralBrowserAction(summarizerConfig: summarizerConfig, summarizerTrigger: .toolbarIcon, @@ -362,7 +362,7 @@ final class ToolbarMiddleware { store.dispatch(action) case .readerModeWithSummarizer: Task { - guard let webView = windowManager.tabManager(for: action.windowUUID).selectedTab?.webView else { return } + guard let webView = windowManager.tabManager(for: action.windowUUID)?.selectedTab?.webView else { return } let summarizerConfig = await summarizerConfigFactory.makeConfiguration(from: webView) let action = GeneralBrowserAction(summarizerConfig: summarizerConfig, summarizerTrigger: .toolbarIcon, @@ -470,7 +470,7 @@ final class ToolbarMiddleware { @MainActor private func checkPageCanSummarize(action: ToolbarMiddlewareAction) { - guard let webView = windowManager.tabManager(for: action.windowUUID).selectedTab?.webView, + guard let webView = windowManager.tabManager(for: action.windowUUID)?.selectedTab?.webView, isSummarizerOn else { return } @@ -490,7 +490,7 @@ final class ToolbarMiddleware { // MARK: - Helper @MainActor private func cancelEditMode(windowUUID: WindowUUID) { - var url = tabManager(for: windowUUID).selectedTab?.url + var url = tabManager(for: windowUUID)?.selectedTab?.url if let currentURL = url { url = (currentURL.isWebPage() && !currentURL.isReaderModeURL) ? url : nil } @@ -531,7 +531,7 @@ final class ToolbarMiddleware { toolbarTelemetry.readerModeButtonTapped(isPrivate: toolbarState.isPrivateMode, isEnabled: isReaderModeEnabled) } - private func tabManager(for uuid: WindowUUID) -> TabManager { + private func tabManager(for uuid: WindowUUID) -> TabManager? { return windowManager.tabManager(for: uuid) } } diff --git a/firefox-ios/Client/Frontend/NativeErrorPage/NativeErrorPageMiddleware.swift b/firefox-ios/Client/Frontend/NativeErrorPage/NativeErrorPageMiddleware.swift index e70d285b67ec7..f96210061ebc3 100644 --- a/firefox-ios/Client/Frontend/NativeErrorPage/NativeErrorPageMiddleware.swift +++ b/firefox-ios/Client/Frontend/NativeErrorPage/NativeErrorPageMiddleware.swift @@ -53,7 +53,7 @@ final class NativeErrorPageMiddleware { } private func handleBypassCertificateWarning(windowUUID: WindowUUID) { - let selectedTab: Tab? = windowManager.tabManager(for: windowUUID).selectedTab + let selectedTab: Tab? = windowManager.tabManager(for: windowUUID)?.selectedTab if selectedTab == nil { logger.log( "handleBypassCertificateWarning: Failed to fetch selected tab", diff --git a/firefox-ios/Client/Frontend/Summarizer/SummarizeMiddleware.swift b/firefox-ios/Client/Frontend/Summarizer/SummarizeMiddleware.swift index 3c172002078de..478aed633dda0 100644 --- a/firefox-ios/Client/Frontend/Summarizer/SummarizeMiddleware.swift +++ b/firefox-ios/Client/Frontend/Summarizer/SummarizeMiddleware.swift @@ -82,7 +82,7 @@ final class SummarizerMiddleware: SummarizerConfigFactory { } private func fetchSummarizerConfig(windowUUID: WindowUUID, completion: @escaping (SummarizerConfig?) -> Void) { - guard let webView = windowManager.tabManager(for: windowUUID).selectedTab?.webView else { return } + guard let webView = windowManager.tabManager(for: windowUUID)?.selectedTab?.webView else { return } Task { let configuration = await makeConfiguration(from: webView) completion(configuration) @@ -130,7 +130,7 @@ final class SummarizerMiddleware: SummarizerConfigFactory { } private func dispatchShakeToSummarizeNotAvailable(windowUUID: WindowUUID) { - let isHomePage = windowManager.tabManager(for: windowUUID).selectedTab?.isFxHomeTab ?? false + let isHomePage = windowManager.tabManager(for: windowUUID)?.selectedTab?.isFxHomeTab ?? false guard summarizerNimbusUtils.isShakeGestureEnabled, !isHomePage else { return } store.dispatch( GeneralBrowserAction( diff --git a/firefox-ios/Client/Frontend/Translations/Service/TranslationsService.swift b/firefox-ios/Client/Frontend/Translations/Service/TranslationsService.swift index 63424ef9cab29..f257ba0685d6c 100644 --- a/firefox-ios/Client/Frontend/Translations/Service/TranslationsService.swift +++ b/firefox-ios/Client/Frontend/Translations/Service/TranslationsService.swift @@ -157,7 +157,7 @@ final class TranslationsService: TranslationsServiceProtocol { /// Returns the current WebView for a given window, or throws if it is unavailable. private func currentWebView(for windowUUID: WindowUUID) throws -> WKWebView { - guard let tab = windowManager.tabManager(for: windowUUID).selectedTab, + guard let tab = windowManager.tabManager(for: windowUUID)?.selectedTab, let webView = tab.webView else { throw TranslationsServiceError.missingWebView } diff --git a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift index d137ccd494c6f..5874fa09f8481 100644 --- a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift +++ b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift @@ -345,7 +345,7 @@ final class TranslationsMiddleware: FeatureFlaggable { } private func selectedTab(for windowUUID: WindowUUID) -> Tab? { - windowManager.tabManager(for: windowUUID).selectedTab + windowManager.tabManager(for: windowUUID)?.selectedTab } /// If auto-translate is enabled, triggers translation to the user's top preferred language. diff --git a/firefox-ios/Client/Telemetry/TabErrorTelemetryHelper.swift b/firefox-ios/Client/Telemetry/TabErrorTelemetryHelper.swift index 90f600a062a43..8d0192b3acf3d 100644 --- a/firefox-ios/Client/Telemetry/TabErrorTelemetryHelper.swift +++ b/firefox-ios/Client/Telemetry/TabErrorTelemetryHelper.swift @@ -77,9 +77,8 @@ final class TabErrorTelemetryHelper { // MARK: - Internal Utility private func recordTabCount(_ window: WindowUUID, entryPoint: EntryPoint) { - guard self.tabManagerAvailable(for: window) else { return } var tabCounts = defaults.object(forKey: entryPoint.defaultsKey) as? [String: Int] ?? [String: Int]() - let tabCount = getTotalTabCount(window: window) + guard let tabCount = getTotalTabCount(window: window) else { return } tabCounts[window.uuidString] = tabCount defaults.set(tabCounts, forKey: entryPoint.defaultsKey) } @@ -94,7 +93,10 @@ final class TabErrorTelemetryHelper { // fewer tabs than recorded and we'll send the event erroneously. invalidateTabCount(for: window, entryPoint: entryPoint) } - guard tabManagerAvailable(for: window) else { + + guard let tabCounts = defaults.object(forKey: entryPoint.defaultsKey) as? [String: Int], + let expectedTabCount = tabCounts[window.uuidString] else { return } + guard let currentTabCount = getTotalTabCount(window: window) else { logger.log("Can't validate tab count. Tab manager unavailable.", level: .info, category: .tabs, @@ -103,10 +105,6 @@ final class TabErrorTelemetryHelper { return } - guard let tabCounts = defaults.object(forKey: entryPoint.defaultsKey) as? [String: Int], - let expectedTabCount = tabCounts[window.uuidString] else { return } - let currentTabCount = getTotalTabCount(window: window) - if expectedTabCount > 1 && (expectedTabCount - currentTabCount) > 1 { // Potential tab loss bug detected. Log a MetricKit error. sendTelemetryTabLossDetectedEvent( @@ -123,17 +121,12 @@ final class TabErrorTelemetryHelper { defaults.set(tabCounts, forKey: entryPoint.defaultsKey) } - /// It's possible for this telemetry helper to be called during onboarding flow before - /// any tab managers have been configured. - private func tabManagerAvailable(for uuid: WindowUUID) -> Bool { - guard let info = windowManager.windows[uuid], - info.tabManager != nil else { return false } - return true - } - - private func getTotalTabCount(window: WindowUUID) -> Int { - assert(tabManagerAvailable(for: window), "getTabCount() should not be called prior to TabManager config.") - return windowManager.tabManager(for: window).normalTabs.count + private func getTotalTabCount(window: WindowUUID) -> Int? { + guard let tabManager = windowManager.tabManager(for: window) else { + assertionFailure("getTabCount() should not be called prior to TabManager config.") + return nil + } + return tabManager.normalTabs.count } private func sendTelemetryTabLossDetectedEvent(expected: Int, actual: Int, entryPoint: EntryPoint) { diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockWindowManager.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockWindowManager.swift index 6da1668c0a55f..2fca2eda302b8 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockWindowManager.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockWindowManager.swift @@ -37,7 +37,7 @@ final class MockWindowManager: WindowManager { wrappedManager.newBrowserWindowConfigured(windowInfo, uuid: uuid) } - func tabManager(for windowUUID: WindowUUID) -> TabManager { + func tabManager(for windowUUID: WindowUUID) -> TabManager? { return tabManager } @@ -71,11 +71,7 @@ final class MockWindowManager: WindowManager { wrappedManager.performMultiWindowAction(action) } - func window(for tab: TabUUID) -> WindowUUID? { - wrappedManager.window(for: tab) - } - - func windowExists(uuid: Common.WindowUUID) -> Bool { - wrappedManager.windowExists(uuid: uuid) + func windowUUID(forTab tab: TabUUID) -> WindowUUID? { + wrappedManager.windowUUID(forTab: tab) } } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/TabTray/TabManagerMiddlewareTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/TabTray/TabManagerMiddlewareTests.swift index cdd56bc5f9cef..b472667cfa70d 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/TabTray/TabManagerMiddlewareTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/TabTray/TabManagerMiddlewareTests.swift @@ -283,7 +283,7 @@ final class TabManagerMiddlewareTests: XCTestCase, StoreTestUtility { wait(for: [expectation]) - let selectedTab = mockWindowManager.tabManager(for: .XCTestDefaultUUID).selectedTab + let selectedTab = mockWindowManager.tabManager(for: .XCTestDefaultUUID)!.selectedTab XCTAssertEqual(selectedTab?.displayTitle, "www.mozilla.org") XCTAssertEqual(selectedTab?.url?.absoluteString, "www.mozilla.org") } @@ -451,7 +451,7 @@ final class TabManagerMiddlewareTests: XCTestCase, StoreTestUtility { ) subject.tabsPanelProvider(appState, action) - let selectedTab = mockWindowManager.tabManager(for: .XCTestDefaultUUID).selectedTab + let selectedTab = mockWindowManager.tabManager(for: .XCTestDefaultUUID)!.selectedTab XCTAssertEqual(selectedTab, tab) } @@ -466,7 +466,7 @@ final class TabManagerMiddlewareTests: XCTestCase, StoreTestUtility { ) subject.tabsPanelProvider(appState, action) - let selectedTab = mockWindowManager.tabManager(for: .XCTestDefaultUUID).selectedTab + let selectedTab = mockWindowManager.tabManager(for: .XCTestDefaultUUID)!.selectedTab XCTAssertNotEqual(selectedTab, tab) }