Skip to content

Commit

Permalink
Introduce a ContentManager helper (#14851)
Browse files Browse the repository at this point in the history
## Summary

_Thus we come to the introduction of a new servant, the
`ContentManager`, a singular entity that serves at the behest of the
`emperor`. It is its charge to keep track of all `TermControl` instances
created by the windows, for each window must seek its blessing before
calling forth such an instance._
_With the aid of the `ContentManager`, the `TermControl` shall now be
traced by the hand of fate through the use of unique identifying marks,
known as `GUID`s. Yet, its purpose remains yet unknown, for it is merely
a waypoint upon the journey yet to come._
_This act of bridging also brings a change to the handling of events
within the `TermControl`. This change shall see the addition of a
revoker, similar to the manner in which the `AppHost` hath employed it,
to the `TermControl`. Additionally, there is a new layer of indirection
between the `ControlCore` and the `App` layer, making ready for the day
when the `TermControl` may be repositioned and re-parented with ease._
_Consider this but a trivial act, a mere shadow of things yet to come,
for its impact shall be felt but briefly, like the passing of a gentle
breeze._

Related to #5000
Related to #1256

# Detailed description

This PR is another small bridge PR between the big work in #14843, and
the PR that will enable panes to move between windows.

This introduces a new class, called `ContentManager`. This is a global
singleton object, owned by the emperor. Whenever a window wants to
instantiate a new `TermControl`, it must ask the ContentManager to give
it one. This allows the ContentManager to track each "content" by GUID.
That's it. We don't do anything with them in this PR by itself, we just
track them.

This also includes a small change to the way TermControl events are
handled. It adds an `AppHost`-like revoker struct, and weak_ref's all
the handlers. We also add a layer of indirection between the
ControlCore's raising of events and the App layer's handling. This will
make reparenting content easier in the future.

This is a pretty trivial change which shouldn't have any major side
effects. Consider it exposition of the things to come. It's
intentionally small to try and keep the reviews more managable.
  • Loading branch information
zadjii-msft authored Mar 22, 2023
1 parent e0046a4 commit f3a722e
Show file tree
Hide file tree
Showing 21 changed files with 297 additions and 46 deletions.
15 changes: 11 additions & 4 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "../TerminalApp/ShortcutActionDispatch.h"
#include "../TerminalApp/TerminalTab.h"
#include "../TerminalApp/CommandPalette.h"
#include "../TerminalApp/ContentManager.h"
#include "CppWinrtTailored.h"

using namespace Microsoft::Console;
Expand Down Expand Up @@ -112,6 +113,7 @@ namespace TerminalAppLocalTests
CascadiaSettings initialSettings);
winrt::com_ptr<winrt::TerminalApp::implementation::TerminalPage> _commonSetup();
winrt::com_ptr<winrt::TerminalApp::implementation::WindowProperties> _windowProperties;
winrt::com_ptr<winrt::TerminalApp::implementation::ContentManager> _contentManager;
};

template<typename TFunction>
Expand Down Expand Up @@ -199,8 +201,11 @@ namespace TerminalAppLocalTests
_windowProperties = winrt::make_self<winrt::TerminalApp::implementation::WindowProperties>();
winrt::TerminalApp::WindowProperties props = *_windowProperties;

auto result = RunOnUIThread([&page, props]() {
page = winrt::make_self<winrt::TerminalApp::implementation::TerminalPage>(props);
_contentManager = winrt::make_self<winrt::TerminalApp::implementation::ContentManager>();
winrt::TerminalApp::ContentManager contentManager = *_contentManager;

auto result = RunOnUIThread([&page, props, contentManager]() {
page = winrt::make_self<winrt::TerminalApp::implementation::TerminalPage>(props, contentManager);
VERIFY_IS_NOT_NULL(page);
});
VERIFY_SUCCEEDED(result);
Expand Down Expand Up @@ -246,9 +251,11 @@ namespace TerminalAppLocalTests

_windowProperties = winrt::make_self<winrt::TerminalApp::implementation::WindowProperties>();
winrt::TerminalApp::WindowProperties props = *_windowProperties;
_contentManager = winrt::make_self<winrt::TerminalApp::implementation::ContentManager>();
winrt::TerminalApp::ContentManager contentManager = *_contentManager;
Log::Comment(NoThrowString().Format(L"Construct the TerminalPage"));
auto result = RunOnUIThread([&projectedPage, &page, initialSettings, props]() {
projectedPage = winrt::TerminalApp::TerminalPage(props);
auto result = RunOnUIThread([&projectedPage, &page, initialSettings, props, contentManager]() {
projectedPage = winrt::TerminalApp::TerminalPage(props, contentManager);
page.copy_from(winrt::get_self<winrt::TerminalApp::implementation::TerminalPage>(projectedPage));
page->_settings = initialSettings;
});
Expand Down
7 changes: 6 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ namespace winrt::TerminalApp::implementation
warnings,
_settings);

auto window = winrt::make_self<implementation::TerminalWindow>(*ev);
auto window = winrt::make_self<implementation::TerminalWindow>(*ev, _contentManager);

this->SettingsChanged({ window->get_weak(), &implementation::TerminalWindow::UpdateSettingsHandler });
if (_hasSettingsStartupActions)
Expand All @@ -731,6 +731,11 @@ namespace winrt::TerminalApp::implementation
return *window;
}

winrt::TerminalApp::ContentManager AppLogic::ContentManager()
{
return _contentManager;
}

bool AppLogic::ShouldUsePersistedLayout() const
{
return _settings.GlobalSettings().ShouldUsePersistedLayout();
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "LanguageProfileNotifier.h"
#include "AppCommandlineArgs.h"
#include "TerminalWindow.h"
#include "ContentManager.h"

#include <inc/cppwinrt_utils.h>
#include <ThrottledFunc.h>
Expand Down Expand Up @@ -70,6 +71,8 @@ namespace winrt::TerminalApp::implementation

TerminalApp::TerminalWindow CreateNewWindow();

winrt::TerminalApp::ContentManager ContentManager();

TerminalApp::ParseCommandlineResult GetParseCommandlineMessage(array_view<const winrt::hstring> args);

TYPED_EVENT(SettingsChanged, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SettingsLoadEventArgs);
Expand Down Expand Up @@ -98,6 +101,8 @@ namespace winrt::TerminalApp::implementation
winrt::com_ptr<LanguageProfileNotifier> _languageProfileNotifier;
wil::unique_folder_change_reader_nothrow _reader;

TerminalApp::ContentManager _contentManager{ winrt::make<implementation::ContentManager>() };

static TerminalApp::FindTargetWindowResult _doFindTargetWindow(winrt::array_view<const hstring> args,
const Microsoft::Terminal::Settings::Model::WindowingMode& windowingBehavior);

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ namespace TerminalApp
Boolean IsRunningElevated();
Boolean CanDragDrop();

ContentManager ContentManager { get; };

Boolean HasSettingsStartupActions();

Boolean ShouldUsePersistedLayout();
Expand Down
51 changes: 51 additions & 0 deletions src/cascadia/TerminalApp/ContentManager.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "pch.h"
#include "ContentManager.h"
#include "ContentManager.g.cpp"

#include <wil/token_helpers.h>

#include "../../types/inc/utils.hpp"

using namespace winrt::Windows::ApplicationModel;
using namespace winrt::Windows::ApplicationModel::DataTransfer;
using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Xaml::Controls;
using namespace winrt::Windows::UI::Core;
using namespace winrt::Windows::System;
using namespace winrt::Microsoft::Terminal;
using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Microsoft::Terminal::Settings::Model;

namespace winrt::TerminalApp::implementation
{
ControlInteractivity ContentManager::CreateCore(const Microsoft::Terminal::Control::IControlSettings& settings,
const IControlAppearance& unfocusedAppearance,
const TerminalConnection::ITerminalConnection& connection)
{
ControlInteractivity content{ settings, unfocusedAppearance, connection };
content.Closed({ get_weak(), &ContentManager::_closedHandler });

_content.emplace(content.Id(), content);

return content;
}

ControlInteractivity ContentManager::TryLookupCore(uint64_t id)
{
const auto it = _content.find(id);
return it != _content.end() ? it->second : ControlInteractivity{ nullptr };
}

void ContentManager::_closedHandler(winrt::Windows::Foundation::IInspectable sender,
winrt::Windows::Foundation::IInspectable e)
{
if (const auto& content{ sender.try_as<winrt::Microsoft::Terminal::Control::ControlInteractivity>() })
{
const auto& contentId{ content.Id() };
_content.erase(contentId);
}
}
}
44 changes: 44 additions & 0 deletions src/cascadia/TerminalApp/ContentManager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*++
Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Class Name:
- ContentManager.h
Abstract:
- This is a helper class for tracking all of the terminal "content" instances of
the Terminal. These are all the ControlInteractivity & ControlCore's of each
of our TermControls. These are each assigned a GUID on creation, and stored in
a map for later lookup.
- This is used to enable moving panes between windows. TermControl's are not
thread-agile, so they cannot be reused on other threads. However, the content
is. This helper, which exists as a singleton across all the threads in the
Terminal app, allows each thread to create content, assign it to a
TermControl, detach it from that control, and reattach to new controls on
other threads.
- When you want to create a new TermControl, call CreateCore to instantiate a
new content with a GUID for later reparenting.
--*/
#pragma once

#include "ContentManager.g.h"

#include <inc/cppwinrt_utils.h>
namespace winrt::TerminalApp::implementation
{
struct ContentManager : ContentManagerT<ContentManager>
{
public:
ContentManager() = default;
Microsoft::Terminal::Control::ControlInteractivity CreateCore(const Microsoft::Terminal::Control::IControlSettings& settings,
const Microsoft::Terminal::Control::IControlAppearance& unfocusedAppearance,
const Microsoft::Terminal::TerminalConnection::ITerminalConnection& connection);
Microsoft::Terminal::Control::ControlInteractivity TryLookupCore(uint64_t id);

private:
std::unordered_map<uint64_t, Microsoft::Terminal::Control::ControlInteractivity> _content;

void _closedHandler(winrt::Windows::Foundation::IInspectable sender,
winrt::Windows::Foundation::IInspectable e);
};
}
6 changes: 6 additions & 0 deletions src/cascadia/TerminalApp/TerminalAppLib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@
<ClInclude Include="AppLogic.h">
<DependentUpon>AppLogic.idl</DependentUpon>
</ClInclude>
<ClInclude Include="ContentManager.h">
<DependentUpon>TerminalPage.idl</DependentUpon>
</ClInclude>
<ClInclude Include="TerminalWindow.h">
<DependentUpon>TerminalWindow.idl</DependentUpon>
</ClInclude>
Expand Down Expand Up @@ -237,6 +240,9 @@
<ClCompile Include="AppLogic.cpp">
<DependentUpon>AppLogic.idl</DependentUpon>
</ClCompile>
<ClCompile Include="ContentManager.cpp">
<DependentUpon>TerminalPage.idl</DependentUpon>
</ClCompile>
<ClCompile Include="TerminalWindow.cpp">
<DependentUpon>TerminalWindow.idl</DependentUpon>
</ClCompile>
Expand Down
8 changes: 6 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ namespace winrt

namespace winrt::TerminalApp::implementation
{
TerminalPage::TerminalPage(TerminalApp::WindowProperties properties) :
TerminalPage::TerminalPage(TerminalApp::WindowProperties properties, const TerminalApp::ContentManager& manager) :
_tabs{ winrt::single_threaded_observable_vector<TerminalApp::TabBase>() },
_mruTabs{ winrt::single_threaded_observable_vector<TerminalApp::TabBase>() },
_startupActions{ winrt::single_threaded_vector<ActionAndArgs>() },
_manager{ manager },
_hostingHwnd{},
_WindowProperties{ std::move(properties) }
{
Expand Down Expand Up @@ -2593,7 +2594,10 @@ namespace winrt::TerminalApp::implementation
// Do any initialization that needs to apply to _every_ TermControl we
// create here.
// TermControl will copy the settings out of the settings passed to it.
TermControl term{ settings.DefaultSettings(), settings.UnfocusedSettings(), connection };

const auto content = _manager.CreateCore(settings.DefaultSettings(), settings.UnfocusedSettings(), connection);

TermControl term{ content };

// GH#12515: ConPTY assumes it's hidden at the start. If we're not, let it know now.
if (_visible)
Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ namespace winrt::TerminalApp::implementation
struct TerminalPage : TerminalPageT<TerminalPage>
{
public:
TerminalPage(TerminalApp::WindowProperties properties);
TerminalPage(TerminalApp::WindowProperties properties, const TerminalApp::ContentManager& manager);

// This implements shobjidl's IInitializeWithWindow, but due to a XAML Compiler bug we cannot
// put it in our inheritance graph. https://github.com/microsoft/microsoft-ui-xaml/issues/3331
Expand Down Expand Up @@ -224,9 +224,10 @@ namespace winrt::TerminalApp::implementation
bool _renamerPressedEnter{ false };

TerminalApp::WindowProperties _WindowProperties{ nullptr };

PaneResources _paneResources;

TerminalApp::ContentManager _manager{ nullptr };

winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::NewConnection_revoker _newConnectionRevoker;

winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowDialogHelper(const std::wstring_view& name);
Expand Down
9 changes: 8 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.idl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import "IDirectKeyListener.idl";

namespace TerminalApp
{
[default_interface] runtimeclass ContentManager
{
Microsoft.Terminal.Control.ControlInteractivity CreateCore(Microsoft.Terminal.Control.IControlSettings settings,
Microsoft.Terminal.Control.IControlAppearance unfocusedAppearance,
Microsoft.Terminal.TerminalConnection.ITerminalConnection connection);
Microsoft.Terminal.Control.ControlInteractivity TryLookupCore(UInt64 id);
}

[default_interface] runtimeclass LastTabClosedEventArgs
{
Expand Down Expand Up @@ -33,7 +40,7 @@ namespace TerminalApp

[default_interface] runtimeclass TerminalPage : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged, IDirectKeyListener
{
TerminalPage(WindowProperties properties);
TerminalPage(WindowProperties properties, ContentManager manager);

// XAML bound properties
String ApplicationDisplayName { get; };
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,10 @@ static Documents::Run _BuildErrorRun(const winrt::hstring& text, const ResourceD

namespace winrt::TerminalApp::implementation
{
TerminalWindow::TerminalWindow(const TerminalApp::SettingsLoadEventArgs& settingsLoadedResult) :
TerminalWindow::TerminalWindow(const TerminalApp::SettingsLoadEventArgs& settingsLoadedResult,
const TerminalApp::ContentManager& manager) :
_settings{ settingsLoadedResult.NewSettings() },
_manager{ manager },
_initialLoadResult{ settingsLoadedResult },
_WindowProperties{ winrt::make_self<TerminalApp::implementation::WindowProperties>() }
{
Expand All @@ -138,7 +140,7 @@ namespace winrt::TerminalApp::implementation
HRESULT TerminalWindow::Initialize(HWND hwnd)
{
// Now that we know we can do XAML, build our page.
_root = winrt::make_self<TerminalPage>(*_WindowProperties);
_root = winrt::make_self<TerminalPage>(*_WindowProperties, _manager);
_dialog = ContentDialog{};

// Pass commandline args into the TerminalPage. If we were supposed to
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalApp/TerminalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace winrt::TerminalApp::implementation
struct TerminalWindow : TerminalWindowT<TerminalWindow, IInitializeWithWindow>
{
public:
TerminalWindow(const TerminalApp::SettingsLoadEventArgs& settingsLoadedResult);
TerminalWindow(const TerminalApp::SettingsLoadEventArgs& settingsLoadedResult, const TerminalApp::ContentManager& manager);
~TerminalWindow() = default;

STDMETHODIMP Initialize(HWND hwnd);
Expand Down Expand Up @@ -177,6 +177,8 @@ namespace winrt::TerminalApp::implementation
Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };
TerminalApp::SettingsLoadEventArgs _initialLoadResult{ nullptr };

TerminalApp::ContentManager _manager{ nullptr };

void _ShowLoadErrorsDialog(const winrt::hstring& titleKey,
const winrt::hstring& contentKey,
HRESULT settingsLoadedResult,
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace TerminalApp
// information.
[default_interface] runtimeclass TerminalWindow : IDirectKeyListener, IDialogPresenter, Windows.UI.Xaml.Data.INotifyPropertyChanged
{
TerminalWindow(SettingsLoadEventArgs result);
TerminalWindow(SettingsLoadEventArgs result, ContentManager manager);

// For your own sanity, it's better to do setup outside the ctor.
// If you do any setup in the ctor that ends up throwing an exception,
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ namespace Microsoft.Terminal.Control
String HoveredUriText { get; };
Windows.Foundation.IReference<Microsoft.Terminal.Core.Point> HoveredCell { get; };

void Close();
void BlinkCursor();
Boolean IsInReadOnlyMode { get; };
Boolean CursorOn;
Expand Down
18 changes: 18 additions & 0 deletions src/cascadia/TerminalControl/ControlInteractivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ static constexpr unsigned int MAX_CLICK_COUNT = 3;

namespace winrt::Microsoft::Terminal::Control::implementation
{
std::atomic<uint64_t> ControlInteractivity::_nextId{ 1 };

static constexpr TerminalInput::MouseButtonState toInternalMouseState(const Control::MouseButtonState& state)
{
return TerminalInput::MouseButtonState{
Expand All @@ -44,9 +46,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_lastMouseClickPos{},
_selectionNeedsToBeCopied{ false }
{
_id = _nextId.fetch_add(1, std::memory_order_relaxed);

_core = winrt::make_self<ControlCore>(settings, unfocusedAppearance, connection);
}

uint64_t ControlInteractivity::Id()
{
return _id;
}

// Method Description:
// - Updates our internal settings. These settings should be
// interactivity-specific. Right now, we primarily update _rowsToScroll
Expand All @@ -73,6 +82,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return *_core;
}

void ControlInteractivity::Close()
{
_ClosedHandlers(*this, nullptr);
if (_core)
{
_core->Close();
}
}

// Method Description:
// - Returns the number of clicks that occurred (double and triple click support).
// Every call to this function registers a click.
Expand Down
Loading

0 comments on commit f3a722e

Please sign in to comment.