Skip to content

Commit 8635052

Browse files
jevansaksmarcelwgn
jevansaks
and
marcelwgn
authored
Add workaround so ItemsRepeater doesn't leak the result of DataTemplate::Load (#3068)
* Add test code and fix * CR feedback * More CR feedback Co-authored-by: Marcel Wagner <[email protected]>
1 parent 3ff3437 commit 8635052

File tree

8 files changed

+184
-10
lines changed

8 files changed

+184
-10
lines changed

‎dev/Repeater/APITests/RepeaterTests.cs

+53
Original file line numberDiff line numberDiff line change
@@ -609,5 +609,58 @@ public void VerifyUIElementsInItemsSource()
609609
}
610610
});
611611
}
612+
613+
[TestMethod]
614+
public void VerifyRepeaterDoesNotLeakItemContainers()
615+
{
616+
ObservableCollection<int> items = new ObservableCollection<int>();
617+
for(int i=0;i<10;i++)
618+
{
619+
items.Add(i);
620+
}
621+
622+
ItemsRepeater repeater = null;
623+
624+
RunOnUIThread.Execute(() =>
625+
{
626+
var template = (DataTemplate)XamlReader.Load("<DataTemplate xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation' "
627+
+ "xmlns:local='using:MUXControlsTestApp.Samples'>"
628+
+ "<local:DisposableUserControl Number='{Binding}'/>"
629+
+ "</DataTemplate>");
630+
Verify.IsNotNull(template);
631+
Verify.AreEqual(0, MUXControlsTestApp.Samples.DisposableUserControl.OpenItems, "Verify we start with 0 DisposableUserControl");
632+
633+
repeater = new ItemsRepeater() {
634+
ItemsSource = items,
635+
ItemTemplate = template,
636+
VerticalAlignment = VerticalAlignment.Top,
637+
HorizontalAlignment = HorizontalAlignment.Left
638+
};
639+
640+
Content = repeater;
641+
642+
});
643+
644+
IdleSynchronizer.Wait();
645+
646+
RunOnUIThread.Execute(() =>
647+
{
648+
649+
Verify.IsGreaterThanOrEqual(MUXControlsTestApp.Samples.DisposableUserControl.OpenItems, 10, "Verify we created at least 10 DisposableUserControl");
650+
651+
// Clear out the repeater and make sure everything gets cleaned up.
652+
Content = null;
653+
repeater = null;
654+
});
655+
656+
IdleSynchronizer.Wait();
657+
658+
GC.Collect();
659+
GC.WaitForPendingFinalizers();
660+
GC.Collect();
661+
662+
Verify.AreEqual(0, MUXControlsTestApp.Samples.DisposableUserControl.OpenItems, "Verify we cleaned up all the DisposableUserControl that were created");
663+
}
664+
612665
}
613666
}

‎dev/Repeater/ItemsRepeater.cpp

+11-1
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,17 @@ void ItemsRepeater::OnItemTemplateChanged(const winrt::IElementFactory& oldValue
621621
if (auto dataTemplate = newValue.try_as<winrt::DataTemplate>())
622622
{
623623
m_itemTemplateWrapper = winrt::make<ItemTemplateWrapper>(dataTemplate);
624-
if (!dataTemplate.LoadContent().as<winrt::FrameworkElement>()) {
624+
if (auto content = dataTemplate.LoadContent().as<winrt::FrameworkElement>())
625+
{
626+
// Due to bug https://github.com/microsoft/microsoft-ui-xaml/issues/3057, we need to get the framework
627+
// to take ownership of the extra implicit ref that was returned by LoadContent. The simplest way to do
628+
// this is to add it to a Children collection and immediately remove it.
629+
auto children = Children();
630+
children.Append(content);
631+
children.RemoveAtEnd();
632+
}
633+
else
634+
{
625635
// We have a DataTemplate which is empty, so we need to set it to true
626636
m_isItemTemplateEmpty = true;
627637
}

‎dev/Repeater/TestUI/Repeater_TestUI.projitems

+7
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
<SubType>Designer</SubType>
2020
<Generator>MSBuild:Compile</Generator>
2121
</Page>
22+
<Page Include="$(MSBuildThisFileDirectory)Samples\ItemTemplateSamples\DisposableUserControl.xaml">
23+
<SubType>Designer</SubType>
24+
<Generator>MSBuild:Compile</Generator>
25+
</Page>
2226
<Page Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\VirtualLayoutPages\ActivityFeedSamplePage.xaml">
2327
<SubType>Designer</SubType>
2428
<Generator>MSBuild:Compile</Generator>
@@ -112,6 +116,9 @@
112116
<Compile Include="$(MSBuildThisFileDirectory)Samples\ItemsSourceSamples\SortingAndFilteringPage.xaml.cs">
113117
<DependentUpon>SortingAndFilteringPage.xaml</DependentUpon>
114118
</Compile>
119+
<Compile Include="$(MSBuildThisFileDirectory)Samples\ItemTemplateSamples\DisposableUserControl.xaml.cs">
120+
<DependentUpon>DisposableUserControl.xaml</DependentUpon>
121+
</Compile>
115122
<Compile Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\VirtualLayoutPages\ActivityFeedLayout.cs" />
116123
<Compile Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\VirtualLayoutPages\ActivityFeedSamplePage.xaml.cs">
117124
<DependentUpon>ActivityFeedSamplePage.xaml</DependentUpon>

‎dev/Repeater/TestUI/Samples/ItemTemplateSamples/DisposableUserControl.xaml

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<UserControl
2+
x:Class="MUXControlsTestApp.Samples.DisposableUserControl"
3+
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
4+
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
5+
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
6+
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
7+
mc:Ignorable="d"
8+
d:DesignHeight="300"
9+
d:DesignWidth="400">
10+
11+
<Grid>
12+
<TextBlock Text="{x:Bind Number}"/>
13+
</Grid>
14+
</UserControl>

‎dev/Repeater/TestUI/Samples/ItemTemplateSamples/DisposableUserControl.xaml.cs

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.IO;
4+
using System.Linq;
5+
using System.Runtime.InteropServices.WindowsRuntime;
6+
using System.Threading;
7+
using Windows.Foundation;
8+
using Windows.Foundation.Collections;
9+
using Windows.UI.Xaml;
10+
using Windows.UI.Xaml.Controls;
11+
using Windows.UI.Xaml.Controls.Primitives;
12+
using Windows.UI.Xaml.Data;
13+
using Windows.UI.Xaml.Input;
14+
using Windows.UI.Xaml.Media;
15+
using Windows.UI.Xaml.Navigation;
16+
17+
// The User Control item template is documented at https://go.microsoft.com/fwlink/?LinkId=234236
18+
19+
namespace MUXControlsTestApp.Samples
20+
{
21+
public sealed partial class DisposableUserControl : UserControl
22+
{
23+
24+
public int Number
25+
{
26+
get { return (int)GetValue(NumberProperty); }
27+
set { SetValue(NumberProperty, value); }
28+
}
29+
30+
public static readonly DependencyProperty NumberProperty =
31+
DependencyProperty.Register("Number", typeof(int), typeof(DisposableUserControl), new PropertyMetadata(-1));
32+
33+
34+
public static int OpenItems { get { return _counter; } }
35+
private static int _counter = 0;
36+
37+
public DisposableUserControl()
38+
{
39+
Interlocked.Increment(ref _counter);
40+
this.InitializeComponent();
41+
}
42+
43+
~DisposableUserControl()
44+
{
45+
Interlocked.Decrement(ref _counter);
46+
}
47+
}
48+
}

‎dev/Repeater/TestUI/Samples/ItemTemplateSamples/ItemTemplateDemo.xaml

+18-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
<ColumnDefinition Width="auto" />
1717
<ColumnDefinition Width="auto" />
1818
<ColumnDefinition Width="auto" />
19+
<ColumnDefinition Width="auto" />
1920
</Grid.ColumnDefinitions>
2021
<StackPanel>
2122
<TextBlock>DataTemplate Sample:</TextBlock>
@@ -80,7 +81,7 @@
8081
<TextBlock>ElementFactory Multiple Template Sample:</TextBlock>
8182
<controls:ItemsRepeaterScrollHost Height="400" Width="150">
8283
<ScrollViewer>
83-
<controls:ItemsRepeater x:Name="repeater3"
84+
<controls:ItemsRepeater x:Name="repeater4"
8485
ItemsSource="{x:Bind Data}"
8586
Layout="{StaticResource stackLayout}">
8687
<controls:ItemsRepeater.ItemTemplate>
@@ -99,5 +100,21 @@
99100
</ScrollViewer>
100101
</controls:ItemsRepeaterScrollHost>
101102
</StackPanel>
103+
<StackPanel Grid.Column="4">
104+
<TextBlock>DataTemplate clear count checking:</TextBlock>
105+
<controls:ItemsRepeaterScrollHost Height="400" Width="150">
106+
<ScrollViewer>
107+
<controls:ItemsRepeater x:Name="repeater3"
108+
ItemsSource="{x:Bind Numbers}"
109+
Layout="{StaticResource stackLayout}">
110+
<controls:ItemsRepeater.ItemTemplate>
111+
<DataTemplate x:DataType="local:MyData" >
112+
<local:DisposableUserControl Number="{x:Bind number}"/>
113+
</DataTemplate>
114+
</controls:ItemsRepeater.ItemTemplate>
115+
</controls:ItemsRepeater>
116+
</ScrollViewer>
117+
</controls:ItemsRepeaterScrollHost>
118+
</StackPanel>
102119
</Grid>
103120
</Page>

‎dev/Repeater/TestUI/Samples/ItemTemplateSamples/ItemTemplateDemo.xaml.cs

+19-1
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,18 @@ namespace MUXControlsTestApp.Samples
1313
{
1414
public sealed partial class ItemTemplateDemo : Page
1515
{
16-
public List<int> Data { get; set; }
16+
public List<int> Data { get; set; }
17+
public List<MyData> Numbers = new List<MyData>();
18+
1719
public ItemTemplateDemo()
1820
{
1921
Data = Enumerable.Range(0, 1000).ToList();
22+
23+
for(int i=0;i<10;i++)
24+
{
25+
Numbers.Add(new MyData(i));
26+
}
27+
2028
this.InitializeComponent();
2129
}
2230

@@ -26,6 +34,16 @@ private void OnSelectTemplateKey(RecyclingElementFactory sender, SelectTemplateE
2634
}
2735
}
2836

37+
public class MyData
38+
{
39+
public int number;
40+
41+
public MyData(int number)
42+
{
43+
this.number = number;
44+
}
45+
}
46+
2947
public class MySelector: DataTemplateSelector
3048
{
3149
public DataTemplate TemplateOdd { get; set; }

‎test/MUXControlsTestApp/Utilities/RunOnUIThread.cs

+14-7
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,26 @@ public static void Execute(CoreApplicationView whichView, Action action)
9090
}
9191
}
9292

93-
public async Task WaitForTick()
93+
public static void WaitForTick()
9494
{
95-
var renderingEventFired = new TaskCompletionSource<object>();
95+
var renderingEvent = new ManualResetEvent(false);
9696

97-
EventHandler<object> renderingCallback = (sender, arg) =>
97+
EventHandler<object> renderingHandler = (object sender, object args) =>
9898
{
99-
renderingEventFired.TrySetResult(null);
99+
renderingEvent.Set();
100100
};
101-
CompositionTarget.Rendering += renderingCallback;
102101

103-
await renderingEventFired.Task;
102+
RunOnUIThread.Execute(() =>
103+
{
104+
CompositionTarget.Rendering += renderingHandler;
105+
});
106+
107+
renderingEvent.WaitOne();
104108

105-
CompositionTarget.Rendering -= renderingCallback;
109+
RunOnUIThread.Execute(() =>
110+
{
111+
CompositionTarget.Rendering -= renderingHandler;
112+
});
106113
}
107114

108115
}

0 commit comments

Comments
 (0)