starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Fix circular TermControl reference (#14228)

Browse Source 
This regression was introduced in b3c9f01. Since `TermControl` is the XAML
object that owns its scrollbar and the scrollbar's `VisualStateManager`
a strong reference back to the `TermControl` results in a circular reference.

## Validation Steps Performed
* Set a breakpoint on `TermControl::~TermControl()`
* Breakpoint hits on tab close ✅
This commit is contained in:
Leonard Hecker
2022-10-17 23:14:00 +02:00
committed by GitHub
parent 6d94fbc89f
commit bfd480b885
2 changed files with 29 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/cascadia/TerminalControl/ScrollBarVisualStateManager.cpp
View File

@@ -5,13 +5,10 @@
#include "ScrollBarVisualStateManager.h"
#include "ScrollBarVisualStateManager.g.cpp"
using namespace winrt::Windows::UI::Xaml::Media;
namespace winrt::Microsoft::Terminal::Control::implementation
{
ScrollBarVisualStateManager::ScrollBarVisualStateManager() :
_termControl(nullptr)
{
}
bool ScrollBarVisualStateManager::GoToStateCore(
winrt::Windows::UI::Xaml::Controls::Control const& control,
winrt::Windows::UI::Xaml::FrameworkElement const& templateRoot,
@@ -20,33 +17,39 @@ namespace winrt::Microsoft::Terminal::Control::implementation
winrt::Windows::UI::Xaml::VisualState const& state,
bool useTransitions)
{
if (!_termControl)
if (!_initialized)
{
for (auto parent = winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetParent(control);
parent != nullptr;
parent = winrt::Windows::UI::Xaml::Media::VisualTreeHelper::GetParent(parent))
_initialized = true;
Control::TermControl termControl{ nullptr };
for (auto parent = VisualTreeHelper::GetParent(control); parent; parent = VisualTreeHelper::GetParent(parent))
{
if (parent.try_as(_termControl))
if (parent.try_as(termControl))
{
_termControl = winrt::get_self<TermControl>(termControl)->get_weak();
break;
}
}
assert(termControl);
}
WINRT_ASSERT(_termControl);
auto scrollState = _termControl.Settings().ScrollState();
if (scrollState == ScrollbarState::Always)
if (const auto termControl = _termControl.get())
{
// If we're in Always mode, and the control is trying to collapse,
// go back to expanded
if (stateName == L"Collapsed" || stateName == L"CollapsedWithoutAnimation")
const auto scrollState = termControl->Settings().ScrollState();
if (scrollState == ScrollbarState::Always)
{
for (auto foundState : group.States())
// If we're in Always mode, and the control is trying to collapse,
// go back to expanded
if (stateName == L"Collapsed" || stateName == L"CollapsedWithoutAnimation")
{
if (foundState.Name() == L"ExpandedWithoutAnimation")
for (auto foundState : group.States())
{
return base_type::GoToStateCore(control, templateRoot, foundState.Name(), group, foundState, false);
if (foundState.Name() == L"ExpandedWithoutAnimation")
{
return base_type::GoToStateCore(control, templateRoot, foundState.Name(), group, foundState, false);
}
}
}
}

11
src/cascadia/TerminalControl/ScrollBarVisualStateManager.h
View File

@@ -11,8 +11,10 @@
//
#pragma once
#include "winrt/Windows.UI.Xaml.h"
#include "winrt/Windows.UI.Xaml.Controls.h"
#include <winrt/Windows.UI.Xaml.h>
#include <winrt/Windows.UI.Xaml.Controls.h>
#include "TermControl.h"
#include "ScrollBarVisualStateManager.g.h"
@@ -20,12 +22,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
struct ScrollBarVisualStateManager : ScrollBarVisualStateManagerT<ScrollBarVisualStateManager>
{
ScrollBarVisualStateManager();
bool GoToStateCore(winrt::Windows::UI::Xaml::Controls::Control const& control, winrt::Windows::UI::Xaml::FrameworkElement const& templateRoot, hstring const& stateName, winrt::Windows::UI::Xaml::VisualStateGroup const& group, winrt::Windows::UI::Xaml::VisualState const& state, bool useTransitions);
private:
winrt::Microsoft::Terminal::Control::TermControl _termControl;
winrt::weak_ref<TermControl> _termControl;
bool _initialized = false;
};
}