[C] do not reapply a Binding with a Source when the Context changes (#983)

* [C] do not reapply a Binding with a Source when the Context changes

* apply the same to typedBindings

* [C] Use a flag to Apply()

* [C] Remove 2 other unnecessary Binding refreshs

diff --git a/Xamarin.Forms.Core.UnitTests/BindingUnitTests.cs b/Xamarin.Forms.Core.UnitTests/BindingUnitTests.cs
index 0617d8a..2dac4ed 100644 (file)
--- a/Xamarin.Forms.Core.UnitTests/BindingUnitTests.cs
+++ b/Xamarin.Forms.Core.UnitTests/BindingUnitTests.cs
@@ -2115,5 +2115,48 @@ namespace Xamarin.Forms.Core.UnitTests
 
                        Assert.AreEqual ("Baz", label.Text);
                }
+
+               class VM57081
+               {
+                       string _foo;
+                       public string Foo
+                       {
+                               get {
+                                       Count++;
+                                       return _foo;
+                               }
+                               set { _foo = value; }
+                       }
+
+                       public int Count { get; set; }
+               }
+
+               [Test]
+               // https://bugzilla.xamarin.com/show_bug.cgi?id=57081
+               public void BindingWithSourceNotReappliedWhenBindingContextIsChanged()
+               {
+                       var bindable = new MockBindable();
+                       var model = new VM57081();
+                       var bp = BindableProperty.Create("foo", typeof(string), typeof(MockBindable), null);
+                       Assume.That(model.Count, Is.EqualTo(0));
+                       bindable.SetBinding(bp, new Binding { Path = "Foo", Source = model });
+                       Assume.That(model.Count, Is.EqualTo(1));
+                       bindable.BindingContext = new object();
+                       Assert.That(model.Count, Is.EqualTo(1));
+               }
+
+               [Test]
+               // https://bugzilla.xamarin.com/show_bug.cgi?id=57081
+               public void BindingWithSourceNotReappliedWhenParented()
+               {
+                       var view = new ContentView();
+                       var model = new VM57081();
+                       Assume.That(model.Count, Is.EqualTo(0));
+                       view.SetBinding(BindableObject.BindingContextProperty, new Binding { Path = "Foo", Source = model });
+                       Assume.That(model.Count, Is.EqualTo(1));
+                       var parent = new ContentView { BindingContext = new object() };
+                       parent.Content = view;
+                       Assert.That(model.Count, Is.EqualTo(1));
+               }
        }
 }
\ No newline at end of file

diff --git a/Xamarin.Forms.Core/BindableObject.cs b/Xamarin.Forms.Core/BindableObject.cs
index 9a77411..a6ac084 100644 (file)
--- a/Xamarin.Forms.Core/BindableObject.cs
+++ b/Xamarin.Forms.Core/BindableObject.cs
@@ -117,13 +117,13 @@ namespace Xamarin.Forms
                                bindable._inheritedContext = value;
                        }
 
-                       bindable.ApplyBindings();
+                       bindable.ApplyBindings(skipBindingContext:false, fromBindingContextChanged:true);
                        bindable.OnBindingContextChanged();
                }
 
                protected void ApplyBindings()
                {
-                       ApplyBindings(false);
+                       ApplyBindings(skipBindingContext: false, fromBindingContextChanged: false);
                }
 
                protected virtual void OnBindingContextChanged()
@@ -406,7 +406,7 @@ namespace Xamarin.Forms
                        }
                }
 
-               void ApplyBindings(bool skipBindingContext)
+               internal void ApplyBindings(bool skipBindingContext, bool fromBindingContextChanged)
                {
                        var prop = _properties.ToArray();
                        for (int i = 0, propLength = prop.Length; i < propLength; i++) {
@@ -418,8 +418,8 @@ namespace Xamarin.Forms
                                if (skipBindingContext && ReferenceEquals(context.Property, BindingContextProperty))
                                        continue;
 
-                               binding.Unapply();
-                               binding.Apply(BindingContext, this, context.Property);
+                               binding.Unapply(fromBindingContextChanged: fromBindingContextChanged);
+                               binding.Apply(BindingContext, this, context.Property, fromBindingContextChanged: fromBindingContextChanged);
                        }
                }
 
@@ -438,7 +438,7 @@ namespace Xamarin.Forms
                static void BindingContextPropertyChanged(BindableObject bindable, object oldvalue, object newvalue)
                {
                        bindable._inheritedContext = null;
-                       bindable.ApplyBindings(true);
+                       bindable.ApplyBindings(skipBindingContext: true, fromBindingContextChanged:true);
                        bindable.OnBindingContextChanged();
                }
 

diff --git a/Xamarin.Forms.Core/Binding.cs b/Xamarin.Forms.Core/Binding.cs
index e980fc9..ad4bb34 100644 (file)
--- a/Xamarin.Forms.Core/Binding.cs
+++ b/Xamarin.Forms.Core/Binding.cs
@@ -114,10 +114,15 @@ namespace Xamarin.Forms
                        _expression.Apply(fromTarget);
                }
 
-               internal override void Apply(object newContext, BindableObject bindObj, BindableProperty targetProperty)
+               internal override void Apply(object newContext, BindableObject bindObj, BindableProperty targetProperty, bool fromBindingContextChanged = false)
                {
                        object src = _source;
-                       base.Apply(src ?? newContext, bindObj, targetProperty);
+                       var isApplied = IsApplied;
+
+                       base.Apply(src ?? newContext, bindObj, targetProperty, fromBindingContextChanged: fromBindingContextChanged);
+
+                       if (src != null && isApplied && fromBindingContextChanged)
+                               return;
 
                        object bindingContext = src ?? Context ?? newContext;
                        if (_expression == null && bindingContext != null)
@@ -147,9 +152,12 @@ namespace Xamarin.Forms
                        return base.GetTargetValue(value, sourcePropertyType);
                }
 
-               internal override void Unapply()
+               internal override void Unapply(bool fromBindingContextChanged = false)
                {
-                       base.Unapply();
+                       if (Source != null && fromBindingContextChanged && IsApplied)
+                               return;
+                       
+                       base.Unapply(fromBindingContextChanged: fromBindingContextChanged);
 
                        if (_expression != null)
                                _expression.Unapply();

diff --git a/Xamarin.Forms.Core/BindingBase.cs b/Xamarin.Forms.Core/BindingBase.cs
index fd11ac6..154f5a0 100644 (file)
--- a/Xamarin.Forms.Core/BindingBase.cs
+++ b/Xamarin.Forms.Core/BindingBase.cs
@@ -75,7 +75,7 @@ namespace Xamarin.Forms
                        IsApplied = true;
                }
 
-               internal virtual void Apply(object context, BindableObject bindObj, BindableProperty targetProperty)
+               internal virtual void Apply(object context, BindableObject bindObj, BindableProperty targetProperty, bool fromBindingContextChanged = false)
                {
                        IsApplied = true;
                }
@@ -103,7 +103,7 @@ namespace Xamarin.Forms
                        return SynchronizedCollections.TryGetValue(collection, out synchronizationContext);
                }
 
-               internal virtual void Unapply()
+               internal virtual void Unapply(bool fromBindingContextChanged = false)
                {
                        IsApplied = false;
                }

diff --git a/Xamarin.Forms.Core/Element.cs b/Xamarin.Forms.Core/Element.cs
index d8fdcdf..f2ae4a4 100644 (file)
--- a/Xamarin.Forms.Core/Element.cs
+++ b/Xamarin.Forms.Core/Element.cs
@@ -369,7 +369,7 @@ namespace Xamarin.Forms
                        if (Platform != null)
                                child.Platform = Platform;
 
-                       child.ApplyBindings();
+                       child.ApplyBindings(skipBindingContext: false, fromBindingContextChanged:true);
 
                        if (ChildAdded != null)
                                ChildAdded(this, new ElementEventArgs(child));

diff --git a/Xamarin.Forms.Core/TemplateBinding.cs b/Xamarin.Forms.Core/TemplateBinding.cs
index b01f067..9b18f85 100644 (file)
--- a/Xamarin.Forms.Core/TemplateBinding.cs
+++ b/Xamarin.Forms.Core/TemplateBinding.cs
@@ -75,13 +75,13 @@ namespace Xamarin.Forms
                        _expression.Apply(fromTarget);
                }
 
-               internal override async void Apply(object newContext, BindableObject bindObj, BindableProperty targetProperty)
+               internal override async void Apply(object newContext, BindableObject bindObj, BindableProperty targetProperty, bool fromBindingContextChanged = false)
                {
                        var view = bindObj as Element;
                        if (view == null)
                                throw new InvalidOperationException();
 
-                       base.Apply(newContext, bindObj, targetProperty);
+                       base.Apply(newContext, bindObj, targetProperty, fromBindingContextChanged);
 
                        Element templatedParent = await TemplateUtilities.FindTemplatedParentAsync(view);
                        ApplyInner(templatedParent, bindObj, targetProperty);
@@ -108,9 +108,9 @@ namespace Xamarin.Forms
                        return base.GetTargetValue(value, sourcePropertyType);
                }
 
-               internal override void Unapply()
+               internal override void Unapply(bool fromBindingContextChanged = false)
                {
-                       base.Unapply();
+                       base.Unapply(fromBindingContextChanged: fromBindingContextChanged);
 
                        if (_expression != null)
                                _expression.Unapply();

diff --git a/Xamarin.Forms.Core/TypedBinding.cs b/Xamarin.Forms.Core/TypedBinding.cs
index 51c2aec..8c2e9f6 100644 (file)
--- a/Xamarin.Forms.Core/TypedBinding.cs
+++ b/Xamarin.Forms.Core/TypedBinding.cs
@@ -102,14 +102,18 @@ namespace Xamarin.Forms.Internals
                }
 
                // Applies the binding to a new source or target.
-               internal override void Apply(object context, BindableObject bindObj, BindableProperty targetProperty)
+               internal override void Apply(object context, BindableObject bindObj, BindableProperty targetProperty, bool fromBindingContextChanged = false)
                {
                        _targetProperty = targetProperty;
                        var source = Source ?? Context ?? context;
+                       var isApplied = IsApplied;
 
-#if (!DO_NOT_CHECK_FOR_BINDING_REUSE)
-                       base.Apply(source, bindObj, targetProperty);
+                       if (Source != null && isApplied && fromBindingContextChanged)
+                               return;
 
+                       base.Apply(source, bindObj, targetProperty, fromBindingContextChanged);
+                       
+#if (!DO_NOT_CHECK_FOR_BINDING_REUSE)
                        BindableObject prevTarget;
                        if (_weakTarget.TryGetTarget(out prevTarget) && !ReferenceEquals(prevTarget, bindObj))
                                throw new InvalidOperationException("Binding instances can not be reused");
@@ -162,10 +166,13 @@ namespace Xamarin.Forms.Internals
                        return value;
                }
 
-               internal override void Unapply()
+               internal override void Unapply(bool fromBindingContextChanged = false)
                {
+                       if (Source != null && fromBindingContextChanged && IsApplied)
+                               return;
+
 #if (!DO_NOT_CHECK_FOR_BINDING_REUSE)
-                       base.Unapply();
+                       base.Unapply(fromBindingContextChanged:fromBindingContextChanged);
 #endif
                        if (_handlers != null)
                                Unsubscribe();

diff --git a/Xamarin.Forms.Pages/DataSourceBinding.cs b/Xamarin.Forms.Pages/DataSourceBinding.cs
index 7491ca3..08c3a87 100644 (file)
--- a/Xamarin.Forms.Pages/DataSourceBinding.cs
+++ b/Xamarin.Forms.Pages/DataSourceBinding.cs
@@ -77,13 +77,13 @@ namespace Xamarin.Forms.Pages
                        _expression.Apply(fromTarget);
                }
 
-               internal override async void Apply(object newContext, BindableObject bindObj, BindableProperty targetProperty)
+               internal override async void Apply(object newContext, BindableObject bindObj, BindableProperty targetProperty, bool fromBindingContextChanged)
                {
                        var view = bindObj as VisualElement;
                        if (view == null)
                                throw new InvalidOperationException();
 
-                       base.Apply(newContext, bindObj, targetProperty);
+                       base.Apply(newContext, bindObj, targetProperty, fromBindingContextChanged: fromBindingContextChanged);
 
                        Element dataSourceParent = await FindDataSourceParentAsync(view);
 
@@ -116,9 +116,9 @@ namespace Xamarin.Forms.Pages
                        return base.GetTargetValue(value, sourcePropertyType);
                }
 
-               internal override void Unapply()
+               internal override void Unapply(bool fromBindingContextChanged = false)
                {
-                       base.Unapply();
+                       base.Unapply(fromBindingContextChanged: fromBindingContextChanged);
 
                        var dataSourceProviderer = (IDataSourceProvider)_dataSourceRef?.Target;
                        dataSourceProviderer?.UnmaskKey(_path);

diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Bz34037.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Bz34037.xaml.cs
index ee300cb..ce2c935 100644 (file)
--- a/Xamarin.Forms.Xaml.UnitTests/Issues/Bz34037.xaml.cs
+++ b/Xamarin.Forms.Xaml.UnitTests/Issues/Bz34037.xaml.cs
@@ -3,6 +3,7 @@ using System.Collections.Generic;
 
 using Xamarin.Forms;
 using NUnit.Framework;
+using Xamarin.Forms.Core.UnitTests;
 
 namespace Xamarin.Forms.Xaml.UnitTests
 {
@@ -72,19 +73,30 @@ namespace Xamarin.Forms.Xaml.UnitTests
                        [SetUp]
                        public void Setup ()
                        {
+                               Device.PlatformServices = new MockPlatformServices();
                                Bz34037Converter0.Invoked = 0;
                                Bz34037Converter1.Invoked = 0;
                        }
 
+                       [TearDown]
+                       public void TearDown()
+                       {
+                               Device.PlatformServices = null;
+                               Application.Current = null;
+                       }
+
+
                        [TestCase(true)]
                        [TestCase(false)]
                        public void ConverterParameterOrderDoesNotMatters (bool useCompiledXaml)
                        {
                                var layout = new Bz34037 (useCompiledXaml);
-                               Assert.AreEqual (2, Bz34037Converter0.Invoked);
-//                             Assert.AreEqual (2, Bz34037Converter1.Invoked);
+                               Assert.AreEqual (1, Bz34037Converter0.Invoked);
+                               Assert.AreEqual (1, Bz34037Converter1.Invoked);
                                Assert.AreEqual (typeof(string), Bz34037Converter0.Parameter);
-//                             Assert.AreEqual (typeof(string), Bz34037Converter1.Parameter);
+                               Assert.AreEqual (typeof(string), Bz34037Converter1.Parameter);
+                               Assert.That(layout.s0.IsToggled, Is.True);
+                               Assert.That(layout.s1.IsToggled, Is.True);
                        }
                }
        }