Skip to content

Commit

Permalink
fix(binding): Avoir reading source property on UpdateSource for non-DP
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeromelaban
jeromelaban committed Sep 19, 2023
1 parent d3e22c1 commit 203609d
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 16 deletions.
81 changes: 81 additions & 0 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ComboBox.cs
View file
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
89 changes: 76 additions & 13 deletions src/Uno.UI/DataBinding/BindingPath.BindingItem.cs
View file
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
View file
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

0 comments on commit 203609d

Please sign in to comment.