Skip to content

Commit 2c9d152

Browse files
authored
Convert the Open New Window handler to an event (#20311)
From ye old days where we were a multi-process app, we would `shellexecute` to create a new Terminal process when we wanted to open a new window. Now that we're all one process (#18215), this dance doesn't make sense anymore. So: let's just add a bunch of plumbing to take the IContentArgs up to the emperor and have it hand them back to a new window. (this will be used to resurrect #20162)
1 parent 60a58c9 commit 2c9d152

11 files changed

Lines changed: 61 additions & 63 deletions

File tree

src/cascadia/TerminalApp/AppActionHandlers.cpp

Lines changed: 19 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -872,70 +872,27 @@ namespace winrt::TerminalApp::implementation
872872
}
873873
}
874874

875-
// Function Description:
876-
// - Helper to launch a new WT instance. It can either launch the instance
877-
// elevated or unelevated.
878-
// - To launch elevated, it will as the shell to elevate the process for us.
879-
// This might cause a UAC prompt. The elevation is performed on a
880-
// background thread, as to not block the UI thread.
881-
// Arguments:
882-
// - newTerminalArgs: A NewTerminalArgs describing the terminal instance
883-
// that should be spawned. The Profile should be filled in with the GUID
884-
// of the profile we want to launch.
885-
// Return Value:
886-
// - <none>
887-
// Important: Don't take the param by reference, since we'll be doing work
888-
// on another thread.
889-
safe_void_coroutine TerminalPage::_OpenNewWindow(const INewContentArgs newContentArgs)
875+
// Ask the WindowEmperor (in-process) to create a brand-new window whose
876+
// first tab is described by `contentArgs`. This will bubble up to AppHost,
877+
// who will call WindowEmperor::CreateNewWindow.
878+
void TerminalPage::_OpenNewWindow(const INewContentArgs& contentArgs)
890879
{
891-
auto terminalArgs{ newContentArgs.try_as<NewTerminalArgs>() };
892-
893-
// Do nothing for non-terminal panes.
894-
//
895-
// Theoretically, we could define a `IHasCommandline` interface, and
896-
// stick `ToCommandline` on that interface, for any kind of pane that
897-
// wants to be convertable to a wt commandline.
898-
//
899-
// Another idea we're thinking about is just `wt do {literal json for an
900-
// action}`, which might be less leaky
901-
if (terminalArgs == nullptr)
880+
if (!contentArgs)
902881
{
903-
co_return;
882+
return;
904883
}
905884

906-
// ShellExecuteExW may block, so do it on a background thread.
907-
//
908-
// NOTE: All remaining code of this function doesn't touch `this`, so we don't need weak/strong_ref.
909-
// NOTE NOTE: Don't touch `this` when you make changes here.
910-
co_await winrt::resume_background();
911-
912-
// This will get us the correct exe for dev/preview/release. If you
913-
// don't stick this in a local, it'll get mangled by ShellExecute. I
914-
// have no idea why.
915-
const auto exePath{ GetWtExePath() };
916-
917-
// Build the commandline to pass to wt for this set of NewTerminalArgs
918-
// `-w -1` will ensure a new window is created.
919-
const auto commandline = terminalArgs.ToCommandline();
920-
const auto cmdline = fmt::format(FMT_COMPILE(L"-w -1 new-tab {}"), commandline);
921-
922-
// Build the args to ShellExecuteEx. We need to use ShellExecuteEx so we
923-
// can pass the SEE_MASK_NOASYNC flag. That flag allows us to safely
924-
// call this on the background thread, and have ShellExecute _not_ call
925-
// back to us on the main thread. Without this, if you close the
926-
// Terminal quickly after the UAC prompt, the elevated WT will never
927-
// actually spawn.
928-
SHELLEXECUTEINFOW seInfo{ 0 };
929-
seInfo.cbSize = sizeof(seInfo);
930-
seInfo.fMask = SEE_MASK_NOASYNC;
931-
// `open` will just run the executable normally.
932-
seInfo.lpVerb = L"open";
933-
seInfo.lpFile = exePath.c_str();
934-
seInfo.lpParameters = cmdline.c_str();
935-
seInfo.nShow = SW_SHOWNORMAL;
936-
LOG_IF_WIN32_BOOL_FALSE(ShellExecuteExW(&seInfo));
937-
938-
co_return;
885+
ActionAndArgs newTabAction{};
886+
newTabAction.Action(ShortcutAction::NewTab);
887+
newTabAction.Args(NewTabArgs{ contentArgs });
888+
889+
auto actions = winrt::single_threaded_vector<ActionAndArgs>({ std::move(newTabAction) });
890+
891+
// It's fine to pass `0` as the window ID, since this event path will
892+
// always land in CreateNewWindow, which will just ignore it.
893+
winrt::TerminalApp::WindowRequestedArgs request{ 0, winrt::TerminalApp::CommandlineArgs{} };
894+
request.StartupActions(std::move(actions));
895+
RequestNewWindow.raise(*this, request);
939896
}
940897

941898
void TerminalPage::_HandleNewWindow(const IInspectable& /*sender*/,
@@ -959,13 +916,13 @@ namespace winrt::TerminalApp::implementation
959916
newContentArgs = NewTerminalArgs{};
960917
}
961918

962-
if (const auto& terminalArgs{ newContentArgs.try_as<NewTerminalArgs>() })
919+
// Manually fill in the evaluated profile
920+
if (const auto terminalArgs{ newContentArgs.try_as<NewTerminalArgs>() })
963921
{
964922
const auto profile{ _settings.GetProfileForArgs(terminalArgs) };
965923
terminalArgs.Profile(::Microsoft::Console::Utils::GuidToString(profile.Guid()));
966924
}
967925

968-
// Manually fill in the evaluated profile.
969926
_OpenNewWindow(newContentArgs);
970927
actionArgs.Handled(true);
971928
}

src/cascadia/TerminalApp/Remoting.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ namespace winrt::TerminalApp::implementation
8585
WINRT_PROPERTY(TerminalApp::CommandlineArgs, Command, nullptr);
8686
WINRT_PROPERTY(winrt::hstring, Content);
8787
WINRT_PROPERTY(Windows::Foundation::IReference<Windows::Foundation::Rect>, InitialBounds);
88+
WINRT_PROPERTY(Windows::Foundation::Collections::IVector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs>, StartupActions, nullptr);
8889
};
8990
}
9091

src/cascadia/TerminalApp/Remoting.idl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,6 @@ namespace TerminalApp
5151
CommandlineArgs Command { get; };
5252
String Content { get; };
5353
Windows.Foundation.IReference<Windows.Foundation.Rect> InitialBounds { get; };
54+
Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.ActionAndArgs> StartupActions;
5455
};
5556
}

src/cascadia/TerminalApp/TerminalPage.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ namespace winrt::TerminalApp::implementation
215215
til::typed_event<Windows::Foundation::IInspectable, winrt::TerminalApp::RequestReceiveContentArgs> RequestReceiveContent;
216216

217217
til::typed_event<IInspectable, winrt::TerminalApp::LaunchPositionRequest> RequestLaunchPosition;
218+
til::typed_event<IInspectable, winrt::TerminalApp::WindowRequestedArgs> RequestNewWindow;
218219

219220
WINRT_OBSERVABLE_PROPERTY(winrt::Windows::UI::Xaml::Media::Brush, TitlebarBrush, PropertyChanged.raise, nullptr);
220221
WINRT_OBSERVABLE_PROPERTY(winrt::Windows::UI::Xaml::Media::Brush, FrameBrush, PropertyChanged.raise, nullptr);
@@ -334,7 +335,7 @@ namespace winrt::TerminalApp::implementation
334335
winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _duplicateConnectionForRestart(const TerminalApp::TerminalPaneContent& paneContent);
335336
void _restartPaneConnection(const TerminalApp::TerminalPaneContent&, const winrt::Windows::Foundation::IInspectable&);
336337

337-
safe_void_coroutine _OpenNewWindow(const Microsoft::Terminal::Settings::Model::INewContentArgs newContentArgs);
338+
void _OpenNewWindow(const Microsoft::Terminal::Settings::Model::INewContentArgs& contentArgs);
338339

339340
void _OpenNewTerminalViaDropdown(const Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);
340341

src/cascadia/TerminalApp/TerminalPage.idl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,7 @@ namespace TerminalApp
103103
event Windows.Foundation.TypedEventHandler<Object, RequestReceiveContentArgs> RequestReceiveContent;
104104

105105
event Windows.Foundation.TypedEventHandler<Object, LaunchPositionRequest> RequestLaunchPosition;
106+
107+
event Windows.Foundation.TypedEventHandler<Object, WindowRequestedArgs> RequestNewWindow;
106108
}
107109
}

src/cascadia/TerminalApp/TerminalWindow.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,15 @@ namespace winrt::TerminalApp::implementation
11031103
_initialContentArgs = wil::to_vector(args);
11041104
}
11051105

1106+
// Method Description:
1107+
// - Provide a pre-built list of startup actions for this window. Used by
1108+
// the in-proc OpenNewWindow event (see TerminalPage::_OpenNewWindow ->
1109+
// TerminalWindow -> AppHost -> WindowEmperor::OpenNewWindow)
1110+
void TerminalWindow::SetStartupActions(const IVector<ActionAndArgs>& actions)
1111+
{
1112+
_initialContentArgs = wil::to_vector(actions);
1113+
}
1114+
11061115
// Method Description:
11071116
// - Parse the provided commandline arguments into actions, and try to
11081117
// perform them immediately.

src/cascadia/TerminalApp/TerminalWindow.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ namespace winrt::TerminalApp::implementation
7979

8080
int32_t SetStartupCommandline(TerminalApp::CommandlineArgs args);
8181
void SetStartupContent(const winrt::hstring& content, const Windows::Foundation::IReference<Windows::Foundation::Rect>& contentBounds);
82+
void SetStartupActions(const Windows::Foundation::Collections::IVector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs>& actions);
8283
int32_t ExecuteCommandline(TerminalApp::CommandlineArgs args);
8384
void SetSettingsStartupArgs(const std::vector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs>& actions);
8485

@@ -231,6 +232,7 @@ namespace winrt::TerminalApp::implementation
231232
FORWARDED_TYPED_EVENT(RequestReceiveContent, Windows::Foundation::IInspectable, winrt::TerminalApp::RequestReceiveContentArgs, _root, RequestReceiveContent);
232233

233234
FORWARDED_TYPED_EVENT(RequestLaunchPosition, Windows::Foundation::IInspectable, winrt::TerminalApp::LaunchPositionRequest, _root, RequestLaunchPosition);
235+
FORWARDED_TYPED_EVENT(RequestNewWindow, Windows::Foundation::IInspectable, winrt::TerminalApp::WindowRequestedArgs, _root, RequestNewWindow);
234236

235237
#ifdef UNIT_TESTING
236238
friend class TerminalAppLocalTests::CommandlineTest;

src/cascadia/TerminalApp/TerminalWindow.idl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ namespace TerminalApp
5656

5757
Int32 SetStartupCommandline(CommandlineArgs args);
5858
void SetStartupContent(String json, Windows.Foundation.IReference<Windows.Foundation.Rect> bounds);
59+
void SetStartupActions(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.ActionAndArgs> actions);
5960
Int32 ExecuteCommandline(CommandlineArgs args);
6061

6162
Boolean ShouldImmediatelyHandoffToElevated();
@@ -140,6 +141,7 @@ namespace TerminalApp
140141
event Windows.Foundation.TypedEventHandler<Object, RequestMoveContentArgs> RequestMoveContent;
141142
event Windows.Foundation.TypedEventHandler<Object, RequestReceiveContentArgs> RequestReceiveContent;
142143
event Windows.Foundation.TypedEventHandler<Object, LaunchPositionRequest> RequestLaunchPosition;
144+
event Windows.Foundation.TypedEventHandler<Object, WindowRequestedArgs> RequestNewWindow;
143145

144146
void AttachContent(String content, UInt32 tabIndex);
145147
void SendContentToOther(RequestReceiveContentArgs args);

src/cascadia/WindowsTerminal/AppHost.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ void AppHost::_HandleCommandlineArgs(const winrt::TerminalApp::WindowRequestedAr
137137
_windowLogic.SetStartupContent(content, windowArgs.InitialBounds());
138138
_launchShowWindowCommand = SW_NORMAL;
139139
}
140+
else if (const auto actions = windowArgs.StartupActions(); actions && actions.Size() > 0)
141+
{
142+
_windowLogic.SetStartupActions(actions);
143+
_launchShowWindowCommand = SW_NORMAL;
144+
}
140145
else
141146
{
142147
const auto args = windowArgs.Command();
@@ -271,6 +276,7 @@ void AppHost::Initialize()
271276
_revokers.ShowWindowChanged = _windowLogic.ShowWindowChanged(winrt::auto_revoke, { this, &AppHost::_ShowWindowChanged });
272277
_revokers.RequestMoveContent = _windowLogic.RequestMoveContent(winrt::auto_revoke, { this, &AppHost::_handleMoveContent });
273278
_revokers.RequestReceiveContent = _windowLogic.RequestReceiveContent(winrt::auto_revoke, { this, &AppHost::_handleReceiveContent });
279+
_revokers.RequestNewWindow = _windowLogic.RequestNewWindow(winrt::auto_revoke, { this, &AppHost::_HandleNewWindowRequested });
274280

275281
// BODGY
276282
// On certain builds of Windows, when Terminal is set as the default
@@ -410,6 +416,18 @@ void AppHost::_HandleRequestLaunchPosition(const winrt::Windows::Foundation::IIn
410416
args.Position(_GetWindowLaunchPosition());
411417
}
412418

419+
// In-process replacement for the old `ShellExecute("wt -w -1 new-tab ...")`
420+
// dance. The page hands us a pre-built WindowRequestedArgs (with its
421+
// StartupActions already populated); we just forward it to the WindowEmperor.
422+
void AppHost::_HandleNewWindowRequested(const winrt::Windows::Foundation::IInspectable&,
423+
const winrt::TerminalApp::WindowRequestedArgs& args)
424+
{
425+
if (_windowManager && args)
426+
{
427+
_windowManager->CreateNewWindow(args);
428+
}
429+
}
430+
413431
LaunchPosition AppHost::_GetWindowLaunchPosition()
414432
{
415433
LaunchPosition pos{};

src/cascadia/WindowsTerminal/AppHost.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ class AppHost : public std::enable_shared_from_this<AppHost>
9797
void _OpenSystemMenu(const winrt::Windows::Foundation::IInspectable& sender,
9898
const winrt::Windows::Foundation::IInspectable& args);
9999

100+
void _HandleNewWindowRequested(const winrt::Windows::Foundation::IInspectable& sender,
101+
const winrt::TerminalApp::WindowRequestedArgs& args);
102+
100103
void _SystemMenuChangeRequested(const winrt::Windows::Foundation::IInspectable& sender,
101104
const winrt::TerminalApp::SystemMenuChangeArgs& args);
102105

@@ -161,6 +164,7 @@ class AppHost : public std::enable_shared_from_this<AppHost>
161164
winrt::TerminalApp::TerminalWindow::RequestMoveContent_revoker RequestMoveContent;
162165
winrt::TerminalApp::TerminalWindow::RequestReceiveContent_revoker RequestReceiveContent;
163166
winrt::TerminalApp::TerminalWindow::RequestLaunchPosition_revoker RequestLaunchPosition;
167+
winrt::TerminalApp::TerminalWindow::RequestNewWindow_revoker RequestNewWindow;
164168
winrt::TerminalApp::TerminalWindow::PropertyChanged_revoker PropertyChanged;
165169
winrt::TerminalApp::TerminalWindow::SettingsChanged_revoker SettingsChanged;
166170
winrt::TerminalApp::TerminalWindow::WindowSizeChanged_revoker WindowSizeChanged;

0 commit comments

Comments
 (0)