terminal icon indicating copy to clipboard operation
terminal copied to clipboard

Scenario: Windows Terminal 2.0 Process Model Improvements

Open zadjii-msft opened this issue 4 years ago • 18 comments

This scenario is tracking a number of enhancements we'd like to make to the fundamental process model of the Windows Terminal in v2.0. The following deliverables all have something to do with the way the Terminal is launched, or the way connections or instances are managed, and as such, the solution to any one of these areas should make sure to take into account the others.

Window / Session Managment

  • [ ] #1256 Tab Tearoff/ Reattach
    • #2080 is a draft spec for tearoff and default terminal app (below)
    • Presumably this will involve some way to communicate different terminal instances from one window to another.
      • Proposed ideas include a manager process that's hosting all the terminal instances and connections, and then separate windows for actually drawing these instances on the screen
  • [x] Run wt in the current window
    • See #4472.
    • This is being included because it ties strongly with the above deliverable.
    • See #3495 / #607 - Commandline Arguments for the Windows Terminal.md, which also brought this idea up.
    • This involves finding some way for the user to specify on the commandline "I want to perform [some command] in [the current window | a specified other WT window | a new WT window]"
    • ✔️ Implemented in #8898
  • [x] Single Instance Mode
    • See #2227
    • This would be an important parallel work thread for #653 and #492
    • ✔️ Implemented in #9118
  • [x] #653 Quake-style dropdown
    • Again, included in this list for it's fundamental strong ties to session management.
    • Often mentioned in the discussion of quake-style dropdown is the need for a single-session mode, so pressing the global hotkey would always activate the single instance, and new windows would instead glom to the existing instance
    • See #8888 for follow-up tasks related to Quake mode

Elevation

  • [x] #1032 Mixed Elevation
    • This is a hard problem, and I'm making no firm commitments that we'll be able to solve it for sure in 2.0.
    • The goal of this deliverable is to find a safe way that we can support both elevated (High-IL) and unelevated (Medium-IL) tabs, panes, etc all in the same window.
    • Ideally, if a user creates a new elevated tab from an unelevated window, then all the unelevated tabs would seamlessly appear in a new elevated HWND hosting both unelevated and elevated tabs simultaneously.
    • Elevated connections can't be hosted in an unelevated HWND, because that allows for a trivial escalation-of-privilege vector utilizing the Terminal.
    • This problem should arguably be investigated first. If this is something that's technically possible, it will certainly have an influence on the way the rest of these deliverables are architected.
    • Resolution: This is not going to be technically achievable, unfortunately.
  • [x] #632 Open a Profile as elevated
    • If the above is not achievable technically, then the user should still be able to specify a way to force a profile to open elevated.
    • If mixed elevation isn't possible:
      • If the current window isn't currently elevated, open a new window when someone's profile is marked "elevated: true"
      • If the current window is elevated, open in the same window.

Default Terminal

  • [x] #492 Default Terminal Application
    • This one's a little different than the others. While the rest were Windows Terminal specific features, this deliverable is more of a OS feature than a Terminal feature.
    • This concerns the ability for an arbitrary application to be registered with the OS as the "Default Terminal Application", and be started automatically when the user starts an arbitrary commandline application.
    • This also has some interaction with the above deliverables. When a commandline application is started this way, should they open as a new tab in an existing Terminal window, or start a new window?

Specs

  • Spec for Windows Terminal Process Model 2.0 #7240
  • Spec for Windows Terminal Window Management #8135
  • Spec for Elevation QOL improvements #8455
  • Default Terminal Spec #7414
  • Quake mode spec #9274

TODOs

These are also tracked in https://github.com/microsoft/terminal/projects/5. This provides a nested list, to mentally track which things are dependent upon other things. This is basically a copy of a page of my notebook.

Window Management
  • [x] Window Management, PR: #8898
    • [ ] Next/Prev window keybindings
    • [x] Windowing Behavior, PR: #9118
      • [ ] Smart wt -w 0 handling
      • [x] Name windows on commandline
        • [x] nameWindow action (#9662)
        • [x] openWindowRenamer action (#9662)
        • [x] window-level toast for displaying window ID, action: idenfifyWindows (#9523)
          • Originally I thought I'd need generic toasts for this, but I'm gonna just do this first, and come back to generic toasts
  • [x] Quake Mode has been moved to #8888
  • [x] New Window Action PR: #9208

Tear Out

Currently planned state, notes:

  • [ ] wt.exe accepts --content guid on the commandline to start in content mode. Additionally, Make the Control able to have the Core OOP.
    • these two were originally different bullet points, but I think they need to be combined into one singular PR. They don't make sense alone.
    • [x] in content process mode, it registers ContentProcess for that GUID with CoRegisterClassObject
    • [x] ContentProcess::Initialize(IControlSettings, IConnection) instantiates a new ControlInteractivity (in the content process) (if one hasn't already been initialized)
    • [x] ContentProcess::RequestSwapChainHandle(pid) will duplicate the handle if the pid is different
    • [x] wt properly tracks the lifetime of its content. When that's discarded by the last window process, we'll exit.
    • [x] Do the above line, but better. Now we're using a lock and an event, which is ew. Instead, we should do the Conhost ExitThread() thing on the main thread, and then ExitProcess() when the content process is dtor'd.
    • [x] Move all the chaos I've introduced in main.cpp into its own dedicated file/class or something.
    • [x] An embedding process needs a way to know that the content process is ready, better than a Sleep(2000)
      • A handle to an event is probably the cleanest way - have the window create the event, then pass the handle in CreateProcess & on the commandline to wt --content {guid} --signal {handle}, then have the content process set the event when it's ready.
    • 📝 in ~dev/migrie/oop/the-whole-thing~ dev/migrie/oop/infinity-war
    • [x] The Connection is made in the core
    • [x] When the Core signals that the swapchain changed, and the TermControl asks what the swapchain handle is, the Control passes in it's PID. If the PID isn't the same, DuplicateHandle the swapchain to that PID.
    • [x] The sample is updated with a version of the Control that's also OOP
    • [x] If the content process dies, the control needs to be able to display a message box.
    • [x] Make sure UIA signaling still works (accevent.exe in the SDK, C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64)
    • [x] Make sure that reading the contents of the buffer with Accessibility Insights still works
      • 💩
      • InteractivityAutomationPeer extends Windows.UI.Xaml.Automation.Peers.AutomationPeer. But we're making the interactivity peer in the content process, not in the control layer.
      • When we construct the IAP, we throw instantly, because the WUX..AutomationPeer can't be created off the main thread.
      • How do we only expose the WUX..AutomationPeer via TermControl, but not IAP?
    • [x] The _raiseContentDied should display the error internal to the TermControl. Kinda like the renderer warning, but for this, we need to kill the SwapChainPanel too.
    • in ~dev/migrie/oop/the-whole-thing~ dev/migrie/oop/infinity-war
    • Relevant diff: 23a19c58181b489b10a2049232f23b033dbdab8d...5c17603a94a76d47c4427f49493bca4eeb8fa774
    • [x] Add a velocity flag to gate behind dev builds only. Probably safe to just gate the --content flag for now.
    • [ ] Guard calls to _interactivity, _core in TermControl. Those objects might go out of scope at any moment, so pretty much all access needs to be try/caught.
      • we may want to do a function-level try, with a CATCH_RAISE_DIALOG() macro we write to raise the connection died notice. Might be the most minimal diff.
      • Could likely be a follow up to original PR.
      • There's a silly idea in dev/migrie/oop/2/cptn-marvel. I really hate it, it's a HUGE amount of churn.
    • [x] There's a lot of debug spew that makes me think the OOP core does something off the main thread.
      • BriAl and I synced - this is nothing to worry about. This is basically just COM logging "i'm about to increment the refcount"
      • Still trying to find a viable way to suppress that logging.
    • [x] What the hell did I do with the automation peer. Was what was in main correct? Revisit the original PRs for inspiration.
      • Did it ever work? there's seemingly no record of it working in 23a19c58181b489b10a2049232f23b033dbdab8d...5c17603a94a76d47c4427f49493bca4eeb8fa774
      • #10051 has a fairly different implementation than the above diff
      • I may have simply forgotten that narrator / UIA uses exceptions for control flow, which makes debugging a pain
    • [x] ControlCore::_IsClosing TODO!
    • [x] ContentProcess::RequestSwapChainHandle tracelogging TODO!
    • [x] TermControlAutomationPeer::GetLocalizedControlTypeCore that string used to come from localized resources, it can't anymore.
    • [x] The Control properly manages the lifetime of the HANDLE duplicated to it. A wil::unique_handle?
    • [x] IMPORTANT: For whatever reason, when the second window attaches to the content, it renders basically nothing. Input gets sent just fine, but the output doesn't render.. Fixed in 7b3ca8332
    • 📝 Now moved to dev/migrie/oop/2/infinity-war
    • 👀 in #12938
  • [x] TermControls need to have a local TerminalSettings instance. (Related: #7219)
    • What a shockingly simple oversight. We did all this work to get the connections in the same proc as the TermControl, we totally forgot the Settings.
    • [x] Make sure updating/changing settings works fine across processes
    • [x] Make sure previewing the color scheme still works. That works with some parent/child trickery that won't work OOP.
    • 📝 in dev/migrie/oop/ragnarok
  • [ ] Some misc cleanup before endgame:
    • [ ] existingConnection in NewTab was redundant / unused
    • [ ] _SplitPane renamed to _SplitPaneActiveTab etc.
    • [ ] Refactoring to allow the TerminalTab to be initialized without a control
    • [ ] Other stuff to, to minimize the diff
    • not started, in dev/migrie/oop/2/black-widow cause this one's not really consequential, is it now.
  • [ ] Make the app able to do the OOP instantiation.
    • Notes in https://github.com/microsoft/terminal/issues/5000#issuecomment-1137653858
  • [x] Make the sample able to pop a control into a new window.
    • ✔️ in dev/migrie/oop/the-whole-thing. I didn't pop it into a new window, just made it so multiple windows could connect to the same one.
  • [ ] Defterm is out of proc.
    • [ ] DEFTERM: Incoming connections need to be tossed to the ContentProcess. Guh.
    • not started in dev/migrie/oop/2/captain-falcon.
  • [x] Tabs can be serialized to a json blob, with their tab & pane structure.
    • See #766, because this is highly relevant
    • This may have already been done for me by #10972
    • [x] Serialize TerminalTabs
    • [x] Serialize SettingsTabs
  • [ ] move-pane -w <id> to move a pane/tab to another Terminal window. This will prove we can move panes without the whole instantiation of a new window process.
    • Notes in https://github.com/microsoft/terminal/issues/5000#issuecomment-1183694124
    • 📝 in dev/migrie/oop/wandavision (link to relevant diff from dev/migrie/oop/endgame)
  • [ ] When a tab is torn out of a Terminal window, it can be dropped & re-attached onto another terminal window
    • Notes in https://github.com/microsoft/terminal/issues/5000#issuecomment-1185447944
    • 📝 in dev/migrie/oop/2/loki
  • [ ] When a tab is torn out of a Terminal window, it can be dropped somewhere else and turned into a new terminal window
    • not started yet, in dev/migrie/oop/no-way-home
  • [ ] When a tab is torn out, we'll create a new window for the tabs left behind, and move the current window around.
    • We'll need to still be able to drop this window on an existing window somehow.
    • Use EvanK's prototype here. We're gonna need to extend the drag bar over the TabView and do everything ourselves
    • [ ] #12616
    • not started yet, in dev/migrie/oop/multiverse-of-madness

other notes

Just moving these lower in the body to make managing this doc easier

This is a pseudo design that Dustin and I discussed. There's a lot more work that needs to be done here, but I need to save this somewhere outside of a Teams chat history
---
struct TerminalControl: winrt::implements<MUX::IUIElement> {} // XAML Control
// calls: interactivity.PointerMoved(Point{x, y});
// calls: interactivity.PointerClicked(Point{x, y});
// links: TerminalCore.lib <----> or consumes TerminalCore.dll from RT
^^^ TerminalControl.dll ^^^

---
struct WPFTerminalControl { HWND _h; }
// calls: interactivity.PointerMoved(Point{x, y});
// calls: interactivity.PointerClicked(Point{x, y});
// links: TerminalCore.lib
^^^ WPFTerminalControl.dll ^^^

vvv TerminalCore.lib vvv // Cancelled. TerminalCore.lib or .dll
struct Coord {};
struct PointerInfo {};
struct KeyInfo {};
struct TerminalControlInteractivity;
struct ControlCore; // < implements ... something? // this should just be TerminalCore?
struct TerminalCore; // < implements the "conhosty APIs (ITerminalDispatch, etc.)
// links: dx.lib buffer.lib types.lib

vvv TerminalCore.UnitTests.dll vvv
// contains: interactivity tests
// contains: core tests
// links: TerminalCore.lib
  • [x] Split TerminalControl into lib and dll
  • [x] Split TermControl into Control.TermControl and Control.ControlCore and Control.Interactivity
  • [ ] Add some Core.ControlCore tests from things in #7001
  • [ ] Create a WinRT boundary on Core.Interactivity between it and TermControl
  • [ ] Enable Core.ControlCore to exist out-of-proc with XAML, or in-proc
    • This might involve creating a scratch project to consume an in-proc one and an out-of-proc one
  • [ ] the Core can create connections, or have them passed in. (so an OOP core can have a connection in it's own process)
  • [ ] Much more. This is just what's immediately on my radar.
_4-30-21_: I need to do the DX `HANDLE` thing before I can do the rest of the projection boundary. The projection boundary is in `dev/migrie/interactivity-projection-000`, which is almost done now.

dev/migrie/oop-scratch-4 has the final state of the OOP prototyping on this element. In that branch,

  • HostAndProcess::CreateSwapChain creates a swapchain with DCompositionCreateSurfaceHandle.
  • It attaches it to the SwapChainPanel with ISwapChainPanelNative2::SetSwapChainHandle.
  • It duplicates that handle to the content process, and calls HostClass::ThisIsInsane
  • Later on, the HostClass passes that handle to the DxEngine

So we'll need to do basically that, but a little different, to handle all the actual edge cases we have.

pre-1.10 era work
  • [x] First, update DxEngine to accept the handle, but don't otherwise change anything (#10023)
    • ~~Then, change TermControl to create the handle with DCompositionCreateSurfaceHandle, hook it up to the SwapChainPanel with ISwapChainPanelNative2::SetSwapChainHandle.~~
      • DxEngine::_CreateDeviceResources will need to trigger a callback in the Core, that will then ask the TermControl to create a new swapchain for it.
      • Wait no that's wrong. the DxEngine will always be making the swapchains, because it knows when it needs a new one.
    • [x] DxEngine will always make new swapchains, when needed, with DCompositionCreateSurfaceHandle. It'll raise an event when that happens. The Core will handle that, then raise an event the control will listen to. The Control will handle that event, then ask the Core for the new swapchain HANDLE, as a uint64_t. The Control will use that uint64_t to attach to the SwapChainPanel via ISwapChainPanelNative2::SetSwapChainHandle.
      • This is also the way it is in (#10023)
    • 👀 in #10023
  • [x] Then we'll come back to the rest of the projection
    • 👀 in #10051
  • [x] At this point, we should make a sample app that's just a grid with two spaces in it. One for an in-proc term control, and one for an out of proc one. We won't do the OOP one yet.
    • 👀 in #10067
  • [x] Merge #10115 into #10067
  • [x] Enable the Connection able to be created OOP from the rest of the TerminalPage. The content process will be the one to instantiate the connection.
    • [x] The Core is passed a (guid? String?) of a type of connection to spawn, and an IConnectionSettings object. The Core instantiates a type of connection based on that guid/string, and passes the IConnectionSettings from the window process to that new constructor.
    • [x] How does the azure connection work? I think that one's implemented in TerminalApp. If it's a String, we could pass the winrt name, ITerminalConnection conn = RoCreateInstance("TerminalApp.AzureConnection"); conn.Create(iConnectionSettings); , right?
      • No, turns out the Azure connection is in TerminalConnection. So this was actually fine the whole time.
      • Whatever, we're keeping the RoGetActivationFactory thing though, for future needs.
    • [ ] How does the debug tap work?
    • ~~Switch from RoActivateInstance&Initialize to RoGetActivationFactory~~
      • Even neglecting the Windows Runtime, this isn’t even possible in C++ or C#.

    • [x] Reattach event handlers that I disabled for performance's sake
    • [x] Merge #10115 into this branch, or into dev/migrie/oop/connection-factory-rebase, which is attempting to rebase these changes onto dev/migrie/oop/sample-project
    • 📝 in dev/migrie/oop/connection-factory
  • [x] 🦶 PUNTED ~Move the Core, the Interactivity, into Core.dll. We'll need that for:~
    • We may not actually need this. It'd be nice to not have to load all the settings, but we could live with it.

zadjii-msft avatar Mar 18 '20 21:03 zadjii-msft

Elevation

  • [x] #1032 Mixed Elevation

    • This is a hard problem, and I'm making no firm commitments that we'll be able to solve it for sure in 2.0.
    • The goal of this deliverable is to find a safe way that we can support both elevated (High-IL) and unelevated (Medium-IL) tabs, panes, etc all in the same window.
    • Ideally, if a user creates a new elevated tab from an unelevated window, then all the unelevated tabs would seamlessly appear in a new elevated HWND hosting both unelevated and elevated tabs simultaneously.
    • Elevated connections can't be hosted in an unelevated HWND, because that allows for a trivial escalation-of-privilege vector utilizing the Terminal.
    • This problem should arguably be investigated first. If this is something that's technically possible, it will certainly have an influence on the way the rest of these deliverables are architected.
    • Resolution: This is not going to be technically achievable, unfortunately.
  • [ ] #632 Open a Profile as elevated

    • If the above is not achievable technically, then the user should still be able to specify a way to force a profile to open elevated.

    • If mixed elevation isn't possible:

      • If the current window isn't currently elevated, open a new window when someone's profile is marked "elevated: true"
      • If the current window is elevated, open in the same window.

What does this mean re. https://github.com/microsoft/terminal/issues/6893#issuecomment-657554241 and https://github.com/microsoft/terminal/issues/3581#issuecomment-581624314 etc.?

Will each window belong to a different process? Otherwise I don't understand what does "elevated window" even means.

And what if "elevation" actually mean "OTS elevation" rather than "AAM elevation", since the non-elevated user is not an admin? Currently at least that works. I hope the new model won't somehow break that.

@DHowett-MSFT @miniksa

conioh avatar Feb 11 '21 22:02 conioh

Currently, yes, each terminal window is its own process. Each WT process has one window, so when you're running WT elevated, the window itself is elevated. Unfortunately, nothing in the new process model is going to have an affect on elevated drag drop. The team that owns the platform drag/drop APIs has specifically told us that drag/drop is unsupported in an elevated HWND.

Furthermore, we're not going to be messing with mixed elevation as a part of this effort anymore, because that turned out to be technically less possible than we hoped. So nothing about that story is about to change.

zadjii-msft avatar Feb 11 '21 22:02 zadjii-msft

Maybe I'm missing something, but I just opened two elevated instances of WordPad, and dragged some text, a picture and an OLE Paint drawing from one to the other. And inside each document.

If by "platform" you mean UWP/Store/XAML/whatever-you-call-it-today, I just launched WinDbg Preview, installed from the Store, uses XAML and all that jazz, and managed to rearrange the windows in the elevated instance by dragging.

conioh avatar Feb 11 '21 23:02 conioh

Specifically: "UWP XAML" (not WPF, which WinDbg Preview uses) uses the modern WinRT drag/drop APIs. Those APIs require a data broker process. That broker just doesn't work from an elevated context. Terminal, by way of it using "UWP XAML", is subject to this specific limitation.

DHowett avatar Feb 11 '21 23:02 DHowett

About the Mixed Elevation part: Is there anything preventing possibility of a feature to launch medium integrity shells from high integrity wt instance? I know this is rather limited in comparison to the original plans, however should be much easier to support

namazso avatar Apr 11 '21 15:04 namazso

to launch medium integrity shells from high integrity wt instance

I would like to hear what others think about this approach. This can be achieved running gsudo -i medium on a wt tab or by creating a profile for it. There is also MediumPlus Integrity, which is higher than Medium (no medium process can tamper with it) but still hasn't admin acesss... (gsudo -i MediumPlus)

gerardog avatar Apr 11 '21 19:04 gerardog

Just as something that I saw mentioned as a comment on the spec, but not incorporated, I think the idea of having a separated elevated-settings.json file makes a lot of sense as a counterpart to elevanted-state.json. This gives a number of benefits

  • The file can be admin writeable only to help prevent malicious modification
  • You get for "free" admin specific settings modifications, including but not limited to custom styling, admin only profiles, and admin-specific startup actions.

Rosefield avatar Sep 01 '21 15:09 Rosefield

separated elevated-settings.json file makes a lot of sense as a counterpart to elevanted-state.json.

I don't think this is a terrible idea. How do we deal with users who are currently using their unelevated settings in admin windows though? Would users need to maintain basically two copies of their settings file?

Maybe we only use the elevated-settings file if we find it? If the file doesn't exist, then just fall back to the normal settings.

I guess it might make more sense if the elevated-settings could be layered on top of settings.json (a la #2303, #6502, #2933)

zadjii-msft avatar Sep 01 '21 15:09 zadjii-msft

I wasn't thinking in terms of migration as much as qol, but I did also consider layering as an approach or adding a "create from unelevated settings" option in the settings UI. I didn't think thoroughly about the security implications (neither are technically worse than what exists now, but it could be better) so I didn't want to suggest something.

Rosefield avatar Sep 01 '21 15:09 Rosefield

I'd say use settings.json as a base and override/merge values with anything in the elevated.json. Changed values or added sessions.

andrisi avatar Sep 01 '21 15:09 andrisi

i agree, layering / overriding is the best choice, in either direction. although checking if elevated-settings.json exists, then using that and overriding with user one might make more sense so that

The file can be admin writeable only to help prevent malicious modification

is not pointless

namazso avatar Sep 01 '21 15:09 namazso

<notes to self>

📝 dev/migrie/oop/2/endgame

📝 in dev/migrie/oop/2/endgame Relevant diff (probably): 5c17603a94a76d47c4427f49493bca4eeb8fa774...81a5a736fef41bd90339c36a6cc4d7a339b6badf

_MakePane usage:

  • [x] AppActionHandlers.cpp:207 - _HandleSplitPane
    • always creates a new split via _SplitPane (_SplitPaneActiveTab)
  • [x] TabManagement.cpp:88 - _OpenNewTab
    • always hits up _CreateNewTabFromPane
    • Actually, literally used in a _CreateNewTabFromPane call. Pane is not used afterwards.
  • [x] TabManagement.cpp:351 - _DuplicateTab
    • always hits up _CreateNewTabFromPane
    • DOES USE THE TAB afterwards. Should be possible to just init the TerminalTab, do the copying of the title to the new tab, then connect the content later.
  • [x] TabManagement.cpp:374 - _SplitTab
    • used in a call to _SplitPane (_SplitPaneOnTab)
    • This is used in the tab context menu for SplitTab. Can't just use an action, cause the tab context is lost then.
  • [x] TerminalPage.cpp:1032 - _OpenNewTerminalViaDropdown
    • Either creates a new tab (_CreateNewTabFromPane) or a new pane (_SplitPane, (_SplitPaneActiveTab)), based on alt key state.
    • ⭐ THIS ONE IS THE SPECIAL ONE
    • All the others IMMEDIATELY use the pane in _SplitPane or _CreateNewTabFromPane. This is the only one that uses it in post.
    • This could be two separate calls to AsyncSplitPane or AsyncCreateNewTabFromPane or something like that.
  • [ ] TerminalPage.cpp:3519 - _OnNewConnection
    • always hits up _CreateNewTabFromPane
    • This is the incoming defterm connection, which... needs a lot more work

2022-06-22

No this is dumb.
  • TerminalPage::_SplitPane and TerminalPage::_CreateNewTabFromPane both take a shared_ptr<Pane>, created from _MakePane.
  • Could they be trivially replaced by async versions?
    • NewTabFromPane
      • resume_bg
      • await MakePane().get()
        • create content process
        • resume_fg
        • create control from content
        • create pane
Make content is async. Creating the pane can be sync on the main thread * `IAsyncAction _CreateContentProcess(profile, controlSettings)` * `std::shared_ptr _MakePaneFromContent(ContentProcess content, auto controlSettings, auto profile)` * // MUST BE ON FG THREAD * create control from content * create pane * `_CreateNewTabFromPane` becomes: `winrt::fire_and_forget TerminalPage::_AsyncCreateNewTab(IAsyncAction initContentProc)` * `resume_bg` * `auto content = initContentProc.get()` * `resume_fg` * `auto pane = _MakePaneFromContent()` * `tab = make_self(pane)` * `_InitializeTab(newTabImpl)` * And this is called like: `_AsyncCreateNewTab(_CreateContentProcess(profile, controlSettings));` * `_SplitPaneOnTab` becomes: `winrt::fire_and_forget TerminalPage::_AsyncSplitPaneOnTab(TerminalTab& tab, const SplitDirection splitDirection, const float splitSize, IAsyncAction initContentProc)` * calculate split type * unzoom * resume BG * `auto content = initContentProc.get()` * `resume_fg` * `auto newPane = _MakePaneFromContent()` * `tab.SplitPane(*realSplitType, splitSize, newPane);` * focus the thing * `_SplitPaneActiveTab` becomes: * if (!focusedTab) ... * `_AsyncCreateNewTab` * else * `_AsyncSplitPaneOnTab(focusedTab,...)`

2022-06-23

  • eb4b2a753
    • We probably need to be able to make tabs without a pane, so we can put something on the screen before the content was connected.
  • 19605bf75
    • Changed so that the tab could be created without a pane. This should make things a bit more seamless, the tab will appear and then content will fill the tab when it's ready to appear
    • Defterm should be easy. Every defterm connection automatically becomes a new content process. Each defterm content process then needs to ask the monarch for a window to attach to. EZ.

TODO

  • [ ] How does the debugTap work for this? Connections live in their content process, so the debug tap would have to get created when the content process itself is connected
    • Could we always create debug taps for connections, then have a keybinding to activate them? Or would that add a terrible lot of overhead. Probably. But a neat wackadoodle idea.
  • [x] Make sure pane splitting still works
  • [ ] DEFTERM: Can we just add the defterm tab in-proc, alongside other oop tabs?
  • [x] Add a setting to disable this functionality.
    • [x] Works for new tabs via dropdown
    • [x] new splits
    • [x] new tabs via action
    • [x] ‼️ startup actions / state restore
  • [x] Add velocity, disabled-by-default, enabled for dev builds.
  • [x] Is there any way to get the tab opened faster, while we wait for the content to actually start? Even if we're going to just fake it?
  • [ ] Add a dummy BG to the tab, while we wait for the content to load
  • [x] Make sure moving panes between tabs still works
  • [x] init the content proc with the focused/unfocused appearance pair correctly
  • [x] There's a deadlock in conpty when exiting the Terminal as of 2ab0a50a1. is this the Terminal not releasing the connection cleanly? I'd reckon. There's no windowsterminal.exe processes left in VS when this happens. They're both locked on RundownAndExit
    • [I think this is fixed as of 83649075a71fd3ffb82cad578204fd8eae95d91c, dea94d51ade3f8eeece0d69834b875fe8b9fb30f and 089c0153470f89ece94a0bff13fed24147887e48, but I need to validate.
  • [ ] Make sure the app doesn't superexplode when the content process just dies.
    • [ ] Kill the content proc, mouse over the pane. TermControl explodes in PointerOver, cause the settings are gone. Deal with that.
      • Should we just set Closing when the content dies?
      • Should we add another check?
  • [ ] Make sure the content doesn't superexplode when the window process just dies.
  • [ ] Make sure the app doesn't leak a content process when the content process exits.
  • [ ] Make sure the app doesn't leak a content process when the tab closes (from UI).
  • [ ] hot-reloading the settings crashes the window proc. I'm guessing the call to _core.UpdateSettings on the UI thread when OOP is what gets us.

zadjii-msft avatar May 25 '22 18:05 zadjii-msft

<notes to self>

📝 dev/migrie/oop/2/wandavision

move-pane -w <id> to move a pane/tab to another Terminal window. This will prove we can move panes without the whole instantiation of a new window process. original diff at: in dev/migrie/oop/wandavision (link to relevant diff from dev/migrie/oop/endgame) diff from endgame: https://github.com/microsoft/terminal/compare/dev/migrie/oop/2/endgame...dev/migrie/oop/2/wandavision

Original prototype:

  • TerminalPage::_MovePane raises an event up, with the content guid of the current pane and a string describing the target window
  • That's bubbled through WindowManager to Monarch, to ask the monarch to deal with this.
  • Monarch tells the target peasant to AttachPaneToWindow, with the target tab index and content guid
  • Peasant then raises an event to AppHost to get it to tell the TerminalPage to TerminalPage::AttachPane.

TODO

  • [x] RequestMoveContent(String window, String contentJson, Int tabIndex).
    • [x] BuildStartupActions in Pane and in TabBase accept a const bool asContent = false parameter, to serialize with a content guid
      • [x] Pane
      • [x] TerminalTab
      • [x] SettingsTab
    • [x] We'll need a (TerminalPage) local helper for serializing a std::vector<ActionAndArgs> to a json blob.
      • We have one for a IVector, presumably? Looks like in ApplicationState.h, we just use SetValueForKey(json, TabLayoutKey, val.TabLayout());
      • We'll probably want to get rid of all whitespace in this serialization.
    • [x] convert all the handlers we have around remoting, etc.
  • [x] newTab needs to be smart enough to look for a ContentGuid() in the NewTerminalArgs. If it finds one, do the content process attach instead.
  • [x] same with splitPane.
  • 🦶 Move a pane to another window - PUNTED 🦶
    • This simply isn't a priority. We can file a follow-up for movePane(window). The tab-based code is MUCH easier to wrap your head around, and doesn't have the pane-wise edge cases.
    • [x] MovePane to another window raises a RequestMoveContent with the serialized form of the active pane.
    • [x] When TerminalPage handles MoveContentRequest, deserialize it...
    • [🦶] ... Look at the first action. If it's splitPane...
      • [🦶] if tabIndex is > tabs.size(), make a new tab.
        • "Move a pane to another window, as a tab" (movePane(index=99999, window={window}))
        • In #10780, "If the tab index is greater than the number of current tabs a new tab will be created with the pane as its root".
      • [x] else make a new pane.
        • "Move a pane to another window, as a pane"
  • [x] Move a tab to another window
    • [x] MovePane to another window raises a RequestMoveContent with the serialized form of the active tab.
    • [x] ... Look at the first action. If it's newTab...
      • [x] just run the actions. MoveContentArgs.tabIndex is unused in this case
  • [x] Wire up a callback s.t. the original window can close the tab/pane.
  • [ ] If we try to moveTab to a window that doesn't exist, create a new window for that name, with that content? Is that possible?
    • Let's sort this out in parallel with the callback above.
    • Like, we need to hang on to the content until someone else attaches oh my god it's so easy
    • [x] contentProcess.Attached() event. When the terminal move(Tab|Pane)s, add a callback to Attached. If we get that, we know someone else got it, and we can detach.
    • [x] Oh, uhg. But for a Tab, there's lots of contents (even for a pane). poop.
  • [x] Fix the pane serialization. Dragging a single tab now creates a tab, AND a pane for that single control. Yikes.

After this is all done, we definitely could plop the string into a DataPackage and unwrap that to drop tabs.

As of 35c82d37f: (commented out, there are better gifs below)

zadjii-msft avatar Jul 13 '22 21:07 zadjii-msft

📝 dev/migrie/oop/2/loki

  • [x] add a TabDragStarting to the TabView. In there, stash the serialized tab under the "content" key
  • [x] TabStripDragOver handler on TabView - look for the "content" key, and if it's there, accept it as a move
  • [x] TabView.TabStripDrop: get the content, deserialize, and fire it off to the action handler
  • [x] Make sure we can call back to the original window to remove the tab.

loki-with-detach-too

zadjii-msft avatar Jul 15 '22 11:07 zadjii-msft

I don't know if my issue related to the process model improvements, but I guess so. If I post it in a wrong thread, please move where needed.

Why don't you implement a per-tab execution policy for Windows Terminal? It would be very nice to have separate execution policies for different tabs, for everydays tasks Undefined policy is quite sufficient, but for some special development task like creating Python virtual environment I may need Unrestricted policy. It may be implemented by hotkey, or holding Ctrl-Shift on tab creation, whatever.

In the #632 you discussed why it is bad to have a mix of (non)elevated tabs, but here is a different thing. Changing execution policy and acquiring elevated permissions is not the same thing, and one can be done without another.

Suncatcher avatar Aug 09 '22 20:08 Suncatcher

@Suncatcher Thanks for the request. It's a good idea! The catch here, though, is that the execution policy is a property of the thing that is running inside terminal, right? I think it's actually specifically a PowerShell concept... and I don't believe there's a way using the Windows API to influence the execution policy of any instances of PowerShell under a process tree using CreateProcess (or its friends.) Even if there were, it wouldn't mean anything to cmd.exe or Cygwin or nushell or yori, simply because they don't have an equivalent concept. Perl does, somewhat, have a restricted mode. I don't know if that's equivalent... and perl doesn't see much interactive use.

The easiest way to accomplish this today is going to be to add new profiles that use PowerShell native arguments to set the execution policy you want, and open those profiles from the New Tab dropdown menu.

As an example...

image image
            {
                "commandline": "\"C:\\Program Files\\PowerShell\\7\\pwsh.exe\" -NoExit -Command \"Set-ExecutionPolicy Restricted -Scope Process\"",
                "guid": "{d4207635-ac89-4c36-803f-7566ab0e243b}",
                "hidden": false,
                "icon": "🔐",
                "name": "PowerShell (Restricted Policy)",
                "startingDirectory": "%USERPROFILE%",
                "background": "#330000"
            },

DHowett avatar Aug 09 '22 21:08 DHowett

thanks, didn't know this capability to set PS arguments per profile. Will use it.

Suncatcher avatar Aug 11 '22 12:08 Suncatcher

Process Model v3

Changes as of 2023

Everything is one process.

Proof-of-concept branch: https://github.com/microsoft/terminal/compare/main...dev/migrie/f/process-model-v3-test-0

Each of the headers here is an atomic PR plan, similar to the way I was doing the oop/2/ branches for PMv2. The goal being minimizing the individual code deltas.

dev/migrie/oop/3/foreword - #14825

Do this first, so that we can eventually have an AppLogic without necessitating a whole "window". Necessitated by the following train of thought:

TerminalApp::AppLogic needs to get instantiated after IslandWindow::Initialize (currently called in AppHost::Initialize) We need an AppLogic to get at the settings to see if we want a window at all. So now we need a window to determine if we need to create a window. That's stupid. Can we avoid doing any XAML work before we get at the settings? Will that work?

  • [x] Split AppLogic into AppLogic and TerminalWindow (aka "window logic" aka "window model")

dev/migrie/oop/3/ainulindale #14843

https://github.com/microsoft/terminal/compare/dev/migrie/oop/3/foreword...dev/migrie/oop/3/ainulindale

What if we started somewhere sane?

  • [x] One App per process.
  • [x] One AppHost per thread.
    • Which means one AppLogic per thread/window.
  • [x] One ::WindowEmperor per process. Create it on startup.
    • This is responsible for parsing the commandline and determining who wants a window.
      • To do that, it needs to make an AppLogic, FYI.
    • Also responsible for managing the window threads. It starts by not making any.
    • It has a CreateNewWindowThread(WindowRequestedArgs) method for the monarch to be able to request the creation of a new window thread (with a given ID / name / commandline / CWD).
  • [x] one WindowThread per window.
    • This owns the Peasant for the window, the AppHost, and the std::thread.
    • When it's created, it creates its own Peasant, registers it with the Monarch (who just requested it)
    • Pass that Peasant to AppHost instead of letting AH create it.
  • [x] When the process starts, WindowEmperor will look for an existing monarch
    • [x] Found one? Great - hand the commandline off and bail.
    • [x] No? Okay. We may or may not want to make a window and monarch.
      • [ ] Did the commandline not request a window? If so, display the error and bail.
      • [ ] It did request a window. Cool.
        • [x] register as the monarch, instantiate it.
        • [x] Emperor will set monarch.WindowRequested({ &emperor, Emperor::CreateNewWindowThread})
        • [ ] Make a window, passing the requested ID / Name (WindowRequested)
        • [x] the Window instantiates the peasant (it knows what name it wants)
  • [x] There's some sort of crash where AppLogic::_settings was null after a language reload. Look into that.
  • [x] Settings reloading doesn't seen to work at all anymore
    • fixed in 57d1dd435, needs cherry-pick
  • [x] If you do something like wtd -w 2, then the process that hands off crashes when the App gets released. Maybe we could not?
    • "fixed" in 427a4a51c5f57a01d010bab23803a0dba1b7c62c , needs cherry-pick
  • [x] Except hot reloading doesn't work for two windows open at the same time
    • fixed in 2122eec18, needs cp
  • [x] Defterm kinda... just... worked?
  • [x] Tray Icon
  • [ ] Global Hotkeys
    • [ ] Create the requested window if it didn't exist
  • [x] Window Properties (are they actually TwoWay bound correctly)
  • [ ] State persist/restoration
    • [ ] Because the process doesn't exit immediately when the last window is closed, we end up persisting the 0 window state. That's not what you'd want...
    • [x] Does restoring multiple windows work?
    • [x] Restoring one window in addition to running wtd -- foo.exe in another window is painfully janky
    • [ ] Still sorta off by one on restoring multiple windows.
  • [x] handoff to elevated windows
  • [x] quirky quake logic in TerminalWindow/TerminalPage
  • [x] Fragment extensions on multiple threads?
    • appears I never disabled this in this branch
  • [ ] Lifetime management on exit
    • [ ] Closing all windows should close the Terminal process.
    • [ ] WindowThread should have a pointer to the AppHost, init to nullputr.
    • [ ] It should have the std::thread
    • [ ] WindowThread.Start() should initialize the thread
    • [ ] At the bottom of the loop, it can trigger a callback to Emperor that can remove it from the list of windows
    • [ ] OR emperor could just detach from the thread, but that seems risque
  • [ ] ...

dev/migrie/oop/3/valaquenta

https://github.com/microsoft/terminal/compare/dev/migrie/oop/3/ainulindale...dev/migrie/oop/3/valaquenta

  • [x] ContentManager for storing GUID -> ControlInteractivity's
    • ~Idea: Let's make it map<guid, IInspectable>, and then QI to get the FrameworkElement?~ NO BAD FWE's ARE XAML AND THREAD-BOUND
      • ~Remoting could host it then...~ NO IT CAN'T FWE is XAML
    • but we might want to just do Control, since I think that's already included at a bunch of levels.
    • Actually heck, TerminalApp is a fine enough place. WindowExe already includes that dll, and we don't need it below there.
    • Probably just store in the WindowEmperor
  • [x] ContentManager is instantiated... globally? and passed to each AppHost, then down to the TerminalPage.
  • [x] TermControl adds a ctor where it takes an Interactivity (and settings).
  • [x] When TerminalPage wants to create a new control...
    • [x] Create the new Core and assign it a guid: ContentManager.CreateCore( settings )
    • [x] Use that Interactivity to make a new TermControl
  • [x] When TerminalPage wants to attach an existing tab/pane...
    • [x] get the Core for the given guid: ContentManager.LookupCore( guid )
    • [x] Use that Interactivity to make a new TermControl
    • Interactivity definitely needs to re-instantiate its Dispatcher
      • We'll do that in the next one
    • [x] Controls need to reparent the swapchain to this new panel. How do?
    • [x] When a control is attached to an existing interactivity, detach from the old one. I think this is the Attached event we had before
  • [x] When defterm does its thing...
    • [x] not sure we'll need to make any changes. The connection to _MakePane will be pre-filled in. We don't care about process boundaries for the connection, so uh, that's fine.
  • Wait, is that fine? Each connection will be processed on its own thread, separate from the UI thread for that control. Yea it can get moved from thread to thread, that's fine.
  • [x] Debug tap seemingly doesn't work, but by all accounts, it should.
    • Looks like any panes other than the first don't actually get serialized with __content
    • Actually fixed in 7d903dea4, there's nothing to do for this branch.
  • [ ] How do we handle locking? A ticket_lock? There's going to be multiple different UI threads that will ask for controls separately.
  • [x] Lifetime management: let's make them <guid, weak_ref<IInspectable>>. We'll use attached event handlers to know when it's safe for the original TermControl to go away.
    • [x] fixed in 4eba01f919951934b9b1f0a49fd4a3863c59959f
  • [x] We might want eea6a7b too

dev/migrie/oop/3/quenta-silmarillion #14866

https://github.com/microsoft/terminal/compare/dev/migrie/oop/3/valaquenta...dev/migrie/oop/3/quenta-silmarillion

This would be the equivalent of dev/migrie/oop/2/wandavision, see https://github.com/microsoft/terminal/issues/5000#issuecomment-1183694124

  • [x] Monarch needs to be able to move to a window based on ID, not just name
  • [ ] movePane(window)
    • [ ] A Pane might need to pre-init it's connection state from the control, because that might not be NotConnected anymore
  • [ ] moveTab(window)
    • [x] This needs to be able to support window without a direction, because right now it doesn't
      • fixed in a20201fff, needs cherry pick
  • [x] Pane border brushes are cached as static members, so there's one per process, certainly on the wrong thread. Don't do that anymore
    • 7bad8c964
  • [x] Wait I think 35c82d37f0cb79992e2075e5f6263d0b4e725cab was a mistake
  • [x] Copy or Paste (after moving a pane) seems to do the thing twice.
    • 1cef94e69
  • [x] Toggle Mark Mode seemed to crash - a failfast in the displayMarker lambda, I believe (the stack was hard to interpret). This might be because the TermControl isn't actually _initialized when it gets attached
    • I think this was ee63aff32
    • It was not.
    • auto transform{ marker.RenderTransform().as<Windows::UI::Xaml::Media::ScaleTransform>() }; crashed with a winrt::hresult_error: No such interface supported
    • eea6a7bcc fixed this.
  • [x] Moving needs to detach from the original window
  • [x] Closing a tab/pane shouldn't leak the control. Probably need ewak_ref's for this.
  • [x] After you move a tab/pane, it closes the connection (resulting in the input stopping). This is due to the control.Close() in Pane::Shutdown.
    • fixed in c9ed8dc04
  • [ ] what happens if a content dies after detaching and before re-attaching.
  • [x] Cherry-pick 7d903dea4
  • [ ] ...

dev/migrie/oop/3/akallabeth

https://github.com/microsoft/terminal/compare/dev/migrie/oop/3/quenta-silmarillion...dev/migrie/oop/3/akallabeth

  • [x] Make tabs draggable again
  • [x] I might be able to straight cherry-pick the loki changes.
  • [x] I was
  • [x] Tearing out a tab, but not dropping it onto the tab row yeets it to oblivion
  • [ ] you should be able to drop anywhere on the tab row
  • [x] Add the PID to the DataPackage, so that we can't attempt to accept a d/d from another process.
  • [x] Stare into the Door of Night into the void and see into The Timeless Void. Wait until the Dagor Dagorath for fixes to come to us.

dev/migrie/oop/3/an-unexpected-party

https://github.com/microsoft/terminal/compare/dev/migrie/oop/3/akallabeth...dev/migrie/oop/3/an-unexpected-party

  • [ ] you should be able to drop NOT on a window and have it create a new window

zadjii-msft avatar Jan 27 '23 21:01 zadjii-msft

In case I lose this over the long weekend:

Actual order of events I ge

sequenceDiagram
    participant Source
    participant Target

    Note Left of Source: _onTabDragStarting
    Source ->> Target: Drag tab
    Note right of Target: _onTabStripDragOver
    Target ->> Target: AcceptedOperation(DataPackageOperation::Move)
    Source --> Target: Release mouse (to drop)
    Note right of Target: _onTabStripDrop
    Target -->> Source: 
    Note Left of Source: TabViewTabDragStartingEventArgs<br>.OperationCompleted
    Note Left of Source: _onTabDroppedCompleted

You'll note: There's no message that the source gets prior to the target's TabStripDrop to confirm that the drop worked. So, we can't detach the content from the source before the target would already take it.

FURTHERMORE

There's no event in the target after the source gets the acceptance. So there's no way for the target to wait for the source to detach first.

LASTLY

There's no way for the target to communicate back to the source. The e.Data() is null on the target - only the DataView() is accessible. That's immutable (obviously). And OperationCompleted and TabDropCompleted give you didly.

The incredibly dumb solution I'm considering

sequenceDiagram
    participant Source
    participant Target
    participant Monarch

    Note Left of Source: _onTabDragStarting
    Source --> Source: stash dragged content
    Source --> Source: pack window ID into DataPackage

    Source ->> Target: Drag tab
    Note right of Target: _onTabStripDragOver
    Target ->> Target: AcceptedOperation(DataPackageOperation::Move)
    
    Source --> Target: Release mouse (to drop)
    
    Note right of Target: _onTabStripDrop
    Target --> Target: get WindowID from DataPackage
    Target -) Monarch: Request that WindowID sends content to us<br>RequestRecieveContent
    Monarch -) Source: Tell to send content to Target.Id<br>RequestSendContent, SendContent
    Source --> Source: detach our content
    Source -) Monarch: RequestMoveContent(stashed, target.id)
    Monarch -) Target: AttachContent(stashed)

    # Target -->> Source: 
    # Note Left of Source: TabViewTabDragStartingEventArgs<br>.OperationCompleted
    # Note Left of Source: _onTabDroppedCompleted

Pass the source's window ID in the package. When the target accepts it, have the target ask the monarch to ask the source to send its content to the target. Extremely dumb.

Other notes

This API is particularly painful for the kinds of UI experiences we want.

  • [ ] We probably can manually take a snapshot of our window and stash it in the floating icon, but ew
  • [ ] When the tab gets dragged over the target window, we should like, immediately detach and attach to the target, but attach to the TabItem that's being dragged. So that it seems like that tab is already in the tab row. I don't think that's possible.
  • [ ] Dragging to the right side of the TabView? Forget about it. We'll need to basically just handle drag/drop there manually...
  • [ ] When the dragged tab enters the target TabView, the target TV makes space for the tab. But it doesn't increase its width.
  • [ ] Almost all of this is worse than just rolling it ourselves the way Evan suggested.
  • [ ] Really, the HWND should be spawned as soon as the tabviewitem leaves the tabview. DragStarting however fires even if the tab is still in the "rail"
    • Consider, tear-out-and-snap. That needs an HWND to be dragged.
  • [ ] ...

reference

  • MSFT:41157072
  • MSFT:41383037
  • MSFT:41305287

zadjii-msft avatar Feb 17 '23 22:02 zadjii-msft

zadjii-msft avatar Mar 31 '23 19:03 zadjii-msft