Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to quit all terminal instances #11143

Merged
6 commits merged into from
Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@
"toggleReadOnlyMode",
"toggleShaderEffects",
"wt",
"quit",
"unbound"
],
"type": "string"
Expand Down
30 changes: 30 additions & 0 deletions src/cascadia/Remoting/Monarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

peasant.ShowTrayIconRequested([this](auto&&, auto&&) { _ShowTrayIconRequestedHandlers(*this, nullptr); });
peasant.HideTrayIconRequested([this](auto&&, auto&&) { _HideTrayIconRequestedHandlers(*this, nullptr); });
peasant.QuitAllRequested({ this, &Monarch::_handleQuitAll });

_peasants[newPeasantsId] = peasant;

Expand Down Expand Up @@ -109,6 +110,35 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
}
}

// Method Description:
// - Gives the host process an opportunity to run any pre-close logic then
// requests all peasants to close.
// Arguments:
// - <none> used
// Return Value:
// - <none>
void Monarch::_handleQuitAll(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const winrt::Windows::Foundation::IInspectable& /*args*/)
{
// Let the process hosting the monarch run any needed logic before
// closing all windows.
_QuitAllRequestedHandlers(*this, nullptr);

// Tell all peasants to exit.
const auto callback = [&](const auto& /*id*/, const auto& p) {
p.Quit();
};
const auto onError = [&](const auto& id) {
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_handleQuitAll_Failed",
TraceLoggingInt64(id, "peasantID", "The ID of the peasant which we could not close"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
};

_forEachPeasant(callback, onError);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, so if all windows get told to quit at roughly the same time, including the monarch, would it pose any issues if the monarch was the first to quit while all other peasants are in the middle of quitting (maybe if some peasants had more tabs and took longer to quit)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Since ultimately each of the quit actions is asynchronous (somewhere in the chain is a fire_and_forget method) it is hard to say. I imagine there might be some fighting for who becomes the monarch but I think that shouldn't matter since the other thread will continue quitting even while that happens. I haven't attempted to prove that behavior, so take what I just said with a grain of salt.

Copy link
Contributor Author

@Rosefield Rosefield Sep 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a test I opened ~100 windows and initiated quit. All of the windows closed, but there remained a "runtime broker" process which exited a few seconds afterwards. I don't know what the "runtime broker" is or if I should be concerned that it hung around on its own for a little bit.

Edit: doesn't seem like the runtime broker always gets closed immediately, or at least it can stick around for a little while if there were a lot of windows.

Edit Edit: Maybe a bug, but it doesn't seem like it really matters and it is unrelated anyways, when opening a bunch of windows sometimes the process is named DesktopWindowXamlSource.

Edit Edit Edit: Opening a whole bunch of windows quickly can cause other problems. I managed to get memory corruption? Looks like the monarch died and a new one wasn't made, since quit/rename window/etc all fail to do anything.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leonMSFT With further testing, I was able to sometimes get it to close every window except one, that presumably became the new monarch, so I'll need to figure out how to fix that.

}

// Method Description:
// - Tells the monarch that a peasant is being closed.
// Arguments:
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/Remoting/Monarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TYPED_EVENT(HideTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(WindowCreated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(WindowClosed, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(QuitAllRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);

private:
uint64_t _ourPID;
Expand Down Expand Up @@ -95,6 +96,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
void _renameRequested(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args);

void _handleQuitAll(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& args);

// Method Description:
// - Helper for doing something on each and every peasant.
// - We'll try calling func on every peasant.
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/Remoting/Monarch.idl
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,6 @@ namespace Microsoft.Terminal.Remoting
event Windows.Foundation.TypedEventHandler<Object, Object> HideTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> WindowCreated;
event Windows.Foundation.TypedEventHandler<Object, Object> WindowClosed;
event Windows.Foundation.TypedEventHandler<Object, Object> QuitAllRequested;
};
}
32 changes: 32 additions & 0 deletions src/cascadia/Remoting/Peasant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,36 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
}

void Peasant::RequestQuitAll()
{
try
{
_QuitAllRequestedHandlers(*this, nullptr);
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
}
TraceLoggingWrite(g_hRemotingProvider,
"Peasant_RequestQuit",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
}

void Peasant::Quit()
{
try
{
_QuitRequestedHandlers(*this, nullptr);
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
}
TraceLoggingWrite(g_hRemotingProvider,
"Peasant_Quit",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
TraceLoggingKeyword(TIL_KEYWORD_TRACE));
}
}
4 changes: 4 additions & 0 deletions src/cascadia/Remoting/Peasant.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
void RequestRename(const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args);
void RequestShowTrayIcon();
void RequestHideTrayIcon();
void RequestQuitAll();
void Quit();

winrt::Microsoft::Terminal::Remoting::WindowActivatedArgs GetLastActivatedArgs();

Expand All @@ -45,6 +47,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TYPED_EVENT(SummonRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::SummonWindowBehavior);
TYPED_EVENT(ShowTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(HideTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(QuitAllRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(QuitRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);

private:
Peasant(const uint64_t testPID);
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/Remoting/Peasant.idl
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ namespace Microsoft.Terminal.Remoting
void Summon(SummonWindowBehavior behavior);
void RequestShowTrayIcon();
void RequestHideTrayIcon();
void RequestQuitAll();
void Quit();

event Windows.Foundation.TypedEventHandler<Object, WindowActivatedArgs> WindowActivated;
event Windows.Foundation.TypedEventHandler<Object, CommandlineArgs> ExecuteCommandlineRequested;
Expand All @@ -76,6 +78,8 @@ namespace Microsoft.Terminal.Remoting
event Windows.Foundation.TypedEventHandler<Object, SummonWindowBehavior> SummonRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> ShowTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> HideTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> QuitAllRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> QuitRequested;
};

[default_interface] runtimeclass Peasant : IPeasant
Expand Down
14 changes: 14 additions & 0 deletions src/cascadia/Remoting/WindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_monarch.FindTargetWindowRequested({ this, &WindowManager::_raiseFindTargetWindowRequested });
_monarch.ShowTrayIconRequested([this](auto&&, auto&&) { _ShowTrayIconRequestedHandlers(*this, nullptr); });
_monarch.HideTrayIconRequested([this](auto&&, auto&&) { _HideTrayIconRequestedHandlers(*this, nullptr); });
_monarch.QuitAllRequested([this](auto&&, auto&&) { _QuitAllRequestedHandlers(*this, nullptr); });

_BecameMonarchHandlers(*this, nullptr);
}
Expand Down Expand Up @@ -579,6 +580,19 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_peasant.RequestHideTrayIcon();
}

// Method Description:
// - Ask the monarch to quit all windows.
// Arguments:
// - <none>
// Return Value:
// - <none>
winrt::fire_and_forget WindowManager::RequestQuitAll()
{
auto strongThis{ get_strong() };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the strongThis for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I just copied from the above RequestHideTrayIcon and I assumed it was there for an important reason. It can probably be removed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, found something on it: #10938 (comment)

co_await winrt::resume_background();
_peasant.RequestQuitAll();
}

bool WindowManager::DoesQuakeWindowExist()
{
return _monarch.DoesQuakeWindowExist();
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/Remoting/WindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

winrt::fire_and_forget RequestShowTrayIcon();
winrt::fire_and_forget RequestHideTrayIcon();
winrt::fire_and_forget RequestQuitAll();
bool DoesQuakeWindowExist();
void UpdateActiveTabTitle(winrt::hstring title);

Expand All @@ -56,6 +57,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TYPED_EVENT(WindowClosed, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(ShowTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(HideTrayIconRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(QuitAllRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);

private:
bool _shouldCreateWindow{ false };
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/Remoting/WindowManager.idl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.Terminal.Remoting
void RequestShowTrayIcon();
void RequestHideTrayIcon();
UInt64 GetNumberOfPeasants();
void RequestQuitAll();
void UpdateActiveTabTitle(String title);
Boolean DoesQuakeWindowExist();
Windows.Foundation.Collections.IVectorView<PeasantInfo> GetPeasantInfos();
Expand All @@ -26,5 +27,6 @@ namespace Microsoft.Terminal.Remoting
event Windows.Foundation.TypedEventHandler<Object, Object> WindowClosed;
event Windows.Foundation.TypedEventHandler<Object, Object> ShowTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> HideTrayIconRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> QuitAllRequested;
};
}
9 changes: 8 additions & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,14 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleCloseWindow(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
CloseWindow();
CloseWindow(false);
args.Handled(true);
}

void TerminalPage::_HandleQuit(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
RequestQuit();
args.Handled(true);
}

Expand Down
10 changes: 9 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,14 @@ namespace winrt::TerminalApp::implementation
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));
}

void AppLogic::Quit()
{
if (_root)
{
_root->CloseWindow(true);
}
}

// Method Description:
// - Show a ContentDialog with buttons to take further action. Uses the
// FrameworkElements provided as the title and content of this dialog, and
Expand Down Expand Up @@ -1154,7 +1162,7 @@ namespace winrt::TerminalApp::implementation
{
if (_root)
{
_root->CloseWindow();
_root->CloseWindow(false);
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ namespace winrt::TerminalApp::implementation
void LoadSettings();
[[nodiscard]] Microsoft::Terminal::Settings::Model::CascadiaSettings GetSettings() const noexcept;

void Quit();

int32_t SetStartupCommandline(array_view<const winrt::hstring> actions);
int32_t ExecuteCommandline(array_view<const winrt::hstring> actions, const winrt::hstring& cwd);
TerminalApp::FindTargetWindowResult FindTargetWindow(array_view<const winrt::hstring> actions);
Expand Down Expand Up @@ -173,6 +175,7 @@ namespace winrt::TerminalApp::implementation
FORWARDED_TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs, _root, RenameWindowRequested);
FORWARDED_TYPED_EVENT(IsQuakeWindowChanged, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, IsQuakeWindowChanged);
FORWARDED_TYPED_EVENT(SummonWindowRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, SummonWindowRequested);
FORWARDED_TYPED_EVENT(QuitRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, QuitRequested);

#ifdef UNIT_TESTING
friend class TerminalAppLocalTests::CommandlineTest;
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ namespace TerminalApp
String ParseCommandlineMessage { get; };
Boolean ShouldExitEarly { get; };

void Quit();

void LoadSettings();
Windows.UI.Xaml.UIElement GetRoot();

Expand Down Expand Up @@ -97,5 +99,6 @@ namespace TerminalApp
event Windows.Foundation.TypedEventHandler<Object, Object> SettingsChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> IsQuakeWindowChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> SummonWindowRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> QuitRequested;
}
}
12 changes: 12 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@
<data name="CloseAll" xml:space="preserve">
<value>Close all</value>
</data>
<data name="Quit" xml:space="preserve">
<value>Quit</value>
</data>
<data name="CloseWindowWarningTitle" xml:space="preserve">
<value>Do you want to close all tabs?</value>
</data>
Expand Down Expand Up @@ -441,6 +444,15 @@
<value>Third-Party Notices</value>
<comment>A hyperlink name for the Terminal's third-party notices</comment>
</data>
<data name="QuitDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="QuitDialog.PrimaryButtonText" xml:space="preserve">
<value>Close all</value>
</data>
<data name="QuitDialog.Title" xml:space="preserve">
<value>Do you want to close all windows?</value>
</data>
<data name="CloseAllDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
Expand Down
43 changes: 41 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,21 @@ namespace winrt::TerminalApp::implementation
ShellExecute(nullptr, nullptr, currentPath.c_str(), nullptr, nullptr, SW_SHOW);
}

// Method Description:
// - Displays a dialog to warn the user that they are about to close all open windows.
// Once the user clicks the OK button, shut down the application.
// If cancel is clicked, the dialog will close.
// - Only one dialog can be visible at a time. If another dialog is visible
// when this is called, nothing happens. See _ShowDialog for details
winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> TerminalPage::_ShowQuitDialog()
{
if (auto presenter{ _dialogPresenter.get() })
{
co_return co_await presenter.ShowDialog(FindName(L"QuitDialog").try_as<WUX::Controls::ContentDialog>());
}
co_return ContentDialogResult::None;
}

// Method Description:
// - Displays a dialog for warnings found while closing the terminal app using
// key binding with multiple tabs opened. Display messages to warn user
Expand Down Expand Up @@ -1237,6 +1252,25 @@ namespace winrt::TerminalApp::implementation
}
return nullptr;
}
// Method Description:
// - Warn the user that they are about to close all open windows, then
// signal that we want to close everything.
fire_and_forget TerminalPage::RequestQuit()
{
if (!_displayingCloseDialog)
{
_displayingCloseDialog = true;
ContentDialogResult warningResult = co_await _ShowQuitDialog();
_displayingCloseDialog = false;

if (warningResult != ContentDialogResult::Primary)
{
co_return;
}

_QuitRequestedHandlers(nullptr, nullptr);
}
}

// Method Description:
// - Saves the window position and tab layout to the application state
Expand Down Expand Up @@ -1320,9 +1354,14 @@ namespace winrt::TerminalApp::implementation
// Method Description:
// - Close the terminal app. If there is more
// than one tab opened, show a warning dialog.
fire_and_forget TerminalPage::CloseWindow()
// Arguments:
// - bypassDialog: if true a dialog won't be shown even if the user would
// normally get confirmation. This is used in the case where the user
// has already been prompted by the Quit action.
fire_and_forget TerminalPage::CloseWindow(bool bypassDialog)
{
if (_HasMultipleTabs() &&
if (!bypassDialog &&
_HasMultipleTabs() &&
_settings.GlobalSettings().ConfirmCloseAllTabs() &&
!_displayingCloseDialog)
{
Expand Down
5 changes: 4 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ namespace winrt::TerminalApp::implementation
winrt::hstring ApplicationDisplayName();
winrt::hstring ApplicationVersion();

winrt::fire_and_forget CloseWindow();
winrt::fire_and_forget RequestQuit();
winrt::fire_and_forget CloseWindow(bool bypassDialog);

void ToggleFocusMode();
void ToggleFullscreen();
Expand Down Expand Up @@ -131,6 +132,7 @@ namespace winrt::TerminalApp::implementation
TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs);
TYPED_EVENT(IsQuakeWindowChanged, IInspectable, IInspectable);
TYPED_EVENT(SummonWindowRequested, IInspectable, IInspectable);
TYPED_EVENT(QuitRequested, IInspectable, IInspectable);

private:
friend struct TerminalPageT<TerminalPage>; // for Xaml to bind events
Expand Down Expand Up @@ -189,6 +191,7 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<Toast> _windowRenameFailedToast{ nullptr };

void _ShowAboutDialog();
winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowQuitDialog();
winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowCloseWarningDialog();
winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowCloseReadOnlyDialog();
winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowMultiLinePasteWarningDialog();
Expand Down
Loading