wordpress-mobile / wordpress-mobile/WordPress-iOS

Single-source window creation; drop the windowManager scene-less fallback (follow-up to #25643)

Open
#25,664 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

[Type] Tech Debt
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.displayOverlayingWindow and CompliancePopoverCoordinator assert a scene-attached main window. It's the only UIWindow(frame:) left in the non-test code.
  • The invariant is enforced from both ends — the lazy fabricates, showInitialUI repairs — ~150 lines apart. Removing one without the other silently turns the repair into dead code or reintroduces an orphan-window bug.
  • assertionFailure compiles 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.shared being 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

  1. Single creation site + explicit optional. Make windowManager an assigned WindowManager?, set when the window is built in showInitialUI. Delete the lazy fallback and the existingWindow adoption branch. Early access becomes a nil callers handle (composes with RootViewCoordinator already taking WindowManager?) instead of a fabricated scene-less window. Ripple is a couple of ? at call sites.

  2. Remove the early-access vector (deeper). Stop RootViewCoordinator.shared from eagerly reading windowManager at static-init — resolve it lazily at point-of-use:

    private var windowManager: WindowManager? { WordPressAppDelegate.shared?.windowManager }
    

    Then nothing reaches windowManager before 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.swiftwindowManager lazy getter, showInitialUI(in:)
  • WordPress/Classes/System/Root View/RootViewCoordinator.swiftstatic 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.