Browse Source

Prevent leak when using TemplateBinding from a Setter.

When a `TemplateBinding` is used as a `Setter.Value`, then we need to make sure we don't use the `TemplateBinding` itself as the binding because this can cause a memory leak.

Replace `IRequiresTemplateInSetter` with `ISetterValue` which can be used to require a template in the setter for `Control` and can also be used to notify `TemplateBinding`  that it's in a setter and so should always clone itself.

Fixes #2420
pull/2438/head
Steven Kirk 7 years ago
parent
commit
1cd900c856
  1. 10
      src/Avalonia.Controls/Control.cs
  2. 10
      src/Avalonia.Styling/Styling/IRequiresTemplateInSetter.cs
  3. 13
      src/Avalonia.Styling/Styling/ISetterValue.cs
  4. 8
      src/Avalonia.Styling/Styling/Setter.cs
  5. 17
      src/Markup/Avalonia.Markup/Data/TemplateBinding.cs
  6. 14
      tests/Avalonia.Styling.UnitTests/SetterTests.cs

10
src/Avalonia.Controls/Control.cs

@ -1,6 +1,7 @@
// Copyright (c) The Avalonia Project. All rights reserved.
// Licensed under the MIT license. See licence.md file in the project root for full license information.
using System;
using System.ComponentModel;
using Avalonia.Controls.Primitives;
using Avalonia.Controls.Templates;
@ -20,7 +21,7 @@ namespace Avalonia.Controls
///
/// - A <see cref="Tag"/> property to allow user-defined data to be attached to the control.
/// </remarks>
public class Control : InputElement, IControl, INamed, ISupportInitialize, IVisualBrushInitialize, IRequiresTemplateInSetter
public class Control : InputElement, IControl, INamed, ISupportInitialize, IVisualBrushInitialize, ISetterValue
{
/// <summary>
/// Defines the <see cref="FocusAdorner"/> property.
@ -90,6 +91,13 @@ namespace Avalonia.Controls
/// <inheritdoc/>
bool IDataTemplateHost.IsDataTemplatesInitialized => _dataTemplates != null;
/// <inheritdoc/>
void ISetterValue.Initialize(ISetter setter)
{
throw new InvalidOperationException(
"Cannot use a control as a Setter value. Wrap the control in a <Template>.");
}
/// <inheritdoc/>
void IVisualBrushInitialize.EnsureInitialized()
{

10
src/Avalonia.Styling/Styling/IRequiresTemplateInSetter.cs

@ -1,10 +0,0 @@
namespace Avalonia.Styling
{
/// <summary>
/// This is an interface for advanced scenarios to assist users in correct style development.
/// You as a user will not need to use this interface directly.
/// </summary>
public interface IRequiresTemplateInSetter
{
}
}

13
src/Avalonia.Styling/Styling/ISetterValue.cs

@ -0,0 +1,13 @@
namespace Avalonia.Styling
{
/// <summary>
/// Customizes the behavior of a class when added as a value to an <see cref="ISetter"/>.
/// </summary>
public interface ISetterValue
{
/// <summary>
/// Notifies that the object has been added as a setter value.
/// </summary>
void Initialize(ISetter setter);
}
}

8
src/Avalonia.Styling/Styling/Setter.cs

@ -65,13 +65,7 @@ namespace Avalonia.Styling
set
{
if (value is IRequiresTemplateInSetter)
{
throw new ArgumentException(
"Cannot assign a control to Setter.Value. Wrap the control in a <Template>.",
nameof(value));
}
(value as ISetterValue)?.Initialize(this);
_value = value;
}
}

17
src/Markup/Avalonia.Markup/Data/TemplateBinding.cs

@ -3,6 +3,7 @@ using System.Globalization;
using System.Reactive.Subjects;
using Avalonia.Data.Converters;
using Avalonia.Reactive;
using Avalonia.Styling;
namespace Avalonia.Data
{
@ -12,8 +13,10 @@ namespace Avalonia.Data
public class TemplateBinding : SingleSubscriberObservableBase<object>,
IBinding,
IDescription,
ISubject<object>
ISubject<object>,
ISetterValue
{
private bool _isSetterValue;
private IStyledElement _target;
private Type _targetType;
@ -35,10 +38,11 @@ namespace Avalonia.Data
{
// Usually each `TemplateBinding` will only be instantiated once; in this case we can
// use the `TemplateBinding` object itself as the instanced binding in order to save
// allocating a new object. If the binding *is* instantiated more than once (which can
// happen if it appears in a `Setter` for example, then just make a clone and instantiate
// that.
if (_target == null)
// allocating a new object.
//
// If the binding appears in a `Setter`, then make a clone and instantiate that because
// because the setter can outlive the control and cause a leak.
if (_target == null && !_isSetterValue)
{
_target = (IStyledElement)target;
_targetType = targetProperty?.PropertyType;
@ -106,6 +110,9 @@ namespace Avalonia.Data
}
}
/// <inheritdoc/>
void ISetterValue.Initialize(ISetter setter) => _isSetterValue = true;
protected override void Subscribed()
{
TemplatedParentChanged();

14
tests/Avalonia.Styling.UnitTests/SetterTests.cs

@ -1,17 +1,15 @@
// Copyright (c) The Avalonia Project. All rights reserved.
// Licensed under the MIT license. See licence.md file in the project root for full license information.
using System;
using System.Globalization;
using System.Reactive.Subjects;
using Moq;
using Avalonia.Controls;
using Avalonia.Data;
using Xunit;
using System;
using Avalonia.Controls.Templates;
using Avalonia.Markup.Data;
using Avalonia.Markup;
using System.Globalization;
using Avalonia.Data;
using Avalonia.Data.Converters;
using Moq;
using Xunit;
namespace Avalonia.Styling.UnitTests
{
@ -22,7 +20,7 @@ namespace Avalonia.Styling.UnitTests
{
var target = new Setter();
Assert.Throws<ArgumentException>(() => target.Value = new Border());
Assert.Throws<InvalidOperationException>(() => target.Value = new Border());
}
[Fact]

Loading…
Cancel
Save