Skip to content

Commit cb8b14a

Browse files
Refactor FXIOS-15851 WindowManager tabManager API should return an optional value (#33928)
* [FXIOS-15851] Refactor tabManager(for:) to return an optional value * Unit test updates * [FXIOS-15851] Comments * Spacing * [FXIOS-15851] Add unit tests
1 parent 6721f5d commit cb8b14a

15 files changed

Lines changed: 105 additions & 109 deletions

File tree

firefox-ios/Client/Application/AppDelegate.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ class AppDelegate: UIResponder,
244244
logger.log("Received memory warning", level: .info, category: .lifecycle)
245245
Task {
246246
for uuid in windowManager.allWindowUUIDs(includingReserved: false) {
247-
await windowManager.tabManager(for: uuid).offloadBackgroundWebViews()
247+
await windowManager.tabManager(for: uuid)?.offloadBackgroundWebViews()
248248
}
249249
}
250250
}

firefox-ios/Client/Application/SceneDelegate.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class SceneDelegate: UIResponder,
176176
// If so, we want to be sure that we select the tab in the correct iPad window.
177177
if shouldRerouteIncomingURLToSpecificWindow(url),
178178
let tabUUID = URLScanner(url: url)?.value(query: "uuid"),
179-
let targetWindow = (AppContainer.shared.resolve() as WindowManager).window(for: tabUUID),
179+
let targetWindow = (AppContainer.shared.resolve() as WindowManager).windowUUID(forTab: tabUUID),
180180
targetWindow != sceneCoordinator?.windowUUID {
181181
DefaultApplicationHelper().open(url, inWindow: targetWindow)
182182
} else {

firefox-ios/Client/Application/WindowManager+DebugUtilities.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ extension WindowManagerImplementation {
1515
var result = "----------- Window Debug Info ------------\n"
1616
result.append("Open windows (\(windows.count)) & normal tabs (via TabManager):\n")
1717
for (idx, (uuid, _)) in windows.enumerated() {
18-
let tabMgr = tabManager(for: uuid)
18+
guard let tabMgr = tabManager(for: uuid) else { continue }
1919
let window = windows[uuid]?.sceneCoordinator?.window
2020
let frame = window?.frame ?? .zero
2121
result.append(" \(idx + 1): \(short(uuid)) (\(tabMgr.normalTabs.count) tabs) (frame: \(frame.debugDescription))\n")

firefox-ios/Client/Application/WindowManager.swift

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ protocol WindowManager {
3030
func newBrowserWindowConfigured(_ windowInfo: AppWindowInfo, uuid: WindowUUID)
3131

3232
/// Convenience. Returns the TabManager for a specific window.
33-
func tabManager(for windowUUID: WindowUUID) -> TabManager
33+
func tabManager(for windowUUID: WindowUUID) -> TabManager?
3434

3535
/// Convenience. Returns all TabManagers for all open windows.
3636
func allWindowTabManagers() -> [TabManager]
@@ -73,11 +73,7 @@ protocol WindowManager {
7373
/// - Parameter tab: the UUID of the tab.
7474
/// - Returns: the UUID of the window hosting it (if available and open).
7575
@MainActor
76-
func window(for tab: TabUUID) -> WindowUUID?
77-
78-
/// Convenience. Provides opportunity for safety checks or window validation.
79-
@MainActor
80-
func windowExists(uuid: WindowUUID) -> Bool
76+
func windowUUID(forTab tab: TabUUID) -> WindowUUID?
8177
}
8278

8379
/// Captures state and coordinator references specific to one particular app window.
@@ -135,25 +131,21 @@ final class WindowManagerImplementation: WindowManager {
135131
clearReservedUUID(uuid)
136132
}
137133

138-
func tabManager(for windowUUID: WindowUUID) -> TabManager {
139-
func unsafeAnyTabManager() -> TabManager {
140-
// This is unsafe, but is the best fallback we have to try to handle non-fatally (but may crash anyway)
141-
if let tabManager = windows.first?.value.tabManager {
142-
logger.log("Unsafe tab manager with windowUUID: \(tabManager.windowUUID)", level: .fatal, category: .window)
143-
}
144-
return windows.first!.value.tabManager!
145-
}
146-
134+
func tabManager(for windowUUID: WindowUUID) -> TabManager? {
147135
guard let window = window(for: windowUUID) else {
148-
assertionFailure("No window for UUID: \(windowUUID). This is a client error.")
149-
logger.log("No window for UUID: \(windowUUID)", level: .fatal, category: .window)
150-
return unsafeAnyTabManager()
136+
if !AppConstants.isRunningUnitTest {
137+
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.")
138+
logger.log("No window for UUID: \(windowUUID)", level: .fatal, category: .window)
139+
}
140+
return nil
151141
}
152142

153143
guard let manager = window.tabManager else {
154-
assertionFailure("Window alive, but no TabManager for UUID: \(windowUUID). This is a client error.")
155-
logger.log("Window alive, but no TabManager for UUID: \(windowUUID)", level: .fatal, category: .window)
156-
return unsafeAnyTabManager()
144+
if !AppConstants.isRunningUnitTest {
145+
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.")
146+
logger.log("Window alive, but no TabManager for UUID: \(windowUUID)", level: .fatal, category: .window)
147+
}
148+
return nil
157149
}
158150

159151
return manager
@@ -285,15 +277,10 @@ final class WindowManagerImplementation: WindowManager {
285277
}
286278
}
287279

288-
func window(for tab: TabUUID) -> WindowUUID? {
280+
func windowUUID(forTab tab: TabUUID) -> WindowUUID? {
289281
return allWindowTabManagers().first(where: { $0.tabs.contains(where: { $0.tabUUID == tab }) })?.windowUUID
290282
}
291283

292-
func windowExists(uuid: WindowUUID) -> Bool {
293-
guard uuid != .unavailable else { return false }
294-
return windows[uuid] != nil
295-
}
296-
297284
// MARK: - Internal Utilities
298285

299286
private func clearReservedUUID(_ uuid: WindowUUID) {

firefox-ios/Client/Frontend/Browser/StartAtHome/StartAtHomeMiddleware.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ final class StartAtHomeMiddleware {
4545
/// - Returns: `true` if a homepage tab was selected and displayed, `false` otherwise.
4646
@MainActor
4747
private func startAtHomeCheck(windowUUID: WindowUUID) -> Bool {
48-
let tabManager = windowManager.tabManager(for: windowUUID)
48+
guard let tabManager = windowManager.tabManager(for: windowUUID) else { return false }
4949
let startAtHomeManager = StartAtHomeHelper(
5050
prefs: prefs,
5151
isRestoringTabs: !tabManager.tabRestoreHasFinished

0 commit comments

Comments
 (0)