Skip to content

Commit c4a9ada

Browse files
7a6163stonerl
authored andcommitted
fix(menubar): preserve settling deadline on re-entry; end settling on layout reset
Two follow-up fixes from code review of the startup settling period: **Preserve settling deadline on performSetup() re-entry (HIGH)** Replace the `Duration`-based `Task.sleep(for:)` with a deadline-based `Task.sleep(until:clock:.continuous)` anchored to a stored `settlingDeadline: ContinuousClock.Instant?`. On re-entry, the new deadline is `max(existingDeadline, newDeadline)`, so a second performSetup() call that fires after systemUptime crosses the 60 s boundary can no longer silently truncate the original 30-second login settling window to 5 seconds. On normal completion, `settlingDeadline` is nil'd so subsequent sessions start fresh. **End settling immediately on user-initiated layout reset (MEDIUM)** `resetLayoutToFreshState()` now cancels `startupSettlingTask` and clears both `isInStartupSettling` and `settlingDeadline` at the top of the function. A user-initiated reset is authoritative: the post-reset cache must be free to run `saveSectionOrder()` and restore logic without being blocked by a settling period that was started before the reset.
1 parent d7966e0 commit c4a9ada

File tree

1 file changed

+25
-6
lines changed

1 file changed

+25
-6
lines changed

Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,11 @@ final class MenuBarItemManager: ObservableObject {
157157
/// subsequent performSetup() call can cancel the previous settling period
158158
/// before starting a new one, preventing multiple concurrent settling tasks.
159159
private var startupSettlingTask: Task<Void, Never>?
160+
/// Absolute deadline for the current startup settling period. Stored so
161+
/// that a re-entry of performSetup() (e.g. permission re-grant) can
162+
/// preserve any remaining time from the original period rather than
163+
/// resetting to a shorter delay based on current systemUptime.
164+
private var settlingDeadline: ContinuousClock.Instant?
160165
/// Persisted bundle identifiers explicitly placed in hidden section.
161166
private var pinnedHiddenBundleIDs = Set<String>()
162167
/// Persisted bundle identifiers explicitly placed in always-hidden section.
@@ -346,28 +351,36 @@ final class MenuBarItemManager: ObservableObject {
346351
// that conflicts with the next, producing the "icon parade" effect.
347352
// After the settling period ends, one final cacheItemsRegardless() enforces the
348353
// user's saved layout against whatever macOS placed items.
349-
let settleDelay: Duration = ProcessInfo.processInfo.systemUptime < 60 ? .seconds(30) : .seconds(5)
354+
//
355+
// On re-entry (e.g. a permission re-grant during the login window): take the
356+
// MAX of the previous deadline and the newly computed one. This prevents a
357+
// second performSetup() call from resetting systemUptime to a higher value
358+
// (> 60 s) and silently truncating the 30-second login settling window.
359+
let preferredDelay: Duration = ProcessInfo.processInfo.systemUptime < 60 ? .seconds(30) : .seconds(5)
360+
let newDeadline = ContinuousClock.now.advanced(by: preferredDelay)
361+
let deadline = max(settlingDeadline ?? newDeadline, newDeadline)
362+
settlingDeadline = deadline
350363
// Cancel any in-flight settling task before starting a new one.
351364
// Prevents multiple concurrent settling tasks if performSetup() is called
352-
// again (e.g. after a display reconnect or permission re-grant triggers
353-
// a second setup pass). The cancelled task exits without touching shared
354-
// state; this call manages isInStartupSettling for the new period.
365+
// again. The cancelled task exits without touching shared state; this call
366+
// manages isInStartupSettling for the new period.
355367
startupSettlingTask?.cancel()
356368
isInStartupSettling = true
357-
MenuBarItemManager.diagLog.debug("performSetup: startup settling period started (\(settleDelay))")
369+
MenuBarItemManager.diagLog.debug("performSetup: startup settling period started (delay: \(preferredDelay))")
358370
// @MainActor ensures the flag flip and final cache call are never
359371
// interleaved with notification-triggered cache cycles between them.
360372
startupSettlingTask = Task { @MainActor [weak self] in
361373
guard let self else { return }
362374
do {
363-
try await Task.sleep(for: settleDelay)
375+
try await Task.sleep(until: deadline, clock: .continuous)
364376
} catch {
365377
// Cancelled by a subsequent performSetup() call; exit without
366378
// touching shared state — the new call manages isInStartupSettling.
367379
MenuBarItemManager.diagLog.debug("performSetup: startup settling task cancelled")
368380
return
369381
}
370382
isInStartupSettling = false
383+
settlingDeadline = nil
371384
MenuBarItemManager.diagLog.debug("performSetup: startup settling period ended, running restore")
372385
// skipRecentMoveCheck: true — relocateNewLeftmostItems/relocatePendingItems
373386
// may have stamped lastMoveOperationTimestamp during settling; without this
@@ -3523,6 +3536,12 @@ extension MenuBarItemManager {
35233536
/// - Returns: The number of items that failed to move.
35243537
func resetLayoutToFreshState() async throws -> Int {
35253538
MenuBarItemManager.diagLog.info("Resetting menu bar layout to fresh state")
3539+
// A user-initiated reset is authoritative: end the startup settling period
3540+
// immediately so that the post-reset cache is not blocked from running restore
3541+
// and saveSectionOrder by an in-flight settling task.
3542+
startupSettlingTask?.cancel()
3543+
isInStartupSettling = false
3544+
settlingDeadline = nil
35263545
isResettingLayout = true
35273546
defer { isResettingLayout = false }
35283547

0 commit comments

Comments
 (0)