From 4cee6ce313436b010aef819919c9547aaf967782 Mon Sep 17 00:00:00 2001 From: Lakr233 Date: Fri, 25 Sep 2026 19:47:39 +0700 Subject: [PATCH] =?UTF-8?q?Say=20"Connecting=E2=80=A6"=20only=20when=20con?= =?UTF-8?q?necting=20is=20slow=20(#7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reopening the app a few seconds after closing it flashed "Connecting to the inspector service…" for a few hundred milliseconds: long enough to see, too short to read. The spinner went up the moment the phase turned to `.connecting`, however quickly the daemon answered. DelayedLoadingIndicator keeps the time instead. A wait that ends inside three quarters of a second shows a blank list and then the rows; a wait that runs longer shows the indicator, which then stays at least half a second so it doesn't blink off, and rows that land in that half second are drawn when it ends. A failure or an empty result never waits: it replaces the indicator, or the blank, at once. The wait for the first sample after connecting counts too, so a slow connection no longer drops from the spinner to an empty list before the rows arrive. The detail tables had the same flash on every push while their records loaded, and use the same indicator. Because the indicator now turns up on a screen that is already showing, VoiceOver is told the layout changed. The first sample itself is not delayed: `firstSampleDelay` is the gap before the second one, which is what CPU use needs. `make harness` now steps the indicator through its grace period, minimum time, failure and retry on macOS. Co-Authored-By: Claude Opus 5.5 --- Inspector/DelayedLoadingIndicator.swift | 109 +++++++++++++ Inspector/InspectorUnavailableView.swift | 9 +- .../ProcessDetailListViewController.swift | 12 +- Inspector/ProcessListViewController.swift | 17 +- Makefile | 4 + Tests/DelayedLoadingHarness.swift | 147 ++++++++++++++++++ 6 files changed, 294 insertions(+), 4 deletions(-) create mode 100644 Inspector/DelayedLoadingIndicator.swift create mode 100644 Tests/DelayedLoadingHarness.swift diff --git a/Inspector/DelayedLoadingIndicator.swift b/Inspector/DelayedLoadingIndicator.swift new file mode 100644 index 0000000..ee0e705 --- /dev/null +++ b/Inspector/DelayedLoadingIndicator.swift @@ -0,0 +1,109 @@ +import Foundation + +// Decides when a loading indicator is worth showing. Loading that finishes +// within the grace period shows nothing at all, so a fast connection doesn't +// flash a spinner for a few frames; loading that runs longer shows the +// indicator, which then stays for a minimum time so it doesn't blink off. +// +// It only keeps time. The owner reports whether it is loading on every render +// and draws what `update(isLoading:)` answers; `onChange` asks it to render +// again when the answer changes on its own. Main thread only. Foundation only, +// so `make harness` can drive it on macOS with a scheduler of its own. +final class DelayedLoadingIndicator { + /// Runs `action` after `delay` seconds, and returns a way to cancel it. + typealias Schedule = (_ delay: TimeInterval, _ action: @escaping () -> Void) -> () -> Void + + private enum Stage { + case idle + // Loading, and the indicator is not due yet. + case waiting + // The indicator is up, and has not been for its minimum time. + case showing + // The indicator is up, and may go whenever the loading ends. + case settled + } + + static let defaultGracePeriod: TimeInterval = 0.75 + static let defaultMinimumVisibleTime: TimeInterval = 0.5 + + /// Called when the indicator appears, and when its minimum time is over. + var onChange: () -> Void = {} + + private let gracePeriod: TimeInterval + private let minimumVisibleTime: TimeInterval + private let schedule: Schedule + private var stage = Stage.idle + private var cancelPending: (() -> Void)? + + init( + gracePeriod: TimeInterval = defaultGracePeriod, + minimumVisibleTime: TimeInterval = defaultMinimumVisibleTime, + schedule: @escaping Schedule = DelayedLoadingIndicator.onMainQueue + ) { + self.gracePeriod = gracePeriod + self.minimumVisibleTime = minimumVisibleTime + self.schedule = schedule + } + + deinit { + cancelPending?() + } + + /// Whether the indicator is on screen. + var isVisible: Bool { + stage == .showing || stage == .settled + } + + /// The indicator has only just appeared. Content that is ready waits for + /// the next `onChange` rather than replacing it at once; a message such + /// as a failure need not wait, and ends it with `update(isLoading: false)`. + var holdsContent: Bool { + stage == .showing + } + + /// Reports whether the screen is loading, and answers whether the + /// indicator belongs on it. Loading that ends ends the indicator at once. + @discardableResult + func update(isLoading: Bool) -> Bool { + if !isLoading { + cancel() + } else if stage == .idle { + stage = .waiting + after(gracePeriod) { [weak self] in self?.reveal() } + } + return isVisible + } + + private func reveal() { + guard stage == .waiting else { return } + stage = .showing + after(minimumVisibleTime) { [weak self] in self?.settle() } + onChange() + } + + private func settle() { + guard stage == .showing else { return } + stage = .settled + cancelPending = nil + onChange() + } + + private func after(_ delay: TimeInterval, _ action: @escaping () -> Void) { + cancelPending?() + cancelPending = schedule(delay, action) + } + + private func cancel() { + cancelPending?() + cancelPending = nil + stage = .idle + } + + // A dispatch deadline, unlike a default-mode timer, still fires while a + // scroll view is tracking a finger. + static let onMainQueue: Schedule = { delay, action in + let work = DispatchWorkItem(block: action) + DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: work) + return work.cancel + } +} diff --git a/Inspector/InspectorUnavailableView.swift b/Inspector/InspectorUnavailableView.swift index 2784ed3..10fa464 100644 --- a/Inspector/InspectorUnavailableView.swift +++ b/Inspector/InspectorUnavailableView.swift @@ -116,6 +116,13 @@ extension UITableView { action: @escaping () -> Void = {} ) { if (backgroundView as? InspectorUnavailableView)?.content == content { return } - backgroundView = content.map { InspectorUnavailableView($0, action: action) } + let view = content.map { InspectorUnavailableView($0, action: action) } + backgroundView = view + // A loading state that turns up on a screen already showing (see + // DelayedLoadingIndicator) changes nothing VoiceOver was told about, + // so it is pointed out; one there from the start is read with the rest. + if case .loading = content, window != nil { + UIAccessibility.post(notification: .layoutChanged, argument: view) + } } } diff --git a/Inspector/ProcessDetailListViewController.swift b/Inspector/ProcessDetailListViewController.swift index c7191ed..f870ecc 100644 --- a/Inspector/ProcessDetailListViewController.swift +++ b/Inspector/ProcessDetailListViewController.swift @@ -16,6 +16,9 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu private var rowsByID: [String: (cells: [String], inspection: DetailRowInspection)] = [:] private var dataSource: UITableViewDiffableDataSource! private var failure: String? + // Details that come back quickly show a blank screen, not a flash of a + // spinner; details that take a while show one long enough to be read. + private let loadingIndicator = DelayedLoadingIndicator() private var searchText = "" private var sort: ProcessDetailSort { didSet { @@ -123,6 +126,9 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu cell.accessibilityHint = String(localized: "Shows the full record") return cell } + loadingIndicator.onChange = { [weak self] in + self?.render() + } render() Task { await load() } } @@ -137,6 +143,10 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu // MARK: Rendering private func render() { + // Rows that arrive just after the spinner appeared wait out its + // minimum time rather than blinking it away; a message never waits. + if loadingIndicator.holdsContent, !rows.isEmpty { return } + let showsLoading = loadingIndicator.update(isLoading: failure == nil && detail == nil) shareButton.isEnabled = detail != nil // An overlay owns the whole screen, so nothing is listed under one. var snapshot = NSDiffableDataSourceSnapshot() @@ -189,7 +199,7 @@ final class ProcessDetailListViewController: UITableViewController, UISearchResu tableView.setUnavailableContent(nil) } } else { - tableView.setUnavailableContent(.loading(nil)) + tableView.setUnavailableContent(showsLoading ? .loading(nil) : nil) } } diff --git a/Inspector/ProcessListViewController.swift b/Inspector/ProcessListViewController.swift index 6a61345..370991d 100644 --- a/Inspector/ProcessListViewController.swift +++ b/Inspector/ProcessListViewController.swift @@ -20,6 +20,9 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd // across the reloads a live sample causes. private var selectedIdentity: ProcessIdentity? private let creditsView = ProcessListCreditsView() + // A connection that comes up quickly shows a blank list, not a flash of + // "Connecting…"; one that takes a while shows it long enough to be read. + private let connectingIndicator = DelayedLoadingIndicator() init(model: ProcessListModel) { self.model = model @@ -66,6 +69,9 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd observation = model.changes.sink { [weak self] in self?.render() } + connectingIndicator.onChange = { [weak self] in + self?.render() + } render() } @@ -128,6 +134,9 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd // MARK: Rendering private func render() { + // Rows that arrive just after "Connecting…" appeared wait out its + // minimum time rather than blinking it away; a message never waits. + if connectingIndicator.holdsContent, !model.visibleRows.isEmpty { return } let rows = model.visibleRows let identities = rows.map(\.id) if identities != shownIdentities { @@ -166,10 +175,14 @@ final class ProcessListViewController: UITableViewController, UISearchResultsUpd } private func renderOverlay() { + // Until the first sample lands there is nothing to list, connected or + // not; the indicator decides whether that is worth saying yet. + let isConnecting = model.rows.isEmpty && (model.phase == .connecting || model.phase == .active) + let showsConnecting = connectingIndicator.update(isLoading: isConnecting) switch model.phase { - case .connecting where model.rows.isEmpty: + case .connecting where model.rows.isEmpty, .active where model.rows.isEmpty: tableView.setUnavailableContent( - .loading(String(localized: "Connecting to the inspector service…")) + showsConnecting ? .loading(String(localized: "Connecting to the inspector service…")) : nil ) case .failed(let message): tableView.setUnavailableContent( diff --git a/Makefile b/Makefile index 104bc58..0000ade 100644 --- a/Makefile +++ b/Makefile @@ -175,6 +175,10 @@ harness: trap 'rm -f "$$harness_bin"' EXIT; \ xcrun --sdk macosx swiftc -swift-version 5 "$(ROOT_DIR)/Inspector/SceneRestorationReset.swift" "$(ROOT_DIR)/Tests/SceneRestorationHarness.swift" -o "$$harness_bin"; \ "$$harness_bin" + @harness_bin="$$(mktemp /tmp/inspector-loading-harness.XXXXXX)"; \ + trap 'rm -f "$$harness_bin"' EXIT; \ + xcrun --sdk macosx swiftc -swift-version 5 "$(ROOT_DIR)/Inspector/DelayedLoadingIndicator.swift" "$(ROOT_DIR)/Tests/DelayedLoadingHarness.swift" -o "$$harness_bin"; \ + "$$harness_bin" build: check harness bump-build XCBUILD_LABEL=build-ios $(XCODEBUILD) \ diff --git a/Tests/DelayedLoadingHarness.swift b/Tests/DelayedLoadingHarness.swift new file mode 100644 index 0000000..33e9c09 --- /dev/null +++ b/Tests/DelayedLoadingHarness.swift @@ -0,0 +1,147 @@ +import Foundation + +// A clock that only moves when told to, so every grace period and minimum +// time can be stepped through exactly, without sleeping. +final class ManualScheduler { + private struct Pending { + let id: Int + let deadline: TimeInterval + let action: () -> Void + } + + private(set) var now: TimeInterval = 0 + private var pending: [Pending] = [] + private var nextID = 0 + + var schedule: DelayedLoadingIndicator.Schedule { + { [unowned self] delay, action in + nextID += 1 + let id = nextID + pending.append(Pending(id: id, deadline: now + delay, action: action)) + return { [weak self] in self?.pending.removeAll { $0.id == id } } + } + } + + var pendingCount: Int { pending.count } + + func advance(by interval: TimeInterval) { + let end = now + interval + while let next = pending.filter({ $0.deadline <= end }).min(by: { $0.deadline < $1.deadline }) { + pending.removeAll { $0.id == next.id } + now = next.deadline + next.action() + } + now = end + } +} + +@main +enum DelayedLoadingHarness { + static func main() { + let grace: TimeInterval = 0.75 + let minimum: TimeInterval = 0.5 + + func make() -> (DelayedLoadingIndicator, ManualScheduler, () -> Int) { + let clock = ManualScheduler() + let indicator = DelayedLoadingIndicator( + gracePeriod: grace, + minimumVisibleTime: minimum, + schedule: clock.schedule + ) + var changes = 0 + indicator.onChange = { changes += 1 } + return (indicator, clock, { changes }) + } + + // Loading that ends inside the grace period never shows anything, + // and leaves nothing scheduled behind it. + do { + let (indicator, clock, changes) = make() + precondition(!indicator.update(isLoading: true)) + clock.advance(by: grace - 0.1) + precondition(!indicator.update(isLoading: true)) + precondition(!indicator.update(isLoading: false)) + precondition(clock.pendingCount == 0) + clock.advance(by: 5) + precondition(changes() == 0) + precondition(!indicator.isVisible && !indicator.holdsContent) + } + + // Reporting loading again doesn't restart the grace period: the + // indicator is due a grace period after loading began. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace / 2) + indicator.update(isLoading: true) + clock.advance(by: grace / 2) + precondition(changes() == 1) + precondition(indicator.update(isLoading: true)) + } + + // Loading that outlasts the grace period shows the indicator, which + // holds content back for its minimum time and then lets it through. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace) + precondition(changes() == 1) + precondition(indicator.isVisible && indicator.holdsContent) + clock.advance(by: minimum - 0.1) + precondition(indicator.holdsContent) + clock.advance(by: 0.1) + precondition(changes() == 2) + precondition(indicator.isVisible && !indicator.holdsContent) + // Loading that runs on keeps it, without another change. + clock.advance(by: 10) + precondition(indicator.update(isLoading: true)) + precondition(changes() == 2) + precondition(!indicator.update(isLoading: false)) + precondition(clock.pendingCount == 0) + } + + // A failure doesn't wait: ending the loading while the indicator is + // still inside its minimum removes it at once, with no late change. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace + 0.1) + precondition(indicator.holdsContent) + precondition(!indicator.update(isLoading: false)) + precondition(!indicator.isVisible && !indicator.holdsContent) + precondition(clock.pendingCount == 0) + clock.advance(by: 5) + precondition(changes() == 1) + } + + // Trying again after that starts over from a fresh grace period. + do { + let (indicator, clock, changes) = make() + indicator.update(isLoading: true) + clock.advance(by: grace + minimum) + indicator.update(isLoading: false) + precondition(!indicator.update(isLoading: true)) + clock.advance(by: grace - 0.1) + precondition(!indicator.isVisible) + clock.advance(by: 0.1) + precondition(indicator.isVisible && indicator.holdsContent) + precondition(changes() == 3) + } + + // An indicator that goes away takes its pending work with it. + do { + let clock = ManualScheduler() + var indicator: DelayedLoadingIndicator? = DelayedLoadingIndicator( + gracePeriod: grace, + minimumVisibleTime: minimum, + schedule: clock.schedule + ) + indicator?.update(isLoading: true) + precondition(clock.pendingCount == 1) + indicator = nil + precondition(clock.pendingCount == 0) + } + + print("Inspector delayed-loading harness passed") + } +}