Skip to content

Refactor FXIOS-15851 WindowManager tabManager API should return an optional value #33928

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

Open
mattreaganmozilla wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Refactor FXIOS-15851 WindowManager tabManager API should return an optional value #33928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e169e1e
[FXIOS-15851] Refactor tabManager(for:) to return an optional value
mattreaganmozilla May 20, 2026
ec838e8
Unit test updates
mattreaganmozilla May 20, 2026
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
2 changes: 1 addition & 1 deletion firefox-ios/Client/Application/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Application/SceneDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Application/WindowManager+DebugUtilities.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
33 changes: 8 additions & 25 deletions firefox-ios/Client/Application/WindowManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Collaborator Author

@mattreaganmozilla mattreaganmozilla May 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed windowExists() because I think it was a code smell. Generally callers should not need to directly check this.

func windowUUID(forTab tab: TabUUID) -> WindowUUID?
}

/// Captures state and coordinator references specific to one particular app window.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Frontend/Browser/StartAtHome/StartAtHomeMiddleware.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading