fix(ios)!: add stopMediaHandling() so hosts can break the media cycle

[jkmassel]

Stacked on #637. Follow-up to review feedback on #625.

What?

EditorViewController takes mediaProcessor and mediaUploader at init, and gains a public stopMediaHandling() that stops the upload server, releases both handlers, and withdraws the upload endpoint from the page.

Why?

#625 made mediaProcessor and mediaUploader strong so an in-flight upload can't lose its handler mid-request. That's the right call, but it means a host object that holds the editor back closes a cycle ARC cannot break:

editor → mediaProcessor → coordinator → editor

Dropping the reference later wouldn't help: once the server has started there is a second strong path, editor → uploadServer → HTTPServer → listener → newConnectionHandler → handler → processor → editor, that never touches the property. The only release point was deinit, which is exactly what a cycle prevents, so every leaked editor also stranded a bound loopback NWListener with a live token — one per post opened.

Why not do it automatically, from a lifecycle callback?

An earlier revision of this branch did exactly that — viewDidDisappear, gated on isBeingDismissed || isMovingFromParent. That specific gate doesn't work: isBeingDismissed is true only on the view controller UIKit actually dismisses, and the editor is a child in every real host, so both flags read false on it. Measured across four presentation shapes, the SwiftUI demo, and the real editor in the demo app — the guard blocked the release every single time:

[0] EditorViewController                      beingDismissed=false
[1] NavigationStackHostingController<AnyView> beingDismissed=false
[2] UIKitNavigationController                 beingDismissed=false
[3] PresentationHostingController<AnyView>    beingDismissed=true

This is also how WordPress-iOS hosts us — PostGBKEditorViewController owns the editor as a strong let and installs it with addChild — so it would have blocked there too.

But a better gate does work, and it's worth being precise about that rather than claiming UIKit can't do it. Walking to the ancestor (WordPress-iOS already ships isBeingDismissedDirectlyOrByAncestor()) plus an orphan check at viewDidDisappear — parent, presentingViewController, presentedViewController and viewIfLoaded?.window all nil — was measured across fourteen hosting shapes. It fires correctly on every dismissal and every pop, including WordPress-iOS's exact shape, with no false positive on being covered by a modal or a push, a tab switch, a UIPageViewController returning to the same instance, a cancelled interactive pop, or a UISplitViewController collapse under a real rotation. Detachment and being covered are distinguishable.

What is not observable is whether a detachment is permanent. A host can re-present or re-attach the same editor instance later, and at the moment of the callback that is byte-identical to the last detachment it will ever see. Since this call is terminal — the listener can't restart and the page is told to stop using it — a wrong guess permanently disables media in an editor that survived, which is worse than the leak it would have prevented.

So the gate stays with the host while the action is terminal. The follow-up below (endpoint requested over the bridge instead of baked in at document start) makes a wrong guess cost a restart rather than the session, and at that point adopting the detector is the obvious next move.

Stopping now withdraws the endpoint from the page

Found while reviewing this, and it is the reason stopMediaHandling() is safe to expose at all.

The port and token are injected once as a WKUserScript at document start. nativeMediaUploadMiddleware re-reads them per request, but deliberately does not retry a failed native upload directly — on the stated assumption that an advertised port is a reachable one:

// the native side only advertises a port the WebView can actually reach (server running
// + cleartext-to-localhost permitted, cleared on stop)

Nothing cleared it. A stopped server left every image insert failing with "Unable to connect. Please check your Internet connection." on a working connection, and left an uploader host's orphan-cleanup DELETE broken with it — surviving reload(), because user scripts re-inject at document start.

revokeNativeUploadEndpoint() clears all three copies: the live page, the localStorage copy getGBKit() falls back to, and the injected user script that would otherwise restore the dead port at the next document start, including the reload that recovers a terminated WebContent process. Uploads fall back to the default WebView path — the path every host without a processor already uses. The JS comment is now true.

Making the mistake findable

A rule nobody reads isn't a fix, so three cheap additions:

• MediaProcessor and MediaUploader drop their AnyObject constraint, matching HTTPRequestHandler in GutenbergKitHTTP, which documents the reason: "A value type cannot participate in a reference cycle at all, so the question doesn't arise." Our public protocols had made the opposite call, in the same repo, for the same hazard. This doesn't prevent the cycle — a struct holding a class reference closes it just as well — but it makes the acyclic shape expressible, and an AnyObject protocol taken at init reads like an invitation to pass self. Source-compatible: every existing class conformer still conforms.
• A DEBUG-only census counts live upload servers and logs a fault past four, naming the handler type. Each live server is a bound loopback listener, one per editor, so monotone growth is this cycle and nothing else produces it — warmup() passes no handlers and starts no server. It's the only detectable symptom available: a deinit assertion cannot fire, because a cycle is what stops deinit from running. Logged, never fatal — the threshold is a heuristic, and crashing a host's debug build over a heuristic is a worse trade than the leak.
• The rule moved to where hosts read it — a - Parameters: block on init, which had no doc comment at all, plus a Media Handling section in docs/integration.md with the leaking shape and the leaf-object fix side by side, and what reuse across editor sessions requires — the editor drops only its own reference when it goes, so a host sharing one handler keeps its own. That's the expected shape for an uploader, whose background session or offline queue outlives any editor by definition, and it's the safer one: a handler owned by something longer-lived than any editor is a leaf and can't form the cycle at all. Worth stating there: processFile is called off the main actor, so a @MainActor coordinator can't reach its own state from it anyway. The compliant version is shorter than the forbidden one.

How?

• EditorViewController.swift: handlers move into init and become public private(set) var. Removes hasStartedLoading, lateMediaAssignmentMessage(_:) and both preconditions — supplying the handlers at construction makes "assigned too late to take effect" unrepresentable rather than caught at runtime. (Android keeps its settable property and check(...) because a View is inflated, not constructed by the host.) stopMediaHandling() stops the server, clears both handlers, and revokes the endpoint.
• deinit is unchanged and remains the ordinary path: with no cycle, ARC releases the handlers and deinit stops the server. Confirmed in the demo app, where deinit runs on close.
• Demo-iOS/Sources/Views/EditorView.swift: passes the coordinator at init.

Breaking change

editor.mediaProcessor = processor / editor.mediaUploader = uploader no longer compiles. Pass them to the initializer instead:

EditorViewController(configuration: configuration, mediaProcessor: processor)

Both properties stay publicly readable.

Drive-by

mediaProcessor's doc comment had been merged into mediaUploader's as a single stranded block, leaving mediaProcessor — the primary public extension point — with no documentation at all. Pre-existing, introduced somewhere in #628–#630. Repaired here because this PR rewrites the same lines.

Testing Instructions

• stopMediaHandlingBreaksTheOwnershipCycle fails against a version that skips mediaProcessor = nil — both assertions, with the non-retaining control still passing
• stopReleasesProcessorThatRetainsTheServer — new; covers the second strong path, and pins the Network.framework behaviour this relies on (iOS 16+ cancelling a listener releases its captured blocks, rdar://89677097). Fails with "processor leaked" if the stop() is removed
• iOS Simulator xcodebuild test — 988 tests green
• swift test — host suite green
• Demo-iOS builds and runs against the new initializer; deinit observed running on editor close — note this exercises the acyclic path only. The demo's coordinator holds the view model, not the editor, so it cannot form the cycle in any configuration; stopMediaHandlingBreaksTheOwnershipCycle carries the cyclic case alone
• SwiftLint clean

Pre-existing warnings, not introduced here (both on test/media-mock-cleanup, in files this PR doesn't touch): MediaUploadServerTests.swift weak-variable-never-mutated, and GutenbergKitDebugServer/main.swift:55 optional-interpolation. Happy to fix them in #637.

Follow-up worth considering separately

Make the endpoint dynamic — have the page request the port over the existing bridge on first use instead of baking it in at document start. That makes stop() non-terminal, makes a restart possible, and demotes stopMediaHandling() from load-bearing to advisory. The revocation above is the first step of it either way. JS plus native, so its own PR.

[wpmobilebot]

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/649")

_{Built from 5a88648}