refactor: drive frame autosave from NSWindowDelegate methods, no flicker

Author: datlechin

TablePro/Core/Services/Infrastructure/TabWindowController.swift

@@ -2,28 +2,11 @@
 //  TabWindowController.swift
 //  TablePro
 //
-//  NSWindowController for an editor-tab-window. Replaces the SwiftUI
-//  `WindowGroup(id: "main", for: EditorTabPayload.self)` scene.
-//
-//  Phase 1 scope: window creation, NSHostingView installation, tabbing
-//  configuration. Existing MainContentView lifecycle hooks (.onAppear,
-//  .onDisappear, NSWindow notification observers, .userActivity) continue to
-//  work unchanged — this controller's job in Phase 1 is limited to replacing
-//  SwiftUI scene-driven window construction.
-//
-//  Phase 2 will migrate lifecycle responsibilities (markActivated, teardown,
-//  userActivity, didBecomeKey/didResignKey) into NSWindowDelegate methods
-//  on this controller.
-//
 
 import AppKit
 import os
 import SwiftUI
 
-/// NSWindow subclass that routes the standard tab-related responder selectors
-/// (`performClose:`, `newWindowForTab:`) through the coordinator. Menus and
-/// toolbar buttons reach this via `NSApp.sendAction(_:to:nil:from:)`, the same
-/// pattern AppKit uses for `selectNextTab:` and `selectPreviousTab:`.
 @MainActor
 private final class EditorWindow: NSWindow {
     override func performClose(_ sender: Any?) {
@@ -48,18 +31,11 @@ private final class EditorWindow: NSWindow {
 @MainActor
 internal final class TabWindowController: NSWindowController, NSWindowDelegate {
     private static let lifecycleLogger = Logger(subsystem: "com.TablePro", category: "NativeTabLifecycle")
-    private static let frameLogger = Logger(subsystem: "com.TablePro", category: "WindowFrame")
 
     internal static let frameAutosaveName: NSWindow.FrameAutosaveName = "MainEditorWindow"
 
-    private static var frameDefaultsKey: String { "NSWindow Frame \(frameAutosaveName)" }
-
     internal static var hasSavedFrame: Bool {
-        UserDefaults.standard.object(forKey: frameDefaultsKey) != nil
-    }
-
-    private static func currentSavedFrameString() -> String {
-        UserDefaults.standard.string(forKey: frameDefaultsKey) ?? "<nil>"
+        UserDefaults.standard.object(forKey: "NSWindow Frame \(frameAutosaveName)") != nil
     }
 
     private lazy var dataGridFieldEditor: DataGridFieldEditor = {
@@ -70,18 +46,8 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
 
     internal let payload: EditorTabPayload
 
-    /// Stable identifier for this controller. Distinct from the
-    /// `MainContentView.@State windowId` used inside WindowLifecycleMonitor —
-    /// that one remains the authoritative per-view UUID in Phase 1. Phase 2
-    /// will unify them on this controller's identifier.
     internal let controllerId: UUID
 
-    /// NSUserActivity published while this window is key, so Handoff and
-    /// other continuity flows can pick up the connection (and table, if
-    /// viewing one). Replaces the SwiftUI `.userActivity(...)` modifier we
-    /// removed in Phase 2 — `.userActivity` requires a Scene context and
-    /// emitted `Cannot use Scene methods for URL, NSUserActivity...` warnings
-    /// when used inside an `NSHostingView`.
     private var activity: NSUserActivity?
 
     internal init(payload: EditorTabPayload, sessionState: SessionStateFactory.SessionState? = nil) {
@@ -98,49 +64,22 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
         window.minSize = NSSize(width: 720, height: 480)
         window.isRestorable = false
         window.toolbarStyle = .unified
-        // Hide the window title ("Query 1 / TablePro") embedded in the unified
-        // toolbar — otherwise it claims leading space and pushes our navigation
-        // items to the right of it. Tab group's tab bar already shows the same
-        // "Query N" label, so no information is lost. The Principal toolbar item
-        // continues to show connection name + DB version.
         window.titleVisibility = .hidden
         window.tabbingMode = .preferred
         window.tabbingIdentifier = WindowManager.tabbingIdentifier(for: payload.connectionId)
         window.collectionBehavior.insert([.fullScreenPrimary, .managed])
 
-        // NSSplitViewController as contentViewController so .toggleSidebar and
-        // .sidebarTrackingSeparator find the split view via the responder chain.
+        window.setFrameAutosaveName(Self.frameAutosaveName)
+        _ = window.setFrameUsingName(Self.frameAutosaveName)
+
         let splitVC = MainSplitViewController(payload: payload, sessionState: sessionState)
         window.contentViewController = splitVC
 
         super.init(window: window)
 
-        Self.frameLogger.notice("[init] before-autosave persistedFrame=\(Self.currentSavedFrameString(), privacy: .public) windowFrame=\(NSStringFromRect(window.frame), privacy: .public)")
-
-        self.windowFrameAutosaveName = Self.frameAutosaveName
-        Self.frameLogger.notice("[init] after windowFrameAutosaveName setter windowAutosaveName=\(window.frameAutosaveName, privacy: .public) windowFrame=\(NSStringFromRect(window.frame), privacy: .public)")
-
-        let restored = window.setFrameUsingName(Self.frameAutosaveName)
-        Self.frameLogger.notice("[init] explicit setFrameUsingName returned=\(restored, privacy: .public) windowFrame=\(NSStringFromRect(window.frame), privacy: .public)")
-
-        installFrameAutosaveTrace(on: window)
-
-        // Keep the controller alive after the window closes so NSWindowDelegate
-        // hooks have time to run teardown. WindowManager drops its strong
-        // reference on willClose, which triggers dealloc.
         window.isReleasedWhenClosed = false
-
-        // Become the window's delegate so didBecomeKey/didResignKey/willClose
-        // dispatch to methods on this controller — eliminates the global
-        // NotificationCenter fan-out that previously ran every ContentView
-        // instance's observer per focus change.
         window.delegate = self
 
-        // Toolbar is installed by MainSplitViewController.viewWillAppear when
-        // the session state is available. NSSplitViewController does not
-        // overwrite window.toolbar (unlike NavigationSplitView), so no KVO
-        // workaround is needed.
-
         Self.lifecycleLogger.info(
             "[open] TabWindowController.init payloadId=\(payload.id, privacy: .public) connId=\(payload.connectionId, privacy: .public) controllerId=\(self.controllerId, privacy: .public) eagerToolbar=\(sessionState != nil)"
         )
@@ -151,46 +90,29 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
         fatalError("TabWindowController does not support NSCoder init")
     }
 
-    private var frameTraceObservers: [NSObjectProtocol] = []
-
-    private func installFrameAutosaveTrace(on window: NSWindow) {
-        let center = NotificationCenter.default
-
-        frameTraceObservers.append(center.addObserver(
-            forName: NSWindow.didResizeNotification,
-            object: window,
-            queue: .main
-        ) { [weak window] _ in
-            guard let window else { return }
-            Self.frameLogger.notice("[event] didResize windowFrame=\(NSStringFromRect(window.frame), privacy: .public) inLiveResize=\(window.inLiveResize, privacy: .public) persistedFrame=\(Self.currentSavedFrameString(), privacy: .public)")
-        })
-
-        frameTraceObservers.append(center.addObserver(
-            forName: NSWindow.didMoveNotification,
-            object: window,
-            queue: .main
-        ) { [weak window] _ in
-            guard let window else { return }
-            Self.frameLogger.notice("[event] didMove windowFrame=\(NSStringFromRect(window.frame), privacy: .public) persistedFrame=\(Self.currentSavedFrameString(), privacy: .public)")
-        })
-
-        frameTraceObservers.append(center.addObserver(
-            forName: NSWindow.didEndLiveResizeNotification,
-            object: window,
-            queue: .main
-        ) { [weak window] _ in
-            guard let window else { return }
-            Self.frameLogger.notice("[event] didEndLiveResize windowFrame=\(NSStringFromRect(window.frame), privacy: .public) persistedFrame=\(Self.currentSavedFrameString(), privacy: .public)")
-        })
-    }
-
     // MARK: - NSWindowDelegate
 
     func windowWillReturnFieldEditor(_ sender: NSWindow, to client: Any?) -> Any? {
         guard client is CellTextField else { return nil }
         return dataGridFieldEditor
     }
 
+    internal func windowDidResize(_ notification: Notification) {
+        guard let window = notification.object as? NSWindow else { return }
+        guard !window.inLiveResize else { return }
+        window.saveFrame(usingName: Self.frameAutosaveName)
+    }
+
+    internal func windowDidEndLiveResize(_ notification: Notification) {
+        guard let window = notification.object as? NSWindow else { return }
+        window.saveFrame(usingName: Self.frameAutosaveName)
+    }
+
+    internal func windowDidMove(_ notification: Notification) {
+        guard let window = notification.object as? NSWindow else { return }
+        window.saveFrame(usingName: Self.frameAutosaveName)
+    }
+
     internal func windowDidBecomeKey(_ notification: Notification) {
         let seq = MainContentCoordinator.nextSwitchSeq()
         let t0 = Date()
@@ -235,14 +157,7 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
         guard let window = notification.object as? NSWindow else { return }
         Self.lifecycleLogger.info("[close] windowWillClose seq=\(seq) controllerId=\(self.controllerId, privacy: .public)")
 
-        Self.frameLogger.notice("[close] before saveFrame windowFrame=\(NSStringFromRect(window.frame), privacy: .public) styleMask.fullScreen=\(window.styleMask.contains(.fullScreen), privacy: .public) persistedFrame=\(Self.currentSavedFrameString(), privacy: .public)")
         window.saveFrame(usingName: Self.frameAutosaveName)
-        Self.frameLogger.notice("[close] after saveFrame persistedFrame=\(Self.currentSavedFrameString(), privacy: .public)")
-
-        for observer in frameTraceObservers {
-            NotificationCenter.default.removeObserver(observer)
-        }
-        frameTraceObservers.removeAll()
 
         if let splitVC = window.contentViewController as? MainSplitViewController {
             splitVC.invalidateToolbar()
@@ -262,10 +177,6 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
 
     // MARK: - NSUserActivity
 
-    /// Publish (or refresh) this window's NSUserActivity. Called by
-    /// `windowDidBecomeKey` and by `MainContentView` when the selected tab
-    /// changes — only the second case is a no-op when the window isn't key
-    /// (Handoff only cares about the active activity).
     internal func refreshUserActivity() {
         guard let window, window.isKeyWindow,
               let coordinator = MainContentCoordinator.coordinator(forWindow: window)
@@ -279,8 +190,6 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
         let tableName: String? = (selectedTab?.tabType == .table) ? selectedTab?.tableContext.tableName : nil
         let activityType = tableName != nil ? "com.TablePro.viewTable" : "com.TablePro.viewConnection"
 
-        // Recreate when the activity type flips between viewConnection and
-        // viewTable — NSUserActivity.activityType is immutable.
         if activity?.activityType != activityType {
             activity?.invalidate()
             let newActivity = NSUserActivity(activityType: activityType)
@@ -296,13 +205,6 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate {
         }
         activity.userInfo = info
 
-        // Always promote to current. Both call sites (`windowDidBecomeKey` and
-        // `refreshUserActivity` which guards on `window.isKeyWindow`) only
-        // invoke this method when the window owns Handoff. The previous
-        // `becomeCurrent: Bool` parameter dropped Continuity mid-session
-        // whenever the user switched between table and query tabs in the
-        // same window — the type-flip branch above invalidated the old
-        // activity but never promoted the replacement.
         activity.becomeCurrent()
     }
 }