Extracts the process handling out of the splash controller #233
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Like GUI ones, notifying when the process exit.
Leaving only the window management.
e2646d5
to
baa922a
Compare
EduardGomezEscandell
requested changes
Jul 21, 2022
DistroLauncher-Tests/DistroLauncher-Tests/ChildProcessTests.cpp
Outdated
Show resolved
Hide resolved
DistroLauncher-Tests/DistroLauncher-Tests/DistroLauncher-Tests.vcxproj
Outdated
Show resolved
Hide resolved
|
Requesting changes only for the typo 😄. The others are more debatable. |
Removed the using declaration in the namespace scope on ChildProcess. SplashController now accepts the ProcessAPI type parameter.
EduardGomezEscandell
approved these changes
Jul 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The splash controller is made of a state machine and a strategy class full of static functions to encapsulate the Win32 API's with the duty of starting and controlling the slide show application. There are a few premises related to its job:
It's quite easy to predict that the behavior of that class could be a nightmare of if-elses. That's why the state machine design.
Yet, the strategy class that feeds the controller with the Win32 capabilities is doing too much: controlling the application process and the application window. When porting the OOBE to Windows, most of the process handling behavior would be duplicated, but the window control would not, thus splitting those duties in two different components allows for an easier migration to the new OOBE while not breaking the current behavior (required for the upcoming point release).
So, this PR extracts the process creation, termination and spurious termination notification out of the splash controller component. Also, the splash controller is updated to reflect the changes in such a way that user won't notice any difference in the application behavior.