wordpress-mobile / wordpress-mobile/WordPress-iOS
Single-source window creation; drop the windowManager scene-less fallback (follow-up to #25643)
Nobody has claimed this yet.
- Dominant language
- Swift
- Stars
- 3.9k
- Forks
- 1.2k
- Avg merge
- 23h 51m
- Merged PRs (30d)
- 58
Description
Follow-up to #25643 (UIScene life-cycle adoption). Non-blocking cleanup — captured here so it isn't lost.
Background
#25643 moved window creation to the first scene connect (WordPressAppDelegate.showInitialUI(in:)). To stay safe if windowManager is touched before any scene has connected (e.g. a background-launch code path), the windowManager lazy getter recovers by asserting and fabricating a window:
if window == nil {
assertionFailure("windowManager accessed before any scene connected")
window = UIWindow(frame: UIScreen.main.bounds)
}
showInitialUI(in:) then carries a matching branch whose only job is to adopt that fabricated window instead of orphaning the UI built into it.
Why it's worth revisiting
- The fabricated window is scene-less (
UIWindow(frame:)), which the rest of the new code treats as invalid —WindowManager.displayOverlayingWindowandCompliancePopoverCoordinatorassert a scene-attached main window. It's the onlyUIWindow(frame:)left in the non-test code. - The invariant is enforced from both ends — the lazy fabricates,
showInitialUIrepairs — ~150 lines apart. Removing one without the other silently turns the repair into dead code or reintroduces an orphan-window bug. assertionFailurecompiles out in release, so a missed early-access path ships as "works by accident" (an invisible, half-built UI) rather than a signal.- Correctness rests on every pre-scene-connect caller of
windowManager/RootViewCoordinator.sharedbeing guarded by convention rather than by the type system.
The early-access vector is RootViewCoordinator.shared (a static let) evaluating WordPressAppDelegate.shared?.windowManager at first access and caching it for the process lifetime — so the first touch of RootViewCoordinator.shared / .sharedPresenter is what drags windowManager into existence. Its own windowManager property is already WindowManager?, so the consumer already tolerates nil.
Proposed direction
-
Single creation site + explicit optional. Make
windowManageran assignedWindowManager?, set when the window is built inshowInitialUI. Delete the lazy fallback and theexistingWindowadoption branch. Early access becomes anilcallers handle (composes withRootViewCoordinatoralready takingWindowManager?) instead of a fabricated scene-less window. Ripple is a couple of?at call sites. -
Remove the early-access vector (deeper). Stop
RootViewCoordinator.sharedfrom eagerly readingwindowManagerat static-init — resolve it lazily at point-of-use:private var windowManager: WindowManager? { WordPressAppDelegate.shared?.windowManager }Then nothing reaches
windowManagerbefore the scene exists, the fallback is provably dead and can be deleted outright, and the property could even go back to non-optional.
Files
WordPress/Classes/System/WordPressAppDelegate.swift—windowManagerlazy getter,showInitialUI(in:)WordPress/Classes/System/Root View/RootViewCoordinator.swift—static let shared/sharedPresenter
Surfaced during review of #25643. The PR description already notes that "the separation between the UI and the app process is still not very clean, but that can be tightened in future PRs" — this is part of that.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read the windowManager lazy getter and showInitialUI(in:) in WordPress/Classes/System/WordPressAppDelegate.swift, then trace static let shared and sharedPresenter in WordPress/Classes/System/Root View/RootViewCoordinator.swift. Compare the two proposed approaches and confirm callers handle early access safely. Done means one window creation path remains, the scene-less fallback and matching adoption branch are removed, and relevant Swift tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ios, swift
- Domain
- mobile
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100