[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

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));
+		}
 	}
 }

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();
 		}
 

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();

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;
 		}

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));

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();

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 (Source != null && isApplied && fromBindingContextChanged)
+				return;
+
+			base.Apply(source, bindObj, targetProperty, fromBindingContextChanged);
+			
 #if (!DO_NOT_CHECK_FOR_BINDING_REUSE)
-			base.Apply(source, bindObj, targetProperty);
-
 			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();

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);

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);
 			}
 		}
 	}