Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,85 @@ private async Task<RawBitmap> TakeScreenshot(FrameworkElement SUT)
}
#endif

[TestMethod]
public async Task When_SelectedItem_TwoWay_Binding_Clear()
{
var root = new Grid();

var comboBox = new ComboBox();

root.Children.Add(comboBox);

comboBox.SetBinding(ComboBox.ItemsSourceProperty, new Binding { Path = new("Items") });
comboBox.SetBinding(ComboBox.SelectedItemProperty, new Binding { Path = new("Item"), Mode = BindingMode.TwoWay });

WindowHelper.WindowContent = root;

var dc = new TwoWayBindingClearViewModel();
root.DataContext = dc;

await WindowHelper.WaitForIdle();

dc.Dispose();
root.DataContext = null;

Assert.AreEqual(1, dc.ItemGetCount);
Assert.AreEqual(1, dc.ItemSetCount);
}

public sealed class TwoWayBindingClearViewModel : IDisposable
{
public enum Themes
{
Invalid,
Day,
Night
}

public TwoWayBindingClearViewModel()
{
_item = Items[0];
}

public Themes[] Items { get; } = new Themes[] { Themes.Day, Themes.Night };
private Themes _item;
private bool _isDisposed;
public Themes Item
{
get
{
ItemGetCount++;

if (_isDisposed)
{
return Themes.Invalid;
}

return _item;
}
set
{
ItemSetCount++;

if (_isDisposed)
{
_item = Themes.Invalid;
return;
}

_item = value;
}
}

public int ItemGetCount { get; private set; }
public int ItemSetCount { get; private set; }

public void Dispose()
{
_isDisposed = true;
}
}

public class TwoWayBindingItem : System.ComponentModel.INotifyPropertyChanged
{
private int _selectedNumber;
Expand Down Expand Up @@ -932,6 +1011,8 @@ protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
}

}


#if __IOS__
#region "Helper classes for the iOS Modal Page (UIModalPresentationStyle.pageSheet)"
public partial class MultiFrame : Grid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ public async Task When_Animate()
await target.GetValue(ct, 3);
await Task.Yield();

target.History.Should().BeEquivalentTo(v1, v2, v3);
// v3 is repeated because the target property is not a DependencyProperty
// and no deduplication happens in the binding engine.
target.History.Should().BeEquivalentTo(v1, v2, v3, v3);
sut.State.Should().Be(Timeline.TimelineState.Filling);
}

Expand Down Expand Up @@ -147,7 +149,9 @@ public async Task When_PauseResume()
await target.GetValue(ct, 3);
await Task.Yield();

target.History.Should().BeEquivalentTo(v1, v2, v3);
// v3 is repeated because the target property is not a DependencyProperty
// and no deduplication happens in the binding engine.
target.History.Should().BeEquivalentTo(v1, v2, v3, v3);
sut.State.Should().Be(Timeline.TimelineState.Filling);
}

Expand Down Expand Up @@ -178,7 +182,9 @@ public async Task When_RepeatCount()
await target.GetValue(ct, 9);
await Task.Yield();

target.History.Should().BeEquivalentTo(v1, v2, v3, v1, v2, v3, v1, v2, v3);
// v3 is repeated because the target property is not a DependencyProperty
// and no deduplication happens in the binding engine.
target.History.Should().BeEquivalentTo(v1, v2, v3, v1, v2, v3, v1, v2, v3, v3);
sut.State.Should().Be(Timeline.TimelineState.Filling);
}

Expand Down
89 changes: 76 additions & 13 deletions src/Uno.UI/DataBinding/BindingPath.BindingItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using Uno.Disposables;
using Uno.Foundation.Logging;
using Uno.UI.Xaml.Input;
using Windows.ApplicationModel.Appointments;
using Windows.UI.Xaml;

namespace Uno.UI.DataBinding
Expand All @@ -15,12 +17,10 @@ private sealed class BindingItem : IBindingItem, IDisposable
private delegate void PropertyChangedHandler(object? previousValue, object? newValue, bool shouldRaiseValueChanged);

private ManagedWeakReference? _dataContextWeakStorage;
private Flags _flags;

private readonly SerialDisposable _propertyChanged = new SerialDisposable();
private bool _disposed;
private readonly DependencyPropertyValuePrecedences? _precedence;
private readonly object? _fallbackValue;
private readonly bool _allowPrivateMembers;
private ValueGetterHandler? _valueGetter;
private ValueGetterHandler? _precedenceSpecificGetter;
private ValueGetterHandler? _substituteValueGetter;
Expand All @@ -32,26 +32,35 @@ private sealed class BindingItem : IBindingItem, IDisposable

private Type? _dataContextType;

public BindingItem(BindingItem next, string property, object fallbackValue) :
this(next, property, fallbackValue, null, false)
[Flags]
private enum Flags
{
None = 0,
Disposed = 1 << 0,
AllowPrivateMembers = 1 << 1,
IsDependencyProperty = 1 << 2,
IsDependencyPropertyValueSet = 1 << 3,
}

internal BindingItem(BindingItem? next, string property, object? fallbackValue, DependencyPropertyValuePrecedences? precedence, bool allowPrivateMembers)
public BindingItem(BindingItem next, string property) :
this(next, property, null, false)
{
}

internal BindingItem(BindingItem? next, string property, DependencyPropertyValuePrecedences? precedence, bool allowPrivateMembers)
{
Next = next;
PropertyName = property;
_precedence = precedence;
_fallbackValue = fallbackValue;
_allowPrivateMembers = allowPrivateMembers;
AllowPrivateMembers = allowPrivateMembers;
}

public object? DataContext
{
get => _dataContextWeakStorage?.Target;
set
{
if (!_disposed)
if (!IsDisposed)
{
// Historically, Uno was processing property changes using INPC. Since the inclusion of DependencyObject
// values changes are now filtered by DependencyProperty updates, making equality updates at this location
Expand Down Expand Up @@ -131,7 +140,7 @@ public Type? PropertyType
{
if (DataContext != null)
{
return BindingPropertyHelper.GetPropertyType(_dataContextType!, PropertyName, _allowPrivateMembers);
return BindingPropertyHelper.GetPropertyType(_dataContextType!, PropertyName, AllowPrivateMembers);
}
else
{
Expand Down Expand Up @@ -226,6 +235,7 @@ private void ClearCachedGetters()

if (_dataContextType != currentType && _dataContextType != null)
{
IsDependencyPropertyValueSet = false;
_valueGetter = null;
_precedenceSpecificGetter = null;
_substituteValueGetter = null;
Expand Down Expand Up @@ -312,15 +322,15 @@ private void BuildValueGetter()
{
if (_valueGetter == null && _dataContextType != null)
{
_valueGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, _allowPrivateMembers);
_valueGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, AllowPrivateMembers);
}
}

private void BuildPrecedenceSpecificValueGetter()
{
if (_precedenceSpecificGetter == null && _dataContextType != null)
{
_precedenceSpecificGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, _allowPrivateMembers);
_precedenceSpecificGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, AllowPrivateMembers);
}
}

Expand Down Expand Up @@ -478,10 +488,63 @@ private IDisposable SubscribeToPropertyChanged()

public void Dispose()
{
_disposed = true;
IsDisposed = true;
_propertyChanged.Dispose();
}

private bool IsDisposed
{
get => _flags.HasFlag(Flags.Disposed);
set => SetFlag(value, Flags.Disposed);
}

private bool AllowPrivateMembers
{
get => _flags.HasFlag(Flags.AllowPrivateMembers);
set => SetFlag(value, Flags.AllowPrivateMembers);
}

private bool IsDependencyPropertyValueSet
{
get => _flags.HasFlag(Flags.IsDependencyPropertyValueSet);
set => SetFlag(value, Flags.IsDependencyPropertyValueSet);
}

internal bool IsDependencyProperty
{
get
{
if (!IsDependencyPropertyValueSet)
{
var isDP =
_dataContextType is not null
&& DependencyProperty.GetProperty(_dataContextType!, PropertyName) is not null;

SetFlag(isDP, Flags.IsDependencyProperty);

IsDependencyPropertyValueSet = true;

return isDP;
}
else
{
return _flags.HasFlag(Flags.IsDependencyProperty);
}
}
}

private void SetFlag(bool value, Flags flag)
{
if (!value)
{
_flags &= ~flag;
}
else
{
_flags |= flag;
}
}

/// <summary>
/// Property changed value handler, used to avoid creating a delegate for processing
/// </summary>
Expand Down
12 changes: 9 additions & 3 deletions src/Uno.UI/DataBinding/BindingPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,14 @@ public object? Value
{
if (!_disposed
&& _value != null
&& DependencyObjectStore.AreDifferent(value, _value.GetPrecedenceSpecificValue())
)
&& (
!_value.IsDependencyProperty

// Don't get the source value if we're not accessing a dependency property.
// WinUI does not read the property value before setting the value for a
// non-dependency property source.
|| DependencyObjectStore.AreDifferent(value, _value.GetPrecedenceSpecificValue())
))
{
_value.Value = value;
}
Expand Down Expand Up @@ -428,7 +434,7 @@ private static void TryPrependItem(
}

var itemPath = path.Substring(start, length);
var item = new BindingItem(head, itemPath, fallbackValue, precedence, allowPrivateMembers);
var item = new BindingItem(head, itemPath, precedence, allowPrivateMembers);

head = item;
tail ??= item;
Expand Down