Skip to content

Commit

Permalink
Switch Connections to use ValueSets to initialize them (#10184)
Browse files Browse the repository at this point in the history
#### ⚠️ targets #10051 

## Summary of the Pull Request

This PR does one big, primary thing. It removes all the constructors from any TerminalConnections, and changes them to use an `Initialize` method that accepts a `ValueSet` of properties.

Why?

For the upcoming window/content process work, we'll need the content process to be able to initialize the connection _in the content process_. However, the window process will be the one that knows what type of connection to make. Enter `ConnectionInformation`. This class will let us specify the class name of the type we want to create, and a set of settings to use when initializing that connection.

**IMPORTANT**: As a part of this, the constructor for a connection must have 0 arguments. `RoActivateInstance` lets you just conjure a WinRT type just by class name, but that class must have a 0 arg ctor. Hence the need for `Initialize`, to actually pass the settings.

We're using a `ValueSet` here because it's basically a json blob, with more steps. In the future, when extension authors want to have custom connections, we can always deserialize the json into a `ValueSet`, pass it to their connection's `Initialize`, and let then get what they need out of it.

## References
* Tear-out: #1256
* Megathread: #5000
* Project: https://github.com/microsoft/terminal/projects/5

## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50760298
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

`ConnectionInformation` was included as a part of this PR, to demonstrate how this will eventually be used. `ConnectionInformation` is not _currently_ used.

## Validation Steps Performed

It still builds and runs.
  • Loading branch information
zadjii-msft authored Jul 20, 2021
1 parent b05a557 commit 6e70c4a
Show file tree
Hide file tree
Showing 24 changed files with 304 additions and 79 deletions.
23 changes: 15 additions & 8 deletions scratch/ScratchIslandApp/SampleApp/MyPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "MyPage.h"
#include <LibraryResources.h>
#include "MyPage.g.cpp"
#include "..\..\..\src\cascadia\UnitTests_Control\MockControlSettings.h"
#include "MySettings.h"

using namespace std::chrono_literals;
using namespace winrt::Microsoft::Terminal;
Expand All @@ -26,17 +26,24 @@ namespace winrt::SampleApp::implementation

void MyPage::Create()
{
TerminalConnection::EchoConnection conn{};
auto settings = winrt::make_self<ControlUnitTests::MockControlSettings>();
auto settings = winrt::make_self<implementation::MySettings>();

auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted in-proc...",
winrt::hstring{},
L"",
nullptr,
32,
80,
winrt::guid()) };

// "Microsoft.Terminal.TerminalConnection.ConptyConnection"
winrt::hstring myClass{ winrt::name_of<TerminalConnection::ConptyConnection>() };
TerminalConnection::ConnectionInformation connectInfo{ myClass, connectionSettings };

TerminalConnection::ITerminalConnection conn{ TerminalConnection::ConnectionInformation::CreateConnection(connectInfo) };
Control::TermControl control{ *settings, conn };

InProcContent().Children().Append(control);

// Once the control loads (and not before that), write some text for debugging:
control.Initialized([conn](auto&&, auto&&) {
conn.WriteInput(L"This TermControl is hosted in-proc...");
});
}

// Method Description:
Expand Down
13 changes: 9 additions & 4 deletions scratch/ScratchIslandApp/SampleApp/MyPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@
<RowDefinition Height="*" />
</Grid.RowDefinitions>

<Button x:Name="TabRow"
Grid.Row="0">
&quot;Tabs&quot;
</Button>
<StackPanel Orientation="Horizontal">
<TextBox x:Name="GuidInput"
Width="400"
PlaceholderText="{}{guid here}" />
<Button Grid.Row="0">
Create
</Button>

</StackPanel>

<Grid x:Name="TabContent"
Grid.Row="1"
Expand Down
13 changes: 10 additions & 3 deletions scratch/ScratchIslandApp/SampleApp/MySettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Licensed under the MIT license.
--*/
#pragma once
#include "../../inc/cppwinrt_utils.h"
#include "../types/inc/colorTable.hpp"

#include <DefaultSettings.h>
#include <conattrs.hpp>
#include "MySettings.g.h"
Expand All @@ -12,9 +14,6 @@ namespace winrt::SampleApp::implementation
{
struct MySettings : MySettingsT<MySettings>
{
public:
MySettings() = default;

// --------------------------- Core Settings ---------------------------
// All of these settings are defined in ICoreSettings.

Expand Down Expand Up @@ -88,6 +87,14 @@ namespace winrt::SampleApp::implementation
winrt::Microsoft::Terminal::Core::Color GetColorTableEntry(int32_t index) noexcept { return _ColorTable.at(index); }
std::array<winrt::Microsoft::Terminal::Core::Color, 16> ColorTable() { return _ColorTable; }
void ColorTable(std::array<winrt::Microsoft::Terminal::Core::Color, 16> /*colors*/) {}

MySettings()
{
const auto campbellSpan = ::Microsoft::Console::Utils::CampbellColorTable();
std::transform(campbellSpan.begin(), campbellSpan.end(), _ColorTable.begin(), [](auto&& color) {
return static_cast<winrt::Microsoft::Terminal::Core::Color>(til::color{ color });
});
}
};
}

Expand Down
4 changes: 4 additions & 0 deletions scratch/ScratchIslandApp/SampleApp/dll/SampleApp.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@
<Private>true</Private>
<CopyLocalSatelliteAssemblies>true</CopyLocalSatelliteAssemblies>
</ProjectReference>

<ProjectReference Include="$(OpenConsoleDir)src\types\lib\types.vcxproj">
<Project>{18D09A24-8240-42D6-8CB6-236EEE820263}</Project>
</ProjectReference>
</ItemGroup>


Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/DebugTapConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace winrt::Microsoft::TerminalApp::implementation
_wrappedConnection{ std::move(wrappedConnection) }
{
}
void Initialize(const Windows::Foundation::Collections::ValueSet& /*settings*/) {}
~DebugInputTapConnection() = default;
void Start()
{
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/DebugTapConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace winrt::Microsoft::TerminalApp::implementation
{
public:
explicit DebugTapConnection(Microsoft::Terminal::TerminalConnection::ITerminalConnection wrappedConnection);
void Initialize(const Windows::Foundation::Collections::ValueSet& /*settings*/){};
~DebugTapConnection();
void Start();
void WriteInput(hstring const& data);
Expand Down
31 changes: 16 additions & 15 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -797,13 +797,14 @@ namespace winrt::TerminalApp::implementation
// TODO GH#4661: Replace this with directly using the AzCon when our VT is better
std::filesystem::path azBridgePath{ wil::GetModuleFileNameW<std::wstring>(nullptr) };
azBridgePath.replace_filename(L"TerminalAzBridge.exe");
connection = TerminalConnection::ConptyConnection(azBridgePath.wstring(),
L".",
L"Azure",
nullptr,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid());
connection = TerminalConnection::ConptyConnection();
connection.Initialize(TerminalConnection::ConptyConnection::CreateSettings(azBridgePath.wstring(),
L".",
L"Azure",
nullptr,
::base::saturated_cast<uint32_t>(settings.InitialRows()),
::base::saturated_cast<uint32_t>(settings.InitialCols()),
winrt::guid()));
}

else
Expand Down Expand Up @@ -833,14 +834,14 @@ namespace winrt::TerminalApp::implementation
std::filesystem::path cwd{ cwdString };
cwd /= settings.StartingDirectory().c_str();

auto conhostConn = TerminalConnection::ConptyConnection(
settings.Commandline(),
winrt::hstring{ cwd.c_str() },
settings.StartingTitle(),
envMap.GetView(),
settings.InitialRows(),
settings.InitialCols(),
winrt::guid());
auto conhostConn = TerminalConnection::ConptyConnection();
conhostConn.Initialize(TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
winrt::hstring{ cwd.wstring() },
settings.StartingTitle(),
envMap.GetView(),
::base::saturated_cast<uint32_t>(settings.InitialRows()),
::base::saturated_cast<uint32_t>(settings.InitialCols()),
winrt::guid()));

sessionGuid = conhostConn.Guid();
connection = conhostConn;
Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalAzBridge/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ int wmain(int /*argc*/, wchar_t** /*argv*/)

const auto size = GetConsoleScreenSize(conOut);

AzureConnection azureConn{ gsl::narrow_cast<uint32_t>(size.Y), gsl::narrow_cast<uint32_t>(size.X) };
AzureConnection azureConn{};
winrt::Windows::Foundation::Collections::ValueSet vs{};
vs.Insert(L"initialRows", winrt::Windows::Foundation::PropertyValue::CreateUInt32(gsl::narrow_cast<uint32_t>(size.Y)));
vs.Insert(L"initialCols", winrt::Windows::Foundation::PropertyValue::CreateUInt32(gsl::narrow_cast<uint32_t>(size.X)));
azureConn.Initialize(vs);

const auto state = RunConnectionToCompletion(azureConn, conOut, conIn);

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalAzBridge/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Module Name:
#include <wil/cppwinrt.h>

#include <winrt/Windows.system.h>
#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>

#include <wil/resource.h>
Expand Down
10 changes: 6 additions & 4 deletions src/cascadia/TerminalConnection/AzureConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return (AzureClientID != L"0");
}

AzureConnection::AzureConnection(const uint32_t initialRows, const uint32_t initialCols) :
_initialRows{ initialRows },
_initialCols{ initialCols },
_expiry{}
void AzureConnection::Initialize(const Windows::Foundation::Collections::ValueSet& settings)
{
if (settings)
{
_initialRows = winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialRows").try_as<Windows::Foundation::IPropertyValue>(), _initialRows);
_initialCols = winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialCols").try_as<Windows::Foundation::IPropertyValue>(), _initialCols);
}
}

// Method description:
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalConnection/AzureConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
static winrt::guid ConnectionType() noexcept;
static bool IsAzureConnectionAvailable() noexcept;
AzureConnection(const uint32_t rows, const uint32_t cols);

AzureConnection() = default;
void Initialize(const Windows::Foundation::Collections::ValueSet& settings);

void Start();
void WriteInput(hstring const& data);
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalConnection/AzureConnection.idl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.Terminal.TerminalConnection
static Guid ConnectionType { get; };
static Boolean IsAzureConnectionAvailable();

AzureConnection(UInt32 rows, UInt32 columns);
AzureConnection();
};

}
66 changes: 66 additions & 0 deletions src/cascadia/TerminalConnection/ConnectionInformation.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#include "pch.h"
#include "ConnectionInformation.h"
#include "ConnectionInformation.g.cpp"

namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
ConnectionInformation::ConnectionInformation(hstring const& className,
const Windows::Foundation::Collections::ValueSet& settings) :
_ClassName{ className },
_Settings{ settings }
{
}

// Function Description:
// - Create an instance of the connection specified in the
// ConnectionInformation, and Initialize it.
// - This static method allows the content process to create a connection
// from information that lives in the window process.
// Arguments:
// - info: A ConnectionInformation object that possibly lives out-of-proc,
// containing the name of the WinRT class we should activate for this
// connection, and a bag of setting to use to initialize that object.
// Return Value:
// - <none>
TerminalConnection::ITerminalConnection ConnectionInformation::CreateConnection(TerminalConnection::ConnectionInformation info)
try
{
Windows::Foundation::IInspectable inspectable{};

const auto name = static_cast<HSTRING>(winrt::get_abi(info.ClassName()));
const auto pointer = winrt::put_abi(inspectable);

#pragma warning(push)
#pragma warning(disable : 26490)
// C++/WinRT just loves it's void**, nothing we can do here _except_ reinterpret_cast
::IInspectable** raw = reinterpret_cast<::IInspectable**>(pointer);
#pragma warning(pop)

// RoActivateInstance() will try to create an instance of the object,
// who's fully qualified name is the string in Name().
//
// The class has to be activatable. For the Terminal, this is easy
// enough - we're not hosting anything that's not already in our
// manifest, or living as a .dll & .winmd SxS.
//
// When we get to extensions (GH#4000), we may want to revisit.
if (LOG_IF_FAILED(RoActivateInstance(name, raw)))
{
return nullptr;
}

// Now that thing we made, make sure it's actually a ITerminalConnection
if (const auto connection{ inspectable.try_as<TerminalConnection::ITerminalConnection>() })
{
// Initialize it, and return it.
connection.Initialize(info.Settings());
return connection;
}
return nullptr;
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
return nullptr;
}
}
43 changes: 43 additions & 0 deletions src/cascadia/TerminalConnection/ConnectionInformation.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*++
Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Class Name:
- ConnectionInformation.h
Abstract:
- This is a helper object for storing both the name of a type of connection, and
a bag of settings to use to initialize that connection.
- This helper is used primarily in cross-proc scenarios, to allow the window
process to tell the content process the name of the connection type it wants
created, and how to set that connection up. This is done so the connection can
live entirely in the content process, without having to go through the window
process at all.
--*/

#pragma once
#include "../inc/cppwinrt_utils.h"
#include "ConnectionInformation.g.h"

namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
struct ConnectionInformation : ConnectionInformationT<ConnectionInformation>
{
ConnectionInformation(hstring const& className,
const Windows::Foundation::Collections::ValueSet& settings);

static TerminalConnection::ITerminalConnection CreateConnection(TerminalConnection::ConnectionInformation info);

winrt::hstring ClassName() const { return _ClassName; }
void ClassName(const winrt::hstring& value) { _ClassName = value; }

WINRT_PROPERTY(Windows::Foundation::Collections::ValueSet, Settings);

private:
winrt::hstring _ClassName{};
};
}
namespace winrt::Microsoft::Terminal::TerminalConnection::factory_implementation
{
BASIC_FACTORY(ConnectionInformation);
}
17 changes: 17 additions & 0 deletions src/cascadia/TerminalConnection/ConnectionInformation.idl
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import "ITerminalConnection.idl";

namespace Microsoft.Terminal.TerminalConnection
{
[default_interface] runtimeclass ConnectionInformation
{
ConnectionInformation(String className, Windows.Foundation.Collections.ValueSet settings);
String ClassName { get; };
Windows.Foundation.Collections.ValueSet Settings { get; };

static ITerminalConnection CreateConnection(ConnectionInformation info);
}

}
Loading

0 comments on commit 6e70c4a

Please sign in to comment.