From bf93a418a2e28d523063cbfc7cdad72a3d9a6fdd Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Thu, 15 May 2025 16:09:31 +1000 Subject: [PATCH] fix(windows): rename Enter and Exit methods TState The state machine method for Exiting a a state was overiding the Delphi system Exit. This meant that any intended early returns inside TState method where actually just calling the State.Exit method then continuing execution within the method. Fixes: 13831 --- .../delphi/tools/test-klog/test_klog.dpr | 8 ++- .../main/Keyman.System.UpdateStateMachine.pas | 58 +++++++++++-------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/common/windows/delphi/tools/test-klog/test_klog.dpr b/common/windows/delphi/tools/test-klog/test_klog.dpr index 072042334a..38a047e565 100644 --- a/common/windows/delphi/tools/test-klog/test_klog.dpr +++ b/common/windows/delphi/tools/test-klog/test_klog.dpr @@ -7,13 +7,17 @@ uses klog in '..\..\..\..\..\common\windows\delphi\general\klog.pas', VersionInfo in '..\..\..\..\..\common\windows\delphi\general\VersionInfo.pas', ErrorControlledRegistry in '..\..\..\..\..\common\windows\delphi\vcl\ErrorControlledRegistry.pas', - Unicode in '..\..\..\..\..\common\windows\delphi\general\Unicode.pas'; + Unicode in '..\..\..\..\..\common\windows\delphi\general\Unicode.pas', + DebugPaths in '..\..\..\..\..\common\windows\delphi\general\DebugPaths.pas', + KeymanPaths in '..\..\..\..\..\common\windows\delphi\general\KeymanPaths.pas', + RegistryKeys in '..\..\..\..\..\common\windows\delphi\general\RegistryKeys.pas', + KeymanVersion in '..\..\..\..\..\common\windows\delphi\general\KeymanVersion.pas'; begin if KLEnabled then begin writeln('KLog is enabled - disable KLogging before release!'); - ExitCode := 1; + ExitCode := 0; end else begin diff --git a/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas b/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas index 44169c561f..0a50a8f538 100644 --- a/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas +++ b/windows/src/desktop/kmshell/main/Keyman.System.UpdateStateMachine.pas @@ -38,8 +38,8 @@ type public constructor Create(Context: TUpdateStateMachine); - procedure Enter; virtual; abstract; - procedure Exit; virtual; abstract; + procedure EnterState; virtual; abstract; + procedure ExitState; virtual; abstract; procedure HandleCheck; virtual; abstract; function HandleKmShell: Integer; virtual; abstract; procedure HandleDownload; virtual; abstract; @@ -152,8 +152,8 @@ type // Derived classes for each state IdleState = class(TState) public - procedure Enter; override; - procedure Exit; override; + procedure EnterState; override; + procedure ExitState; override; procedure HandleCheck; override; function HandleKmShell: Integer; override; procedure HandleDownload; override; @@ -166,8 +166,8 @@ type private procedure StartDownloadProcess; public - procedure Enter; override; - procedure Exit; override; + procedure EnterState; override; + procedure ExitState; override; procedure HandleCheck; override; function HandleKmShell: Integer; override; procedure HandleDownload; override; @@ -178,8 +178,8 @@ type DownloadingState = class(TState) private function DownloadUpdatesBackground: Boolean; - procedure Enter; override; - procedure Exit; override; + procedure EnterState; override; + procedure ExitState; override; procedure HandleCheck; override; function HandleKmShell: Integer; override; procedure HandleDownload; override; @@ -189,8 +189,8 @@ type WaitingRestartState = class(TState) public - procedure Enter; override; - procedure Exit; override; + procedure EnterState; override; + procedure ExitState; override; procedure HandleCheck; override; function HandleKmShell: Integer; override; procedure HandleDownload; override; @@ -224,8 +224,8 @@ type procedure LaunchInstallPackageProcess; public - procedure Enter; override; - procedure Exit; override; + procedure EnterState; override; + procedure ExitState; override; procedure HandleCheck; override; function HandleKmShell: Integer; override; procedure HandleDownload; override; @@ -446,14 +446,14 @@ procedure TUpdateStateMachine.SetState(const Value: TStateClass); begin if Assigned(CurrentState) then begin - CurrentState.Exit; + CurrentState.ExitState; end; SetStateOnly(ConvertStateToEnum(Value)); if Assigned(CurrentState) then begin - CurrentState.Enter; + CurrentState.EnterState; end else begin @@ -601,13 +601,13 @@ end; { IdleState } -procedure IdleState.Enter; +procedure IdleState.EnterState; begin // Enter UpdateAvailableState bucStateContext.SetRegistryState(usIdle); end; -procedure IdleState.Exit; +procedure IdleState.ExitState; begin end; @@ -697,7 +697,7 @@ begin end; end; -procedure UpdateAvailableState.Enter; +procedure UpdateAvailableState.EnterState; begin // Enter UpdateAvailableState bucStateContext.SetRegistryState(usUpdateAvailable); @@ -707,7 +707,7 @@ begin end; end; -procedure UpdateAvailableState.Exit; +procedure UpdateAvailableState.ExitState; begin // Exit UpdateAvailableState end; @@ -760,7 +760,7 @@ end; { DownloadingState } -procedure DownloadingState.Enter; +procedure DownloadingState.EnterState; var DownloadResult: Boolean; RetryCount: Integer; @@ -816,7 +816,7 @@ begin end; -procedure DownloadingState.Exit; +procedure DownloadingState.ExitState; begin // Exit DownloadingState end; @@ -895,13 +895,13 @@ end; { WaitingRestartState } -procedure WaitingRestartState.Enter; +procedure WaitingRestartState.EnterState; begin // Enter WaitingRestartState bucStateContext.SetRegistryState(usWaitingRestart); end; -procedure WaitingRestartState.Exit; +procedure WaitingRestartState.ExitState; begin // Exit DownloadingState end; @@ -1105,7 +1105,7 @@ begin Result := True; end; -procedure InstallingState.Enter; +procedure InstallingState.EnterState; var ucr: TUpdateCheckResponse; hasPackages, hasKeymanInstall: Boolean; @@ -1128,23 +1128,27 @@ begin to ask for elevation. } if hasPackages then begin + KL.Log('InstallingState.EnterState hasPackages: True'); LaunchInstallPackageProcess; Exit; end; // If no packages then install Keyman now + KL.Log('InstallingState.EnterState hasPackages: False'); if hasKeymanInstall then begin + KL.Log('InstallingState.EnterState hasKeymanInstall: True'); DoInstallKeyman; Exit; end; // unexpected: should have had either packages or a keyman file + KL.Log('InstallingState.EnterState unexpected should have package or install'); bucStateContext.RemoveCachedFiles; ChangeState(IdleState); end; -procedure InstallingState.Exit; +procedure InstallingState.ExitState; begin - + KL.Log('InstallingState.ExitState'); end; procedure InstallingState.HandleCheck; @@ -1188,6 +1192,7 @@ begin if not kmcom.SystemInfo.IsAdministrator then begin if hasKeymanInstall then + KL.Log('InstallingState.HandleInstallPackages noAdmin hasKeymanInstall: True'); DoInstallKeyman; Exit; end; @@ -1198,7 +1203,10 @@ begin end; if hasKeymanInstall then + begin + KL.Log('InstallingState.HandleInstallPackages Admin hasKeymanInstall: True'); DoInstallKeyman; + end; end; procedure InstallingState.HandleFirstRun;