diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index 81cc2c208..4d0a370c1 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -84,7 +84,6 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro /// The fetched or provided editor dependencies (settings, assets, preload data). private var dependencies: EditorDependencies? - private var dependencyTaskHandle: Task? /// Error encountered while loading dependencies. private var error: Error? { @@ -351,14 +350,8 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro if let dependencies { // FAST PATH: Dependencies were provided at init() - load immediately. - // - // Deliberately NOT tracked in `dependencyTaskHandle`: `viewDidDisappear` - // cancels that handle to abort the async dependency *fetch*, but the - // fast path is cheap local work that must run to completion — a - // transient disappearance (e.g. a modal presented over the editor) - // cancelling it mid `startUploadServer()` silently disabled native - // uploads for the session. `[weak self]` still makes it a no-op once - // the controller is torn down. + // Not cancellable: cancelling mid-`startUploadServer()` silently disables + // native uploads for the session (#357). Task(priority: .userInitiated) { [weak self] in do { try await self?.loadEditor(dependencies: dependencies) @@ -367,8 +360,11 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro } } } else { - // ASYNC FLOW: No dependencies - fetch them asynchronously - self.dependencyTaskHandle = Task(priority: .userInitiated) { [weak self] in + // ASYNC FLOW: No dependencies - fetch them, then load as above. + // Not cancellable either, for the same reason plus one: nothing restarts + // the fetch, so the editor never recovers from a cancel. Note that + // `viewDidDisappear` fires when the editor is merely covered. See #651. + Task(priority: .userInitiated) { [weak self] in await self?.prepareEditor() } } @@ -384,11 +380,6 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro removeNavigationOverlay() } - public override func viewDidDisappear(_ animated: Bool) { - super.viewDidDisappear(animated) - self.dependencyTaskHandle?.cancel() - } - /// Releases the editor's media handling: stops the local upload server, drops the /// host's ``mediaProcessor`` and ``mediaUploader``, and withdraws the upload /// endpoint from the page. diff --git a/ios/Tests/GutenbergKitTests/EditorViewControllerLifecycleTests.swift b/ios/Tests/GutenbergKitTests/EditorViewControllerLifecycleTests.swift new file mode 100644 index 000000000..a672b38d1 --- /dev/null +++ b/ios/Tests/GutenbergKitTests/EditorViewControllerLifecycleTests.swift @@ -0,0 +1,161 @@ +import Foundation +import Testing + +@testable import GutenbergKit + +#if canImport(UIKit) +import UIKit + +/// What happens to an in-flight dependency fetch when its editor is covered or +/// released. Nothing restarts the fetch, so the editor never recovers from a cancel. +@Suite("EditorViewController dependency fetch lifecycle") +struct EditorViewControllerLifecycleTests: MakesTestFixtures { + static let testSiteURL = URL(string: "https://test.example.com")! + static let testApiRoot = URL(string: "https://test.example.com/wp-json/wp/v2")! + + @MainActor + @Test("covering the editor leaves the dependency fetch running") + func coveringTheEditorDoesNotCancelTheDependencyFetch() async throws { + let session = ParkedURLSession() + let configuration = makeIsolatedConfiguration() + defer { removeStorage(for: configuration) } + defer { session.release() } + let editor = makeEditor(configuration: configuration, session: session) + + _ = editor.view // triggers `viewDidLoad`, which starts the fetch + try await session.waitUntilStarted() + + // The editor appears, then a full-screen modal or a push covers it. + editor.beginAppearanceTransition(true, animated: false) + editor.endAppearanceTransition() + editor.beginAppearanceTransition(false, animated: false) + editor.endAppearanceTransition() + + let cancelled = await session.waitUntilCancelled(timeout: .milliseconds(500)) + #expect(!cancelled) + } + + /// Why `deinit` can't cancel the fetch: `await self?.prepareEditor()` keeps the + /// editor alive until the load finishes, so `deinit` only runs once it's over. + @MainActor + @Test("the in-flight fetch keeps the editor alive until it finishes") + func theInFlightFetchKeepsTheEditorAlive() async throws { + let session = ParkedURLSession() + let configuration = makeIsolatedConfiguration() + defer { removeStorage(for: configuration) } + // Safety net if a throw skips the `release()` below; calling it twice is fine. + defer { session.release() } + var editor: EditorViewController? = makeEditor(configuration: configuration, session: session) + weak let releasedEditor = editor + + _ = editor?.view + try await session.waitUntilStarted() + + editor = nil + try await Task.sleep(for: .milliseconds(250)) + #expect(releasedEditor != nil, "the fetch should hold the editor alive") + + session.release() + let clock = ContinuousClock() + let deadline = clock.now + .seconds(10) + while releasedEditor != nil && clock.now < deadline { + try await Task.sleep(for: .milliseconds(20)) + } + #expect(releasedEditor == nil, "the editor should be freed once the fetch ends") + } + + /// A unique `siteId` per call, so no earlier run's cache can serve the fetch. + /// Pair every call with `removeStorage(for:)`: nothing else deletes the site's files. + private func makeIsolatedConfiguration() -> EditorConfiguration { + makeConfiguration( + siteURL: URL(string: "https://\(UUID().uuidString).example.invalid")! + ) + } + + /// An editor whose every network call lands in `session`. + @MainActor + private func makeEditor( + configuration: EditorConfiguration, + session: ParkedURLSession + ) -> EditorViewController { + EditorViewController( + configuration: configuration, + httpClient: EditorHTTPClient(urlSession: session, authHeader: configuration.authHeader) + ) + } + + /// Deletes what the editor wrote for this site. `EditorViewController` can't be + /// pointed at a temporary directory the way `MakesTestFixtures.makeService` can. + private func removeStorage(for configuration: EditorConfiguration) { + try? FileManager.default.removeItem(at: Paths.storageRoot(for: configuration)) + try? FileManager.default.removeItem(at: Paths.cacheRoot(for: configuration)) + } +} + +/// A `URLSessionProtocol` whose requests hang until `release()`, so a fetch stays in +/// flight for as long as the test needs. Records whether any request was cancelled. +private final class ParkedURLSession: URLSessionProtocol, @unchecked Sendable { + private let lock = NSLock() + private var started = false + private var cancelled = false + private var released = false + + private var isStarted: Bool { lock.withLock { started } } + private var isCancelled: Bool { lock.withLock { cancelled } } + private var isReleased: Bool { lock.withLock { released } } + + func data(for request: URLRequest) async throws -> (Data, URLResponse) { + try await park() + } + + func download(for request: URLRequest, delegate: (any URLSessionTaskDelegate)?) async throws -> (URL, URLResponse) { + try await park() + } + + /// Makes every parked request fail, so the fetch ends. Always call it: a request + /// left parked keeps its editor alive for the rest of the run. + func release() { + lock.withLock { released = true } + } + + /// Suspends until `release()` or until the calling task is cancelled. + /// `Never` because every exit throws, so it fits both methods' return types. + private func park() async throws -> Never { + lock.withLock { started = true } + while !isReleased { + do { + try await Task.sleep(for: .milliseconds(20)) + } catch { + lock.withLock { cancelled = true } + throw URLError(.cancelled) + } + } + throw URLError(.networkConnectionLost) + } + + func waitUntilStarted(timeout: Duration = .seconds(10)) async throws { + let clock = ContinuousClock() + let deadline = clock.now + timeout + while clock.now < deadline { + if isStarted { return } + try await Task.sleep(for: .milliseconds(20)) + } + throw ParkedURLSessionTimeout.requestNeverStarted + } + + func waitUntilCancelled(timeout: Duration) async -> Bool { + let clock = ContinuousClock() + let deadline = clock.now + timeout + while clock.now < deadline { + if isCancelled { return true } + try? await Task.sleep(for: .milliseconds(20)) + } + return isCancelled + } +} + +private enum ParkedURLSessionTimeout: Error { + case requestNeverStarted +} + +#endif