perf(launch): hold a loading state back until the work behind it outlasts a grace period

CHANGELOG.md

@@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Table collation on MySQL, previously never read.
 - PluginKit ABI 20. Every registry plugin needs rebuilding before or with this release.
 - Cold launch to a usable window, 470ms down to 260ms.
+- Half-second grace before any connecting, schema or query progress indicator appears, and a minimum time on screen once one does.
+- Window chrome stays put through a connect that finishes inside that grace, instead of collapsing and reopening.
 - Plugin signature checks run after the first window rather than on the launch thread, at 13ms each.
 - One gate in front of every path that loads a plugin's executable, enabling one included.
 - Stale `cloudflared` and `cloud-sql-proxy` cleanup waits for the process to exit before a connection reuses its port.

TablePro/Core/Compare/CompareSyncProfileStorage.swift

@@ -46,7 +46,6 @@ internal struct CompareSyncProfile: Codable, Hashable, Identifiable {
         self.dataOptions = dataOptions
         self.selectedObjects = selectedObjects
     }
-
 }
 
 extension DatabaseScope: Codable {

TablePro/Core/SSH/SSHPublicKeyFile.swift

@@ -29,7 +29,7 @@ internal enum SSHPublicKeyFile {
 
     /// A `.pub` line is a few hundred bytes. The cap is here because the same path may name a
     /// private key, a directory entry, or something else entirely.
-    private static let maximumFileSize = 64 * 1024
+    private static let maximumFileSize = 64 * 1_024
 
     /// Every public key an identity file resolves to, in the order OpenSSH looks for them.
     static func blobs(atIdentityPath path: String) -> [SSHPublicKeyBlob] {

TablePro/Core/Services/Infrastructure/ConnectionWindowPaneResolver.swift

@@ -6,6 +6,11 @@
 import Foundation
 
 internal enum ConnectionWindowPane: Equatable {
+    /// A connect too young to be worth saying anything about. It draws nothing and, unlike every
+    /// other contentless pane, it leaves the window's chrome alone: a local file opens in about
+    /// 40ms, and collapsing the sidebar and inspector for that long only to put them back is a
+    /// layout cycle nobody asked for and a flash the HIG names outright.
+    case preparing
     case connecting
     case unavailable(ConnectionUnavailableReason)
     case content
@@ -23,10 +28,22 @@ internal enum SidebarChromeMode: Equatable {
 }
 
 internal enum ConnectionWindowPaneResolver {
+    /// `hasOutlastedGrace` is false for the first `LoadingRevealPolicy.grace` of a connect and of
+    /// the moment before one starts. Neither is a state worth reporting: the first has not lasted
+    /// long enough to be worth a word, and the second is not "not connected", it is "about to
+    /// dial", a distinction `.idle` alone cannot draw because it answers for both. Measured on the
+    /// SQLite sample, reporting them built three pane hierarchies and ran a whole chrome collapse
+    /// and reveal inside the first 103ms of a window's life, for a 39ms connect.
+    ///
+    /// The grace expiring is the exit from `.preparing` in both directions, which is why `.idle`
+    /// reads it too. A connect that never starts, because the phase disallowed it or the record
+    /// went missing, would otherwise leave the window silently empty for good.
     internal static func pane(
         phase: ConnectionWindowPhase,
         hasConnection: Bool,
-        hasRenderableSession: Bool
+        hasRenderableSession: Bool,
+        awaitsAutoConnect: Bool = false,
+        hasOutlastedGrace: Bool = true
     ) -> ConnectionWindowPane {
         switch phase {
         case .closing:
@@ -35,19 +52,45 @@ internal enum ConnectionWindowPaneResolver {
             return hasRenderableSession ? .content : .empty
         case .idle:
             if hasRenderableSession { return .content }
-            return hasConnection ? .unavailable(.notConnected) : .empty
+            guard hasConnection else { return .empty }
+            guard awaitsAutoConnect, !hasOutlastedGrace else { return .unavailable(.notConnected) }
+            return .preparing
         case .connecting:
-            return hasConnection ? .connecting : .empty
+            guard hasConnection else { return .empty }
+            return hasOutlastedGrace ? .connecting : .preparing
         case .unavailable(let reason):
             return hasConnection ? .unavailable(reason) : .empty
         }
     }
 
+    /// Whether this phase is one the grace timer runs over, so a caller knows when to arm it and
+    /// when to let it go. It is the exact set of phases `pane` answers differently for depending
+    /// on `showsProgress`, plus the pre-dial `.idle` that resolves to `.preparing` on its own.
+    internal static func awaitsProgressGrace(
+        phase: ConnectionWindowPhase,
+        awaitsAutoConnect: Bool
+    ) -> Bool {
+        switch phase {
+        case .connecting:
+            return true
+        case .idle:
+            return awaitsAutoConnect
+        case .connected, .closing, .unavailable:
+            return false
+        }
+    }
+
     /// An object browser and an inspector with nothing to put in them are not chrome, they are two
     /// empty columns that promise a session the window does not have yet.
+    ///
+    /// That argument holds for a wait the user can see and not for one they cannot. `.preparing`
+    /// is the sub-grace case and keeps the chrome, so the window that opens is the window that
+    /// stays: on the happy path nothing collapses, nothing is put back, and the panes are built
+    /// once. Collapsing for 40ms costs `splitView.autosaveName`, both split items and a
+    /// `recalculateKeyViewLoop()` in each direction, all of it to show an empty column briefly.
     internal static func hidesChrome(for pane: ConnectionWindowPane) -> Bool {
         switch pane {
-        case .content:
+        case .content, .preparing:
             return false
         case .connecting, .unavailable, .empty:
             return true

TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift

@@ -205,10 +205,45 @@ internal final class ConnectionWorkspace {
         ConnectionWindowPaneResolver.pane(
             phase: phase,
             hasConnection: connection != nil,
-            hasRenderableSession: session != nil && rightPanelState != nil && sessionState != nil
+            hasRenderableSession: session != nil && rightPanelState != nil && sessionState != nil,
+            awaitsAutoConnect: autoConnect,
+            hasOutlastedGrace: hasOutlastedConnectGrace
         )
     }
 
+    /// Whether this connection's dialling has lasted long enough to be worth reporting.
+    ///
+    /// It belongs to the workspace and not to the window, for the same reason `attemptToken` does:
+    /// a window hosts several connections and each dials on its own clock, so a window-wide flag
+    /// would let one connection's slow server put a progress screen over another's finished one.
+    internal private(set) var hasOutlastedConnectGrace = false
+
+    @ObservationIgnored private var progressGraceTask: Task<Void, Never>?
+
+    /// Starts, or leaves running, the wait that decides whether this connect ever says so.
+    ///
+    /// `onReveal` is how the timer reaches the renderer, because the workspace owns the state and
+    /// the controller owns the panes. Re-arming while a wait is already running is a no-op, so the
+    /// phase churn of a reconnect cannot keep pushing the reveal further out.
+    internal func armConnectingProgressGrace(onReveal: @escaping @MainActor () -> Void) {
+        guard !hasOutlastedConnectGrace, progressGraceTask == nil else { return }
+        progressGraceTask = Task { @MainActor [weak self] in
+            try? await Task.sleep(for: LoadingRevealPolicy.grace)
+            guard !Task.isCancelled, let self, !self.isReleased else { return }
+            self.progressGraceTask = nil
+            self.hasOutlastedConnectGrace = true
+            onReveal()
+        }
+    }
+
+    /// Ends the wait and takes the reveal with it, so the next connect starts its own grace rather
+    /// than inheriting a flag the last one set.
+    internal func cancelConnectingProgressGrace() {
+        progressGraceTask?.cancel()
+        progressGraceTask = nil
+        hasOutlastedConnectGrace = false
+    }
+
     /// Everything the panes are built from, compared against `panes.renderedKey` to decide whether
     /// they have to be built at all.
     internal var paneRenderKey: WorkspacePaneRenderKey {
@@ -253,6 +288,7 @@ internal final class ConnectionWorkspace {
     /// coordinator this tears down, and a coordinator only leaves the app-wide registry on deinit.
     internal func teardown() {
         isReleased = true
+        cancelConnectingProgressGrace()
         browseCancellable = nil
         statusCancellable = nil
         tabsCancellable = nil

TablePro/Core/Services/Infrastructure/MainSplitViewController.swift

@@ -176,6 +176,12 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
             /// even though nothing about the connection has changed.
             workspace.panes.invalidate()
             workspaces.insert(workspace)
+            /// The pending grace belongs to the controller it is leaving for the same reason the
+            /// panes do: its reveal calls back into that one. Dropping it and arming again is what
+            /// re-points it here, and re-arming alone would not, because a wait already running is
+            /// deliberately left alone.
+            workspace.cancelConnectingProgressGrace()
+            syncConnectingProgressGrace(of: workspace)
         } else {
             adoptWorkspace(payload: payload, autoConnect: autoConnect)
         }
@@ -246,6 +252,11 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
             phase: phase
         )
         let adopted = workspaces.insert(workspace)
+        /// Armed at creation and not only at the first phase change, because a workspace waiting to
+        /// dial is already resolving to `.preparing` and nothing else would ever time it out. That
+        /// is the exit `startActivationConnectIfNeeded` cannot promise: it returns without dialling
+        /// when the phase disallows it or the connection record has gone.
+        syncConnectingProgressGrace(of: adopted)
 
         /// A workspace adopted into a window that is already on screen has to dial for itself.
         /// `viewWillAppear` is what starts the connect for the window's first workspace, and it
@@ -635,6 +646,7 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         guard let workspace = workspaces.workspace(for: connectionId) else { return }
         let phaseChanged = workspace.phase != next
         workspace.phase = next
+        syncConnectingProgressGrace(of: workspace)
         syncPanes(of: workspace)
         guard phaseChanged else { return }
         if workspaces.selectedConnectionId == connectionId {
@@ -703,6 +715,29 @@ internal final class MainSplitViewController: NSSplitViewController, InspectorVi
         refreshPanes(of: workspace)
     }
 
+    /// Runs the wait that decides whether a connect ever announces itself, for the workspace it
+    /// names, selected or not. A background connection dials on its own clock.
+    ///
+    /// The reveal repaints through `syncPanes` rather than `refreshPanes`, so it costs nothing on
+    /// the ordinary path where the connect landed first and the flag never flipped: the render key
+    /// carries the flag, so a repaint is only ever done when the pane it names actually moved.
+    private func syncConnectingProgressGrace(of workspace: ConnectionWorkspace) {
+        guard ConnectionWindowPaneResolver.awaitsProgressGrace(
+            phase: workspace.phase,
+            awaitsAutoConnect: workspace.autoConnect
+        ) else {
+            workspace.cancelConnectingProgressGrace()
+            return
+        }
+        workspace.armConnectingProgressGrace { [weak self, weak workspace] in
+            guard let self, let workspace, self.isViewLoaded else { return }
+            self.syncPanes(of: workspace)
+            guard self.isShowing(workspace) else { return }
+            self.applyPaneChrome()
+            self.applyWindowTitle()
+        }
+    }
+
     private func syncSelectedPanes() {
         guard let selected = workspaces.selected else { return }
         syncPanes(of: selected)

TablePro/Models/Sidebar/SidebarObjectListPresentation.swift

@@ -12,6 +12,10 @@ import Foundation
 /// database that genuinely has no objects, and telling the two apart is the whole reason
 /// this lives outside the view.
 internal enum SidebarObjectListPresentation: Equatable {
+    /// Loading, and not yet for long enough to say so. An empty column is the placeholder the HIG
+    /// asks for, and a local database answers in about 110ms, so a spinner there is a flash rather
+    /// than a report.
+    case preparing
     case loading
     case failed(String)
     case noMatch
@@ -23,11 +27,12 @@ internal enum SidebarObjectListPresentation: Equatable {
         hasActiveFilter: Bool,
         hasAnyMatch: Bool,
         hasRoutines: Bool,
-        hasTriggers: Bool
+        hasTriggers: Bool,
+        hasOutlastedGrace: Bool = true
     ) -> SidebarObjectListPresentation {
         switch state {
         case .idle, .loading:
-            return .loading
+            return hasOutlastedGrace ? .loading : .preparing
         case .failed(let message):
             return .failed(message)
         case .loaded(let tables):

TablePro/Models/UI/LoadingRevealPolicy.swift

@@ -0,0 +1,40 @@
+//
+//  LoadingRevealPolicy.swift
+//  TablePro
+//
+
+import Foundation
+
+/// When progress UI has earned its place on screen.
+///
+/// Two rules, and the second is the one that gets left out. Work that finishes inside `grace`
+/// shows nothing at all, because an indicator the user cannot read costs a view hierarchy to
+/// build and tear down and tells them nothing. Work that outlasts it keeps its indicator for
+/// `minimumDwell`, because an indicator revealed at 500ms over work that ends at 510ms is a 10ms
+/// flash, which is worse than either of the states it sits between.
+///
+/// The HIG carries the same rule from both ends: progress indicators are for "situations where
+/// loading takes more than a moment or two", and a first screen that differs from what replaces it
+/// gives "an unpleasant flash between the launch screen and your first screen".
+internal enum LoadingRevealPolicy {
+    /// Long enough that nothing local ever reaches it, short enough to stay under the one second
+    /// at which a wait stops feeling like part of the same gesture. It is the value
+    /// `DelayedProgressIndicator` already used for schema refreshes.
+    internal static let grace: Duration = .milliseconds(500)
+
+    internal static let minimumDwell: Duration = .milliseconds(500)
+
+    /// How much longer an indicator revealed at `revealedAt` has to stay before it may go.
+    ///
+    /// Measured from the reveal rather than from the moment the work ended, so anything slow
+    /// enough to have shown an indicator at all has usually already served its dwell and hides
+    /// the instant it finishes. Only the narrow band just past the grace waits.
+    internal static func remainingDwell(
+        revealedAt: ContinuousClock.Instant,
+        now: ContinuousClock.Instant
+    ) -> Duration {
+        let shown = revealedAt.duration(to: now)
+        guard shown < minimumDwell else { return .zero }
+        return minimumDwell - shown
+    }
+}

TablePro/Views/Components/DelayedProgressIndicator.swift

@@ -5,31 +5,19 @@
 
 import SwiftUI
 
-/// A spinner that only appears once an operation outlasts `delay`. AppKit and SwiftUI
-/// ship no delayed progress indicator, so short refreshes would otherwise flash a
-/// spinner that resolves before the user can read it.
+/// A small spinner that only appears once an operation outlasts `LoadingRevealPolicy.grace`, and
+/// then stays long enough to be read. AppKit and SwiftUI ship no delayed progress indicator.
+///
+/// It used to carry the grace and not the dwell, which left it able to flash: work that ended
+/// just past the grace showed a spinner for the few milliseconds between the two.
 struct DelayedProgressIndicator: View {
     let isActive: Bool
-    var delay: Duration = .milliseconds(500)
-
-    @State private var isVisible = false
 
     var body: some View {
-        Group {
-            if isVisible {
-                ProgressView()
-                    .progressViewStyle(.circular)
-                    .controlSize(.small)
-            }
-        }
-        .task(id: isActive) {
-            guard isActive else {
-                isVisible = false
-                return
-            }
-            try? await Task.sleep(for: delay)
-            guard !Task.isCancelled else { return }
-            isVisible = true
+        LoadingReveal(isActive: isActive) {
+            ProgressView()
+                .progressViewStyle(.circular)
+                .controlSize(.small)
         }
     }
 }