diff --git a/Sparkle/SPUStandardUserDriver.m b/Sparkle/SPUStandardUserDriver.m index 9562e8a4c..30fbec7de 100644 --- a/Sparkle/SPUStandardUserDriver.m +++ b/Sparkle/SPUStandardUserDriver.m @@ -352,12 +352,39 @@ - (void)showUpdateFoundWithAppcastItem:(SUAppcastItem *)appcastItem state:(SPUUs { assert(NSThread.isMainThread); - [self closeCheckingWindow]; - if (_activeUpdateAlert != nil) { SULog(SULogLevelError, @"Error: -[%@ %@] should not be called when _activeUpdateAlert != nil:\n%@", NSStringFromClass([self class]), NSStringFromSelector(_cmd), NSThread.callStackSymbols); } + _regularApplicationUpdate = [appcastItem.installationType isEqualToString:SPUInstallationTypeApplication]; + + // Defer building the alert until the checking window has finished closing. + // This prevents _activeUpdateAlert and _checkingController being non-nil at + // the same time, which would make ordering of nil tests in ... fragile. + __weak id weakDelegate = _delegate; + [self _closeCheckingWindowWithCompletionBlock:^(SPUStandardUserDriver *s, BOOL userCancelled) { + if (s != nil && !userCancelled) { + [s _buildActiveUpdateAlertForAppcastItem:appcastItem state:state reply:reply]; + + // For user initiated checks, let the delegate know we'll be showing an update. + // For scheduled checks, -setUpActiveUpdateAlertForUpdate:state: below will handle this. + id strongDelegate = weakDelegate; + if (state.userInitiated && [strongDelegate respondsToSelector:@selector(standardUserDriverWillHandleShowingUpdate:forUpdate:state:)]) { + [strongDelegate standardUserDriverWillHandleShowingUpdate:YES forUpdate:appcastItem state:state]; + } + + [s setUpActiveUpdateAlertForScheduledUpdate:(state.userInitiated ? nil : appcastItem) state:state]; + } else { + // Either the driver was deallocated mid-flow or the user cancelled + // during the buffered close; in either case the updater is still + // waiting on reply. Dismiss so it can tear down cleanly. + reply(SPUUserUpdateChoiceDismiss); + } + }]; +} + +- (void)_buildActiveUpdateAlertForAppcastItem:(SUAppcastItem *)appcastItem state:(SPUUserUpdateState *)state reply:(void (^)(SPUUserUpdateChoice))reply SPU_OBJC_DIRECT +{ id delegate = _delegate; id customVersionDisplayer = nil; @@ -435,16 +462,6 @@ - (void)showUpdateFoundWithAppcastItem:(SUAppcastItem *)appcastItem state:(SPUUs } } }]; - - _regularApplicationUpdate = [appcastItem.installationType isEqualToString:SPUInstallationTypeApplication]; - - // For user initiated checks, let the delegate know we'll be showing an update - // For scheduled checks, -setUpActiveUpdateAlertForUpdate:state: below will handle this - if (state.userInitiated && [delegate respondsToSelector:@selector(standardUserDriverWillHandleShowingUpdate:forUpdate:state:)]) { - [delegate standardUserDriverWillHandleShowingUpdate:YES forUpdate:appcastItem state:state]; - } - - [self setUpActiveUpdateAlertForScheduledUpdate:(state.userInitiated ? nil : appcastItem) state:state]; } - (void)showUpdateReleaseNotesWithDownloadData:(SPUDownloadData *)downloadData @@ -560,10 +577,30 @@ - (void)showUserInitiatedUpdateCheckWithCancellation:(void (^)(void))cancellatio [_checkingController showWindow:self]; } -- (void)closeCheckingWindow SPU_OBJC_DIRECT +// Close the "Checking for updates…" window. If userCancelled is YES, this is +// the user clicking Cancel: a pending buffered close (if any) is expedited +// — its completion runs with userCancelled=YES, which lets each call site +// skip the queued next-UI step while still signaling the updater to end its +// session. If no buffered close is pending, the in-flight check is aborted via +// _cancellation directly. If userCancelled is NO, this is the abort/teardown +// path: the window is closed silently with no completion firing. +- (void)closeCheckingWindow:(BOOL)userCancelled SPU_OBJC_DIRECT { - if (_checkingController != nil) - { + if (userCancelled) { + // closeImmediately closes the window regardless. Its return tells us + // whether a pending buffered completion fired (and did its own + // cleanup) or not — if not, we still need to abort the active check. + if ([_checkingController closeImmediately]) { + return; + } + if (_cancellation != nil) { + _cancellation(); + _cancellation = nil; + } + _checkingController = nil; + return; + } + if (_checkingController != nil) { [_checkingController close]; _checkingController = nil; _cancellation = nil; @@ -572,11 +609,45 @@ - (void)closeCheckingWindow SPU_OBJC_DIRECT - (void)cancelCheckForUpdates:(id)__unused sender { - if (_cancellation != nil) { - _cancellation(); - _cancellation = nil; + [self closeCheckingWindow:YES]; +} + +// Initiate closing the "Checking for updates…" window, waiting for any time-buffering +// delay before continuing with the subsequent action. userCancelled is YES if +// the user expedited the close by clicking Cancel during the buffered period. +- (void)_closeCheckingWindowWithCompletionBlock:(void (^)(SPUStandardUserDriver * _Nullable s, BOOL userCancelled))completionBlock SPU_OBJC_DIRECT +{ + if (_checkingController == nil) { + completionBlock(self, NO); + return; } - [self closeCheckingWindow]; + __weak __typeof__(self) weakSelf = self; + [_checkingController closeWithCompletionBlock:^(BOOL userCancelled) { + __typeof__(self) strongSelf = weakSelf; + if (strongSelf != nil) { + strongSelf->_checkingController = nil; + strongSelf->_cancellation = nil; + } + completionBlock(strongSelf, userCancelled); + }]; +} + +// Same as -_closeCheckingWindowWithCompletionBlock: but for the install/progress status +// window. No `_cancellation` to clear on this path. +- (void)_closeStatusWindowWithCompletionBlock:(void (^)(SPUStandardUserDriver * _Nullable s, BOOL userCancelled))completionBlock SPU_OBJC_DIRECT +{ + if (_statusController == nil) { + completionBlock(self, NO); + return; + } + __weak __typeof__(self) weakSelf = self; + [_statusController closeWithCompletionBlock:^(BOOL userCancelled) { + __typeof__(self) strongSelf = weakSelf; + if (strongSelf != nil) { + strongSelf->_statusController = nil; + } + completionBlock(strongSelf, userCancelled); + }]; } #pragma mark Update Errors @@ -585,11 +656,27 @@ - (void)showUpdaterError:(NSError *)error acknowledgement:(void (^)(void))acknow { assert(NSThread.isMainThread); - [self closeCheckingWindow]; + [self _closeCheckingWindowWithCompletionBlock:^(SPUStandardUserDriver *s, BOOL userCancelled) { + if (s != nil && !userCancelled) { + [s _proceedWithUpdaterError:error acknowledgement:acknowledgement]; + } else { + acknowledgement(); + } + }]; +} - [_statusController close]; - _statusController = nil; +- (void)_proceedWithUpdaterError:(NSError *)error acknowledgement:(void (^)(void))acknowledgement SPU_OBJC_DIRECT +{ + [self _closeStatusWindowWithCompletionBlock:^(SPUStandardUserDriver *s, BOOL userCancelled) { + if (s != nil && !userCancelled) { + [s _showUpdaterErrorAlertForError:error]; + } + acknowledgement(); + }]; +} +- (void)_showUpdaterErrorAlertForError:(NSError *)error SPU_OBJC_DIRECT +{ #if SPARKLE_COPY_LOCALIZATIONS NSBundle *sparkleBundle = SUSparkleBundle(); #endif @@ -611,16 +698,23 @@ - (void)showUpdaterError:(NSError *)error acknowledgement:(void (^)(void))acknow [alert addButtonWithTitle:SULocalizedStringFromTableInBundle(@"Cancel Update", SPARKLE_TABLE, sparkleBundle, nil)]; [self showAlert:alert secondaryAction:nil]; - - acknowledgement(); } - (void)showUpdateNotFoundWithError:(NSError *)error acknowledgement:(void (^)(void))acknowledgement { assert(NSThread.isMainThread); - [self closeCheckingWindow]; + [self _closeCheckingWindowWithCompletionBlock:^(SPUStandardUserDriver *s, BOOL userCancelled) { + if (s != nil && !userCancelled) { + [s _proceedWithUpdateNotFoundWithError:error acknowledgement:acknowledgement]; + } else { + acknowledgement(); + } + }]; +} +- (void)_proceedWithUpdateNotFoundWithError:(NSError *)error acknowledgement:(void (^)(void))acknowledgement SPU_OBJC_DIRECT +{ id delegate = _delegate; id customVersionDisplayer; @@ -797,10 +891,17 @@ - (void)showDownloadInitiatedWithCancellation:(void (^)(void))cancellation - (void)cancelDownload:(id)__unused sender { + // closeImmediately closes the window regardless. Its return tells us + // whether a pending buffered completion fired (and did its own cleanup) or + // not — if not, we still need to invoke the active-download cancellation. + if ([_statusController closeImmediately]) { + return; + } if (_cancellation != nil) { _cancellation(); _cancellation = nil; } + _statusController = nil; } - (void)showDownloadDidReceiveExpectedContentLength:(uint64_t)expectedContentLength @@ -873,11 +974,17 @@ - (void)showInstallingUpdateWithApplicationTerminated:(BOOL)applicationTerminate // The "quit" event can always be canceled or delayed by the application we're updating // So we can't easily predict how long the installation will take or if it won't happen right away // We close our status window because we don't want it persisting for too long and have it obscure other windows - [_statusController close]; - _statusController = nil; - - // Keep retry handler in case user tries to show update in focus again - _retryTerminatingApplication = [retryTerminatingApplication copy]; + // The retry handler is assigned alongside the close completion so it + // never co-exists with _statusController. + void (^retry)(void) = [retryTerminatingApplication copy]; + [self _closeStatusWindowWithCompletionBlock:^(SPUStandardUserDriver *s, BOOL __unused userCancelled) { + // userCancelled is ignored: the install is in flight regardless of + // what the user does about the visible status window, so the retry + // handler still needs to be available. + if (s != nil) { + s->_retryTerminatingApplication = retry; + } + }]; } } @@ -885,43 +992,45 @@ - (void)showUpdateInstalledAndRelaunched:(BOOL)relaunched acknowledgement:(void { assert(NSThread.isMainThread); - // Close window showing update is installing - [_statusController close]; - _statusController = nil; + // Only show installed prompt when the app is not relaunched — + // when the app is relaunched, there is enough of a UI from relaunching the app. + [self _closeStatusWindowWithCompletionBlock:^(SPUStandardUserDriver *s, BOOL userCancelled) { + if (s != nil && !userCancelled && !relaunched) { + [s _showUpdateInstalledAlert]; + } + acknowledgement(); + }]; +} - // Only show installed prompt when the app is not relaunched - // When the app is relaunched, there is enough of a UI from relaunching the app. - if (!relaunched) { +- (void)_showUpdateInstalledAlert SPU_OBJC_DIRECT +{ #if SPARKLE_COPY_LOCALIZATIONS - NSBundle *sparkleBundle = SUSparkleBundle(); + NSBundle *sparkleBundle = SUSparkleBundle(); #endif - NSAlert *alert = [[NSAlert alloc] init]; - alert.messageText = SULocalizedStringFromTableInBundle(@"Update Installed", SPARKLE_TABLE, sparkleBundle, nil); - - // Extract information from newly updated bundle if available - NSString *hostName; - NSString *hostVersion; - NSBundle *newBundle = [NSBundle bundleWithURL:_oldHostBundleURL]; - if (newBundle != nil) { - SUHost *newHost = [[SUHost alloc] initWithBundle:newBundle]; - hostName = newHost.name; - hostVersion = newHost.displayVersion; - } else { - // This may happen if Sparkle's normalization is enabled - hostName = _oldHostName; - hostVersion = nil; - } + NSAlert *alert = [[NSAlert alloc] init]; + alert.messageText = SULocalizedStringFromTableInBundle(@"Update Installed", SPARKLE_TABLE, sparkleBundle, nil); - if (hostVersion != nil) { - alert.informativeText = [NSString stringWithFormat:SULocalizedStringFromTableInBundle(@"%@ is now updated to version %@!", SPARKLE_TABLE, sparkleBundle, nil), hostName, hostVersion]; - } else { - alert.informativeText = [NSString stringWithFormat:SULocalizedStringFromTableInBundle(@"%@ is now updated!", SPARKLE_TABLE, sparkleBundle, nil), hostName]; - } - [self showAlert:alert secondaryAction:nil]; + // Extract information from newly updated bundle if available + NSString *hostName; + NSString *hostVersion; + NSBundle *newBundle = [NSBundle bundleWithURL:_oldHostBundleURL]; + if (newBundle != nil) { + SUHost *newHost = [[SUHost alloc] initWithBundle:newBundle]; + hostName = newHost.name; + hostVersion = newHost.displayVersion; + } else { + // This may happen if Sparkle's normalization is enabled + hostName = _oldHostName; + hostVersion = nil; } - acknowledgement(); + if (hostVersion != nil) { + alert.informativeText = [NSString stringWithFormat:SULocalizedStringFromTableInBundle(@"%@ is now updated to version %@!", SPARKLE_TABLE, sparkleBundle, nil), hostName, hostVersion]; + } else { + alert.informativeText = [NSString stringWithFormat:SULocalizedStringFromTableInBundle(@"%@ is now updated!", SPARKLE_TABLE, sparkleBundle, nil), hostName]; + } + [self showAlert:alert secondaryAction:nil]; } #pragma mark Aborting Everything @@ -945,7 +1054,7 @@ - (void)dismissUpdateInstallation _cancellation = nil; _retryTerminatingApplication = nil; - [self closeCheckingWindow]; + [self closeCheckingWindow:NO]; if (_permissionPrompt) { [_permissionPrompt close]; diff --git a/Sparkle/SUStatusController.h b/Sparkle/SUStatusController.h index 82b6b8c0b..38d44cf8c 100644 --- a/Sparkle/SUStatusController.h +++ b/Sparkle/SUStatusController.h @@ -32,6 +32,28 @@ // If isDefault is YES, the button's key equivalent will be \r. - (void)setButtonTitle:(NSString *)buttonTitle target:(id)target action:(SEL)action isDefault:(BOOL)isDefault accessibilityIdentifier:(NSString *)accessibilityIdentifier SPU_OBJC_DIRECT; +// -showWindow: is internally buffered so that the window appears only after a short +// delay so a fast-completing operation never causes a flicker. If -close is +// called before the delay elapses, the window never appears at all. Calling +// -showWindow: on an already-visible window brings it to front normally. +- (void)showWindow:(id)sender; + +// Close the window, observing an internal minimum display time. completion +// runs once the window has actually closed — immediately if the window isn't +// on screen (or has been visible long enough), or after the remaining time +// otherwise. The userCancelled flag is YES if the close was expedited by +// -closeImmediately (i.e., the user explicitly cancelled); NO if the +// minimum-display timer elapsed normally. +- (void)closeWithCompletionBlock:(void (^)(BOOL userCancelled))completion SPU_OBJC_DIRECT; + +// Close the window now. If a deferred close scheduled by +// -closeWithCompletionBlock: is still waiting out the minimum-display time, +// fire its completion now with userCancelled=YES. Returns YES if a pending +// completion fired (so the caller knows the completion's cleanup ran); +// returns NO if there was no pending completion (caller is responsible for +// any state cleanup that would have happened in the completion). +- (BOOL)closeImmediately SPU_OBJC_DIRECT; + @end #endif diff --git a/Sparkle/SUStatusController.m b/Sparkle/SUStatusController.m index b6835a5cf..b2499dfa6 100644 --- a/Sparkle/SUStatusController.m +++ b/Sparkle/SUStatusController.m @@ -16,6 +16,13 @@ static NSString *const SUStatusControllerTouchBarIdentifier = @"" SPARKLE_BUNDLE_IDENTIFIER ".SUStatusController"; +// Buffering parameters for -showWindow: and -closeWithCompletionBlock:. We hide +// the window entirely if a close arrives within SUStatusDisplayDelay, +// and we enforce a minimum visible duration of SUStatusMinimumDisplayTime +// once the window has actually appeared on screen. +static const NSTimeInterval SUStatusDisplayDelay = 0.3; +static const NSTimeInterval SUStatusMinimumDisplayTime = 0.7; + @interface SUStatusController () // These properties are used for bindings @@ -36,6 +43,10 @@ @implementation SUStatusController IBOutlet NSButton *_actionButton; IBOutlet NSTextField *_statusTextField; IBOutlet NSProgressIndicator *_progressBar; + + BOOL _waitingToShowWindow; + NSTimeInterval _windowShownTime; + void (^_pendingCloseCompletion)(BOOL userCancelled); BOOL _minimizable; BOOL _closable; @@ -153,6 +164,102 @@ - (BOOL)isButtonEnabled return [_actionButton isEnabled]; } +- (void)showWindow:(id)sender +{ + // If the window is already visible just call through for default handling + if (self.window.visible) { + [super showWindow:sender]; + return; + } + + // Already scheduled - just keep waiting + if (_waitingToShowWindow) { + return; + } + + _waitingToShowWindow = YES; + + __weak __typeof__(self) weakSelf = self; + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(SUStatusDisplayDelay * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + __typeof__(self) strongSelf = weakSelf; + if (strongSelf != nil && strongSelf->_waitingToShowWindow) { + [strongSelf _reallyShowWindow:sender]; + } + }); +} + +- (void)_reallyShowWindow:(id)sender SPU_OBJC_DIRECT +{ + _waitingToShowWindow = NO; + _windowShownTime = [NSDate timeIntervalSinceReferenceDate]; + [super showWindow:sender]; +} + +- (void)closeWithCompletionBlock:(void (^)(BOOL userCancelled))completion +{ + // If the window isn't on screen, close silently and complete immediately. + if (_waitingToShowWindow || !self.window.visible) { + [self close]; + if (completion != nil) { + completion(NO); + } + return; + } + + NSTimeInterval elapsed = [NSDate timeIntervalSinceReferenceDate] - _windowShownTime; + if (elapsed >= SUStatusMinimumDisplayTime) { + [self close]; + if (completion != nil) { + completion(NO); + } + return; + } + + // Replace any previously pending close. (Not expected, but safe.) + _pendingCloseCompletion = [completion copy]; + + NSTimeInterval remaining = SUStatusMinimumDisplayTime - elapsed; + __weak __typeof__(self) weakSelf = self; + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(remaining * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + __typeof__(self) strongSelf = weakSelf; + if (strongSelf == nil) { + return; + } + void (^pendingCompletion)(BOOL) = strongSelf->_pendingCloseCompletion; + if (pendingCompletion == nil) { + // -close was called externally; pending state was discarded. + return; + } + strongSelf->_pendingCloseCompletion = nil; + [strongSelf close]; + pendingCompletion(NO); + }); +} + +- (BOOL)closeImmediately +{ + void (^pending)(BOOL) = _pendingCloseCompletion; + _pendingCloseCompletion = nil; + [self close]; + if (pending == nil) { + return NO; + } + pending(YES); + return YES; +} + +- (void)close +{ + // -close is the abort path: silently discard any pending buffered + // presentation or deferred close. Callers driving the buffered API use + // -closeWithCompletionBlock: instead, which gates the + // window's actual disappearance behind the minimum display time. + _waitingToShowWindow = NO; + _windowShownTime = 0; + _pendingCloseCompletion = nil; + [super close]; +} + - (void)setMaxProgressValue:(double)value { if (value < 0.0) value = 0.0;