mirror of
https://github.com/microsoft/terminal.git
synced 2026-07-08 17:46:05 +00:00
Fix memory leaks with UIA (#19950)
## Summary of the Pull Request
This includes the memory leak fixes that @lhecker and I investigated as
a part of #19710.
The `ITextRangeProvider`s (namely `UiaTextRange`s) weren't being
destroyed after they were done being used by the screen reader.
## Validation Steps Performed
In my own testing, I set a breakpoint on the destructor for
`UiaTextRangeBase`. Prior to this change, that destructor would mainly
be called when the terminal control was closed, which would result in us
leaking these objects. With this change, I've confirmed that these text
ranges are being destroyed immediately after they are done being used
(without needing to close the terminal control).
## PR Checklist
Closes #19710
(cherry picked from commit e2110e716c)
Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgnCP0M
Service-Version: 1.24
This commit is contained in:
committed by
Dustin L. Howett
parent
8474eb8329
commit
9445a75171
1
.github/actions/spelling/expect/expect.txt
vendored
1
.github/actions/spelling/expect/expect.txt
vendored
@@ -1770,6 +1770,7 @@ uld
|
||||
uldash
|
||||
uldb
|
||||
ulwave
|
||||
Unaccess
|
||||
Unadvise
|
||||
unattend
|
||||
UNCPRIORITY
|
||||
|
||||
@@ -50,7 +50,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
void InteractivityAutomationPeer::ParentProvider(AutomationPeer parentProvider)
|
||||
{
|
||||
_parentProvider = parentProvider;
|
||||
// LOAD-BEARING: use _parentProvider->ProviderFromPeer(_parentProvider) instead of this->ProviderFromPeer(*this).
|
||||
// Since we split the automation peer into TermControlAutomationPeer and InteractivityAutomationPeer,
|
||||
// using "this" returns null. This can cause issues with some UIA Client scenarios like any navigation in Narrator.
|
||||
_parentProvider = parentProvider ? parentProvider.as<IAutomationPeerProtected>().ProviderFromPeer(parentProvider) : nullptr;
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
@@ -182,15 +185,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
XamlAutomation::ITextRangeProvider InteractivityAutomationPeer::_CreateXamlUiaTextRange(UIA::ITextRangeProvider* returnVal) const
|
||||
{
|
||||
// LOAD-BEARING: use _parentProvider->ProviderFromPeer(_parentProvider) instead of this->ProviderFromPeer(*this).
|
||||
// Since we split the automation peer into TermControlAutomationPeer and InteractivityAutomationPeer,
|
||||
// using "this" returns null. This can cause issues with some UIA Client scenarios like any navigation in Narrator.
|
||||
const auto parent{ _parentProvider.get() };
|
||||
if (!parent)
|
||||
if (!_parentProvider)
|
||||
{
|
||||
return nullptr;
|
||||
}
|
||||
const auto xutr = winrt::make_self<XamlUiaTextRange>(returnVal, parent.as<IAutomationPeerProtected>().ProviderFromPeer(parent));
|
||||
const auto xutr = winrt::make_self<XamlUiaTextRange>(returnVal, _parentProvider);
|
||||
return xutr.as<XamlAutomation::ITextRangeProvider>();
|
||||
};
|
||||
|
||||
@@ -202,22 +201,24 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
// - com_array of Xaml Wrapped UiaTextRange (ITextRangeProviders)
|
||||
com_array<XamlAutomation::ITextRangeProvider> InteractivityAutomationPeer::WrapArrayOfTextRangeProviders(SAFEARRAY* textRanges)
|
||||
{
|
||||
if (!_parentProvider)
|
||||
{
|
||||
return {};
|
||||
}
|
||||
|
||||
// transfer ownership of UiaTextRanges to this new vector
|
||||
auto providers = SafeArrayToOwningVector<::Microsoft::Terminal::TermControlUiaTextRange>(textRanges);
|
||||
auto count = gsl::narrow<int>(providers.size());
|
||||
const auto len = gsl::narrow<uint32_t>(providers.size());
|
||||
com_array<XamlAutomation::ITextRangeProvider> result{ len };
|
||||
|
||||
std::vector<XamlAutomation::ITextRangeProvider> vec;
|
||||
vec.reserve(count);
|
||||
for (auto i = 0; i < count; i++)
|
||||
for (uint32_t i = 0; i < len; ++i)
|
||||
{
|
||||
if (auto xutr = _CreateXamlUiaTextRange(providers[i].detach()))
|
||||
{
|
||||
vec.emplace_back(std::move(xutr));
|
||||
result[i] = std::move(xutr);
|
||||
}
|
||||
}
|
||||
|
||||
com_array<XamlAutomation::ITextRangeProvider> result{ vec };
|
||||
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,7 +80,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
::Microsoft::WRL::ComPtr<::Microsoft::Terminal::TermControlUiaProvider> _uiaProvider;
|
||||
winrt::Microsoft::Terminal::Control::implementation::ControlInteractivity* _interactivity;
|
||||
weak_ref<Windows::UI::Xaml::Automation::Peers::AutomationPeer> _parentProvider;
|
||||
winrt::Windows::UI::Xaml::Automation::Provider::IRawElementProviderSimple _parentProvider{ nullptr };
|
||||
|
||||
til::rect _controlBounds{};
|
||||
til::rect _controlPadding{};
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
namespace Microsoft.Terminal.Control
|
||||
{
|
||||
[default_interface] runtimeclass InteractivityAutomationPeer :
|
||||
Windows.UI.Xaml.Automation.Peers.AutomationPeer,
|
||||
Windows.UI.Xaml.Automation.Peers.FrameworkElementAutomationPeer,
|
||||
Windows.UI.Xaml.Automation.Provider.ITextProvider
|
||||
{
|
||||
|
||||
|
||||
@@ -122,6 +122,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
// GH#13978: If the TermControl has already been removed from the UI tree, XAML might run into weird bugs.
|
||||
// This will prevent the `dispatcher.RunAsync` calls below from raising UIA events on the main thread.
|
||||
_termControl = {};
|
||||
|
||||
// Solve the circular reference between us and the content automation peer.
|
||||
_contentAutomationPeer.ParentProvider(nullptr);
|
||||
}
|
||||
|
||||
// Method Description:
|
||||
|
||||
@@ -291,9 +291,11 @@ std::vector<wil::com_ptr<T>> SafeArrayToOwningVector(SAFEARRAY* safeArray)
|
||||
std::vector<wil::com_ptr<T>> result{ gsl::narrow<std::size_t>(count) };
|
||||
for (int i = 0; i < count; i++)
|
||||
{
|
||||
result[i].attach(pVals[i]);
|
||||
result[i] = pVals[i];
|
||||
}
|
||||
|
||||
THROW_IF_FAILED(SafeArrayUnaccessData(safeArray));
|
||||
THROW_IF_FAILED(SafeArrayDestroy(safeArray));
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
@@ -257,7 +257,7 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::GetSelection(_Outptr_result_maybenull_
|
||||
UiaTracing::TextProvider::GetSelection(*this, *range.Get());
|
||||
|
||||
LONG currentIndex = 0;
|
||||
hr = SafeArrayPutElement(*ppRetVal, ¤tIndex, range.Detach());
|
||||
hr = SafeArrayPutElement(*ppRetVal, ¤tIndex, range.Get());
|
||||
if (FAILED(hr))
|
||||
{
|
||||
SafeArrayDestroy(*ppRetVal);
|
||||
@@ -301,7 +301,7 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::GetVisibleRanges(_Outptr_result_mayben
|
||||
UiaTracing::TextProvider::GetVisibleRanges(*this, *range.Get());
|
||||
|
||||
LONG currentIndex = 0;
|
||||
hr = SafeArrayPutElement(*ppRetVal, ¤tIndex, range.Detach());
|
||||
hr = SafeArrayPutElement(*ppRetVal, ¤tIndex, range.Get());
|
||||
if (FAILED(hr))
|
||||
{
|
||||
SafeArrayDestroy(*ppRetVal);
|
||||
|
||||
Reference in New Issue
Block a user