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 workaround so ItemsRepeater doesn't leak the result of DataTemplate::Load #3068

Merged
merged 3 commits into from
Aug 7, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
60 changes: 60 additions & 0 deletions dev/Repeater/APITests/RepeaterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
using System.Threading;
using System.Collections.Generic;
using Windows.UI.Xaml.Tests.MUXControls.ApiTests.RepeaterTests.Common.Mocks;
using Windows.UI.Xaml.Tests.MUXControls.ApiTests.RepeaterTests.Common;
using Windows.UI.Composition;
using System.Threading.Tasks;

namespace Windows.UI.Xaml.Tests.MUXControls.ApiTests.RepeaterTests
{
Expand Down Expand Up @@ -609,5 +612,62 @@ public void VerifyUIElementsInItemsSource()
}
});
}

[TestMethod]
public void VerifyRepeaterDoesNotLeakItemContainers()
{
ObservableCollection<int> items = new ObservableCollection<int>();
for(int i=0;i<10;i++)
{
items.Add(i);
}

ItemsRepeater repeater = null;

RunOnUIThread.Execute(() =>
{
var template = (DataTemplate)XamlReader.Load("<DataTemplate xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation' "
+ "xmlns:local='using:MUXControlsTestApp.Samples'>"
+ "<local:DisposableUserControl Number='{Binding}'/>"
+ "</DataTemplate>");
Verify.IsNotNull(template);
Verify.AreEqual(0, MUXControlsTestApp.Samples.DisposableUserControl.OpenItems, "Verify we start with 0 DisposableUserControl");

repeater = new ItemsRepeater() {
ItemsSource = items,
ItemTemplate = template,
VerticalAlignment = VerticalAlignment.Top,
HorizontalAlignment = HorizontalAlignment.Left
};

Content = repeater;

});

IdleSynchronizer.Wait();
// Wait until the Tick to make sure all layout has completed.
RunOnUIThread.WaitForTick();

RunOnUIThread.Execute(() =>
{

Verify.IsGreaterThanOrEqual(MUXControlsTestApp.Samples.DisposableUserControl.OpenItems, 10, "Verify we created at least 10 DisposableUserControl");

// Clear out the repeater and make sure everything gets cleaned up.
Content = null;
repeater = null;
});

IdleSynchronizer.Wait();
// Wait until the Tick to make sure all layout has completed.
RunOnUIThread.WaitForTick();

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();

Verify.AreEqual(0, MUXControlsTestApp.Samples.DisposableUserControl.OpenItems, "Verify we cleaned up all the DisposableUserControl that were created");
}

}
}
12 changes: 11 additions & 1 deletion dev/Repeater/ItemsRepeater.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,17 @@ void ItemsRepeater::OnItemTemplateChanged(const winrt::IElementFactory& oldValue
if (auto dataTemplate = newValue.try_as<winrt::DataTemplate>())
{
m_itemTemplateWrapper = winrt::make<ItemTemplateWrapper>(dataTemplate);
if (!dataTemplate.LoadContent().as<winrt::FrameworkElement>()) {
if (auto content = dataTemplate.LoadContent().as<winrt::FrameworkElement>())
{
// Due to bug https://github.com/microsoft/microsoft-ui-xaml/issues/3057, we need to get the framework
// to take ownership of the extra implicit ref that was returned by LoadContent. The simplest way to do
// this is to add it to a Children collection and immediately remove it.
auto children = Children();
children.Append(content);
children.RemoveAtEnd();
}
else
{
// We have a DataTemplate which is empty, so we need to set it to true
m_isItemTemplateEmpty = true;
}
Expand Down
7 changes: 7 additions & 0 deletions dev/Repeater/TestUI/Repeater_TestUI.projitems
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
</Page>
<Page Include="$(MSBuildThisFileDirectory)Samples\ItemTemplateSamples\DisposableUserControl.xaml">
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
</Page>
<Page Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\VirtualLayoutPages\ActivityFeedSamplePage.xaml">
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
Expand Down Expand Up @@ -112,6 +116,9 @@
<Compile Include="$(MSBuildThisFileDirectory)Samples\ItemsSourceSamples\SortingAndFilteringPage.xaml.cs">
<DependentUpon>SortingAndFilteringPage.xaml</DependentUpon>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)Samples\ItemTemplateSamples\DisposableUserControl.xaml.cs">
<DependentUpon>DisposableUserControl.xaml</DependentUpon>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\VirtualLayoutPages\ActivityFeedLayout.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\VirtualLayoutPages\ActivityFeedSamplePage.xaml.cs">
<DependentUpon>ActivityFeedSamplePage.xaml</DependentUpon>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<UserControl
x:Class="MUXControlsTestApp.Samples.DisposableUserControl"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
d:DesignHeight="300"
d:DesignWidth="400">

<Grid>
<TextBlock Text="{x:Bind Number}"/>
</Grid>
</UserControl>
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.WindowsRuntime;
using System.Threading;
using Windows.Foundation;
using Windows.Foundation.Collections;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Controls.Primitives;
using Windows.UI.Xaml.Data;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Media;
using Windows.UI.Xaml.Navigation;

// The User Control item template is documented at https://go.microsoft.com/fwlink/?LinkId=234236

namespace MUXControlsTestApp.Samples
{
public sealed partial class DisposableUserControl : UserControl
{

public int Number
{
get { return (int)GetValue(NumberProperty); }
set { SetValue(NumberProperty, value); }
}

public static readonly DependencyProperty NumberProperty =
DependencyProperty.Register("Number", typeof(int), typeof(DisposableUserControl), new PropertyMetadata(-1));


public static int OpenItems { get { return _counter; } }
private static int _counter = 0;

public DisposableUserControl()
{
Interlocked.Increment(ref _counter);
this.InitializeComponent();
}

~DisposableUserControl()
{
Interlocked.Decrement(ref _counter);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<ColumnDefinition Width="auto" />
<ColumnDefinition Width="auto" />
<ColumnDefinition Width="auto" />
<ColumnDefinition Width="auto" />
</Grid.ColumnDefinitions>
<StackPanel>
<TextBlock>DataTemplate Sample:</TextBlock>
Expand Down Expand Up @@ -80,7 +81,7 @@
<TextBlock>ElementFactory Multiple Template Sample:</TextBlock>
<controls:ItemsRepeaterScrollHost Height="400" Width="150">
<ScrollViewer>
<controls:ItemsRepeater x:Name="repeater3"
<controls:ItemsRepeater x:Name="repeater4"
ItemsSource="{x:Bind Data}"
Layout="{StaticResource stackLayout}">
<controls:ItemsRepeater.ItemTemplate>
Expand All @@ -99,5 +100,21 @@
</ScrollViewer>
</controls:ItemsRepeaterScrollHost>
</StackPanel>
<StackPanel Grid.Column="4">
<TextBlock>DataTemplate clear count checking:</TextBlock>
<controls:ItemsRepeaterScrollHost Height="400" Width="150">
<ScrollViewer>
<controls:ItemsRepeater x:Name="repeater3"
ItemsSource="{x:Bind Numbers}"
Layout="{StaticResource stackLayout}">
<controls:ItemsRepeater.ItemTemplate>
<DataTemplate x:DataType="local:MyData" >
<local:DisposableUserControl Number="{x:Bind number}"/>
</DataTemplate>
</controls:ItemsRepeater.ItemTemplate>
</controls:ItemsRepeater>
</ScrollViewer>
</controls:ItemsRepeaterScrollHost>
</StackPanel>
</Grid>
</Page>
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@ namespace MUXControlsTestApp.Samples
{
public sealed partial class ItemTemplateDemo : Page
{
public List<int> Data { get; set; }
public List<int> Data { get; set; }
public List<MyData> Numbers = new List<MyData>();

public ItemTemplateDemo()
{
Data = Enumerable.Range(0, 1000).ToList();

for(int i=0;i<10;i++)
{
Numbers.Add(new MyData(i));
}

this.InitializeComponent();
}

Expand All @@ -26,6 +34,16 @@ private void OnSelectTemplateKey(RecyclingElementFactory sender, SelectTemplateE
}
}

public class MyData
{
public int number;

public MyData(int number)
{
this.number = number;
}
}

public class MySelector: DataTemplateSelector
{
public DataTemplate TemplateOdd { get; set; }
Expand Down
21 changes: 14 additions & 7 deletions test/MUXControlsTestApp/Utilities/RunOnUIThread.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,26 @@ public static void Execute(CoreApplicationView whichView, Action action)
}
}

public async Task WaitForTick()
public static void WaitForTick()
{
var renderingEventFired = new TaskCompletionSource<object>();
var renderingEvent = new ManualResetEvent(false);

EventHandler<object> renderingCallback = (sender, arg) =>
EventHandler<object> renderingHandler = (object sender, object args) =>
{
renderingEventFired.TrySetResult(null);
renderingEvent.Set();
};
CompositionTarget.Rendering += renderingCallback;

await renderingEventFired.Task;
RunOnUIThread.Execute(() =>
{
Windows.UI.Xaml.Media.CompositionTarget.Rendering += renderingHandler;
});

renderingEvent.WaitOne();

CompositionTarget.Rendering -= renderingCallback;
RunOnUIThread.Execute(() =>
{
Windows.UI.Xaml.Media.CompositionTarget.Rendering -= renderingHandler;
});
}

}
Expand Down