From d14ff72eab26617ec25334aca055326f8c5f56c0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 16 Sep 2022 08:08:37 +0200 Subject: [PATCH] Unsubscribe inactive bindings. --- .../PropertyStore/BindingEntry.cs | 12 ++-- .../PropertyStore/BindingEntry`1.cs | 12 ++-- .../PropertyStore/UntypedBindingEntry.cs | 16 +++-- src/Avalonia.Base/PropertyStore/ValueStore.cs | 69 +++++++++++++++---- src/Avalonia.Base/StyledElement.cs | 3 +- .../AvaloniaObjectTests_Binding.cs | 26 +++++-- 6 files changed, 98 insertions(+), 40 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntry.cs b/src/Avalonia.Base/PropertyStore/BindingEntry.cs index 2d4eb96736..45040bd245 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntry.cs @@ -8,6 +8,8 @@ namespace Avalonia.PropertyStore IObserver, IDisposable { + private static IDisposable s_Creating = Disposable.Empty; + private static IDisposable s_CreatingQuiet = Disposable.Create(() => { }); private readonly ValueFrame _frame; private IDisposable? _subscription; private bool _hasValue; @@ -72,11 +74,7 @@ namespace Avalonia.PropertyStore { if (_subscription is not null) return; - - // Will only produce a new value when subscription isn't null. - if (produceValue) - _subscription = Disposable.Empty; - + _subscription = produceValue ? s_Creating : s_CreatingQuiet; _subscription = Source.Subscribe(this); } @@ -87,7 +85,7 @@ namespace Avalonia.PropertyStore _hasValue = false; _value = default; - if (_subscription is not null) + if (_subscription is not null && _subscription != s_CreatingQuiet) _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority); } } @@ -118,7 +116,7 @@ namespace Avalonia.PropertyStore _value = typedValue; _hasValue = true; - if (_subscription is not null) + if (_subscription is not null && _subscription != s_CreatingQuiet) _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, typedValue); } } diff --git a/src/Avalonia.Base/PropertyStore/BindingEntry`1.cs b/src/Avalonia.Base/PropertyStore/BindingEntry`1.cs index 224b442378..29c4381464 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntry`1.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntry`1.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reactive.Disposables; using Avalonia.Data; @@ -12,6 +11,8 @@ namespace Avalonia.PropertyStore IObserver>, IDisposable { + private static IDisposable s_Creating = Disposable.Empty; + private static IDisposable s_CreatingQuiet = Disposable.Create(() => { }); private readonly ValueFrame _frame; private readonly object _source; private IDisposable? _subscription; @@ -132,11 +133,11 @@ namespace Avalonia.PropertyStore { _value = value; _hasValue = true; - if (_subscription is not null) + if (_subscription is not null && _subscription != s_CreatingQuiet) _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, value); } } - else if (_subscription is not null) + else if (_subscription is not null && _subscription != s_CreatingQuiet) { _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority); } @@ -153,10 +154,7 @@ namespace Avalonia.PropertyStore if (_subscription is not null) return; - // Will only produce a new value when subscription isn't null. - if (produceValue) - _subscription = Disposable.Empty; - + _subscription = produceValue ? s_Creating : s_CreatingQuiet; _subscription = _source switch { IObservable> bv => bv.Subscribe(this), diff --git a/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs b/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs index e15d455a44..0fb1c61e52 100644 --- a/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs @@ -10,6 +10,8 @@ namespace Avalonia.PropertyStore IObserver, IDisposable { + private static IDisposable s_Creating = Disposable.Empty; + private static IDisposable s_CreatingQuiet = Disposable.Create(() => { }); private readonly ValueFrame _frame; private readonly IObservable _source; private IDisposable? _subscription; @@ -101,7 +103,9 @@ namespace Avalonia.PropertyStore { _hasValue = false; _value = default; - _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority); + + if (_subscription is not null && _subscription != s_CreatingQuiet) + _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority); } } @@ -130,7 +134,9 @@ namespace Avalonia.PropertyStore { _value = typedValue; _hasValue = true; - _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, typedValue); + + if (_subscription is not null && _subscription != s_CreatingQuiet) + _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, typedValue); } } else @@ -150,11 +156,7 @@ namespace Avalonia.PropertyStore { if (_subscription is not null) return; - - // Will only produce a new value when subscription isn't null. - if (produceValue) - _subscription = Disposable.Empty; - + _subscription = produceValue ? s_Creating : s_CreatingQuiet; _subscription = _source.Subscribe(this); } } diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 909283cbe5..9786af2490 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -7,6 +7,7 @@ using Avalonia.Data; using Avalonia.Diagnostics; using Avalonia.Logging; using Avalonia.Utilities; +using JetBrains.Annotations; namespace Avalonia.PropertyStore { @@ -56,11 +57,14 @@ namespace Avalonia.PropertyStore else { var effective = GetEffectiveValue(property); - var frame = GetOrCreateImmediateValueFrame(property, priority); + var frame = GetOrCreateImmediateValueFrame(property, priority, out var frameIndex); var result = frame.AddBinding(property, source); if (effective is null || priority <= effective.Priority) + { result.Start(); + UnsubscribeInactiveValues(frameIndex, property); + } return result; } @@ -83,11 +87,14 @@ namespace Avalonia.PropertyStore else { var effective = GetEffectiveValue(property); - var frame = GetOrCreateImmediateValueFrame(property, priority); + var frame = GetOrCreateImmediateValueFrame(property, priority, out var frameIndex); var result = frame.AddBinding(property, source); if (effective is null || priority <= effective.Priority) + { result.Start(); + UnsubscribeInactiveValues(frameIndex, property); + } return result; } @@ -110,11 +117,14 @@ namespace Avalonia.PropertyStore else { var effective = GetEffectiveValue(property); - var frame = GetOrCreateImmediateValueFrame(property, priority); + var frame = GetOrCreateImmediateValueFrame(property, priority, out var frameIndex); var result = frame.AddBinding(property, source); if (effective is null || priority <= effective.Priority) + { result.Start(); + UnsubscribeInactiveValues(frameIndex, property); + } return result; } @@ -170,8 +180,9 @@ namespace Avalonia.PropertyStore if (priority != BindingPriority.LocalValue) { - var frame = GetOrCreateImmediateValueFrame(property, priority); + var frame = GetOrCreateImmediateValueFrame(property, priority, out var frameIndex); result = frame.AddValue(property, value); + UnsubscribeInactiveValues(frameIndex, property); } if (TryGetEffectiveValue(property, out var existing)) @@ -615,7 +626,7 @@ namespace Avalonia.PropertyStore null); } - private void InsertFrame(ValueFrame frame) + private int InsertFrame(ValueFrame frame) { // Uncomment this line when #8549 is fixed. //Debug.Assert(!_frames.Contains(frame)); @@ -624,11 +635,13 @@ namespace Avalonia.PropertyStore _frames.Insert(index, frame); ++_frameGeneration; frame.SetOwner(this); + return index; } private ImmediateValueFrame GetOrCreateImmediateValueFrame( AvaloniaProperty property, - BindingPriority priority) + BindingPriority priority, + out int frameIndex) { Debug.Assert(priority != BindingPriority.LocalValue); @@ -637,10 +650,13 @@ namespace Avalonia.PropertyStore if (index > 0 && _frames[index - 1] is ImmediateValueFrame f && f.Priority == priority && !f.Contains(property)) + { + frameIndex = index - 1; return f; + } var result = new ImmediateValueFrame(priority); - InsertFrame(result); + frameIndex = InsertFrame(result); return result; } @@ -765,13 +781,11 @@ namespace Avalonia.PropertyStore for (var i = _frames.Count - 1; i >= 0; --i) { var frame = _frames[i]; - - if (!frame.IsActive) - continue; - var priority = frame.Priority; - if (frame.TryGetEntry(property, out var entry) && entry.HasValue) + if (frame.TryGetEntry(property, out var entry) && + frame.IsActive && + entry.HasValue) { if (current is not null) { @@ -785,12 +799,16 @@ namespace Avalonia.PropertyStore } } + if (generation != _frameGeneration) + goto restart; + if (current is not null && current.Priority < BindingPriority.Unset && current.BasePriority < BindingPriority.Unset) + { + UnsubscribeInactiveValues(i, property); return; - if (generation != _frameGeneration) - goto restart; + } } if (current?.Priority == BindingPriority.Unset) @@ -898,6 +916,29 @@ namespace Avalonia.PropertyStore } } + private void UnsubscribeInactiveValues(int activeFrameIndex, AvaloniaProperty property) + { + var foundBaseValue = _frames[activeFrameIndex].Priority != BindingPriority.Animation; + + for (var i = activeFrameIndex - 1; i >= 0; --i) + { + var frame = _frames[i]; + + if (!foundBaseValue && frame.Priority > BindingPriority.Animation) + { + foundBaseValue = true; + continue; + } + + if ((foundBaseValue || frame.Priority <= BindingPriority.Animation) && + frame.TryGetEntry(property, out var entry) && + frame.IsActive) + { + entry.Unsubscribe(); + } + } + } + private bool TryGetEffectiveValue( AvaloniaProperty property, [NotNullWhen(true)] out EffectiveValue? value) diff --git a/src/Avalonia.Base/StyledElement.cs b/src/Avalonia.Base/StyledElement.cs index 5093826bfd..bba9685ed8 100644 --- a/src/Avalonia.Base/StyledElement.cs +++ b/src/Avalonia.Base/StyledElement.cs @@ -352,6 +352,7 @@ namespace Avalonia if (_initCount == 0 && !_styled) { var styler = AvaloniaLocator.Current.GetService(); + var hasPromotedTheme = _hasPromotedTheme; if (styler is object) { @@ -368,7 +369,7 @@ namespace Avalonia } } - if (_hasPromotedTheme) + if (hasPromotedTheme) { _hasPromotedTheme = false; ClearValue(ThemeProperty); diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs index 5319014901..6ecd1d6cfc 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs @@ -525,7 +525,6 @@ namespace Avalonia.Base.UnitTests } [Theory] - [InlineData(BindingPriority.LocalValue)] [InlineData(BindingPriority.Style)] public void Observable_Is_Unsubscribed_When_New_Binding_Of_Higher_Priority_Is_Added(BindingPriority priority) { @@ -564,9 +563,7 @@ namespace Avalonia.Base.UnitTests } [Theory] - [InlineData(BindingPriority.LocalValue)] [InlineData(BindingPriority.Style)] - [InlineData(BindingPriority.Animation)] public void Observable_Is_Unsubscribed_When_New_Value_Of_Higher_Priority_Is_Added(BindingPriority priority) { var scheduler = new TestScheduler(); @@ -578,10 +575,31 @@ namespace Avalonia.Base.UnitTests Assert.Equal(Subscription.Infinite, source.Subscriptions[0].Unsubscribe); target.SetValue(Class1.FooProperty, "foo", priority - 1); - Assert.Equal(0, source.Subscriptions.Count); + Assert.Equal(1, source.Subscriptions.Count); Assert.Equal(0, source.Subscriptions[0].Unsubscribe); } + [Theory] + [InlineData(BindingPriority.LocalValue)] + [InlineData(BindingPriority.Style)] + public void Observable_Is_Not_Unsubscribed_When_Animation_Binding_Is_Added(BindingPriority priority) + { + var scheduler = new TestScheduler(); + var source1 = scheduler.CreateColdObservable>(); + var source2 = scheduler.CreateColdObservable>(); + var target = new Class1(); + + target.Bind(Class1.FooProperty, source1, priority); + Assert.Equal(1, source1.Subscriptions.Count); + Assert.Equal(Subscription.Infinite, source1.Subscriptions[0].Unsubscribe); + + target.Bind(Class1.FooProperty, source2, BindingPriority.Animation); + Assert.Equal(1, source2.Subscriptions.Count); + Assert.Equal(Subscription.Infinite, source2.Subscriptions[0].Unsubscribe); + Assert.Equal(1, source1.Subscriptions.Count); + Assert.Equal(Subscription.Infinite, source1.Subscriptions[0].Unsubscribe); + } + [Fact] public void LocalValue_Binding_Is_Not_Unsubscribed_When_LocalValue_Is_Set() {