pharo
pharo copied to clipboard
pharo-project
15449 debug session must not refer to UI manager
Attempt to fix #15449
It removes the two UIProcess globals that existed in two distinct Morphic classes, and moves it in Process. A new api is proposed in Process to access that global instead of direct access by Morphic.
This is a strange design change. You moved out the variable from the scope of its users:
- it is now defined in Kernel Process
- but all accessors/management still happens under morphic classes.
It is a kind of broken encapsulation. Previosly the global and its users were in one place. Now it is spread over the system
I see the duplication exists in recently copied class MorphicCoreUIManager. But it seems it is not used anywhere. Why it was introduced this way (copy paste) that is the issue.
This is a strange design change. You moved out the variable from the scope of its users:
* it is now defined in Kernel Process * but all accessors/management still happens under morphic classes.It is a kind of broken encapsulation. Previosly the global and its users were in one place. Now it is spread over the system
It is just a start, the goal is to remove the dependency to the UIManager. This means that ultimately, every caller of accessors to the variable going through Morphic should go through Process instead.
This is just a first step, it's not that bad to start with that and see how it goes then continue the effort until we can completely remove the UIManager (which is already planned I think).
This is a strange design change. You moved out the variable from the scope of its users:
* it is now defined in Kernel Process * but all accessors/management still happens under morphic classes.It is a kind of broken encapsulation. Previosly the global and its users were in one place. Now it is spread over the system
It is just a start, the goal is to remove the dependency to the UIManager. This means that ultimately, every caller of accessors to the variable going through Morphic should go through Process instead.
This is just a first step, it's not that bad to start with that and see how it goes then continue the effort until we can completely remove the UIManager (which is already planned I think).
You added the knowledge about UI to the kernel class Process. It does not look like a right change. Instead we should hide from the users any knowledge about UIProcess. Things like "Process uiProcess == interruptedProcess" or #resumeUIProcess: should not be called from the outside of UIManager. And this is not the same as removing all references to UIManager singleton.
This is a strange design change. You moved out the variable from the scope of its users:
* it is now defined in Kernel Process * but all accessors/management still happens under morphic classes.It is a kind of broken encapsulation. Previosly the global and its users were in one place. Now it is spread over the system
It is just a start, the goal is to remove the dependency to the UIManager. This means that ultimately, every caller of accessors to the variable going through Morphic should go through Process instead. This is just a first step, it's not that bad to start with that and see how it goes then continue the effort until we can completely remove the UIManager (which is already planned I think).
You added the knowledge about UI to the kernel class Process. It does not look like a right change. Instead we should hide from the users any knowledge about UIProcess. Things like "Process uiProcess == interruptedProcess" or #resumeUIProcess: should not be called from the outside of UIManager. And this is not the same as removing all references to UIManager singleton.
I agree with all that you said, I just do not see simple solutions that we can implement now. The immediate problem is that the uimanager will be removed (not its references, the manager itself). It's already super ugly. This changes does not make it more ulgy, but it gives time to think architectural changes that are more complex.
I'm open to ideas..
On the problem of exposure to ui knowledge: we could hide it by allowing Process to propose a cache of "special processes" that its clients would use. However, if there is no UI manager, clients would require to specifically ask for "ui stuff" to the Process (or any other place where that knowledge would be).
Actually, I am not found either of the clients asking for ui stuff (i.e., should DebugSession objects ask about ui?)
One other aspect: the PR is also to see if such change can pass the test. I can add a "do not merge" tag.
This changes does not make it more ulgy, but it gives time to think architectural changes that are more complex.
I dont really see how the moving of Morphic party to Kernel will help with it. UIProcess should not exist outside of Morphic as it is a pure Morphic mechanizm. Nether Bloc or gtk based UIs have it (tell me if I am wrong).
So the real task is to refactor the debugger model to avoid this hardcoded errorWasInUIProcess logic. I imagine some kind of debugger pluggins where we can implement specific debugging procedures/hooks for given set of processes. Morphic will implement own MorphicUIProcessDebuggerPlugging with current logic.
Well I can investigate a Morphic extension of the debugger infrastructure, that would be removed the day we remove Morphic.
I am not convinced it would entirely solve the "UImanager problem" since it is also used to day to tell that there is no ui (which also matters in the case of bloc).
Ok there are no satisfying ways of doing this with what we have, tools, constraints, architecture... I do not see how it can work. We need something new.
Can we not have an announcement launched every time we "change mode"?
For example, if we start in headless, the image announces a "HeadlessMode" event and every tool who should take care of that just get the announcement and downgrade their capabilities?
And same if we switch to GUI mode (that may be by default actually).
Ok there are no satisfying ways of doing this with what we have, tools, constraints, architecture... I do not see how it can work. We need something new.
Can we not have an announcement launched every time we "change mode"?
For example, if we start in headless, the image announces a "HeadlessMode" event and every tool who should take care of that just get the announcement and downgrade their capabilities?
And same if we switch to GUI mode (that may be by default actually).
This comment concerns all the similar issues with the debugger, that needs to know if it can open or not. For this issue we also need to make it morphic specific, and generalize it so that we can give all backend an API to tell if the debugging their ui threads has to be handled in a specific way or not.
So basically:
- When we debug a morphic-based GUI, we need to know if it's the gui process
- When we're not in a morphic-based GUI, we might or not need to know if we debug the gui process
Considering that requesting a debugger does not depend on the gui system, and can even happen in a headless system, the question is:
- if not a UIManager, to what entity should we ask that question?
It cannot be deffered on a debugger, because debuggers do not care about the underlying gui (in the spec logic). It could be deffered on the debugger application, but then the application should know the backnd (and then we can ask the backend what to do).
OK I see what you are saying is that we need to know information about the UI Process. Let us talk with G and P.
I discussed with Esteban and actually applications know the backend already. So it should be possible to work something nice around.