[Android] Don't crash if Control is accessed in Effect OnDetached when Page is Disposed. (#773)

* Add repro for 51505

* [Android] Don't dispose of EffectControlProvider

* Oops

Xamarin.Forms.ControlGallery.Android/Activity1.cs

@@ -43,6 +43,7 @@ namespace Xamarin.Forms.ControlGallery.Android
 
 		protected override void OnDetached ()
 		{
+			Control.SetBackgroundColor(global::Android.Graphics.Color.Beige);
 		}
 
 		protected override void OnElementPropertyChanged (PropertyChangedEventArgs args)

Xamarin.Forms.ControlGallery.WindowsUniversal/MainPage.xaml.cs

@@ -8,34 +8,59 @@ using Windows.UI.ViewManagement;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Controls;
 using Windows.UI.Xaml.Media;
+using Xamarin.Forms;
+using Xamarin.Forms.ControlGallery.WindowsUniversal;
 using Xamarin.Forms.Controls;
 using Xamarin.Forms.Platform.UWP;
 
+[assembly: ExportEffect(typeof(BorderEffect), "BorderEffect")]
 namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 {
+	public class BorderEffect : PlatformEffect
+	{
+		protected override void OnAttached()
+		{
+			var control = Control as Control;
+			if (control == null)
+				return;
+
+			control.Background = new SolidColorBrush(Windows.UI.Colors.Aqua);
+		}
+
+		protected override void OnDetached()
+		{
+			var control = Control as Control;
+			if (control == null)
+				return;
+
+			control.Background = new SolidColorBrush(Windows.UI.Colors.Beige);
+		}
+	}
+
 	/// <summary>
 	/// An empty page that can be used on its own or navigated to within a Frame.
 	/// </summary>
 	public sealed partial class MainPage
-    {
-        public MainPage()
-        {
-            InitializeComponent();
+	{
+		public MainPage()
+		{
+			InitializeComponent();
 
-			var app = new Controls.App ();
+			var app = new Controls.App();
 
 			// When the native control gallery loads up, it'll let us know so we can add the nested native controls
 			MessagingCenter.Subscribe<NestedNativeControlGalleryPage>(this, NestedNativeControlGalleryPage.ReadyForNativeControlsMessage, AddNativeControls);
-					
+
 			// When the native binding gallery loads up, it'll let us know so we can set up the native bindings
-			MessagingCenter.Subscribe<NativeBindingGalleryPage >(this, NativeBindingGalleryPage.ReadyForNativeBindingsMessage, AddNativeBindings);
+			MessagingCenter.Subscribe<NativeBindingGalleryPage>(this, NativeBindingGalleryPage.ReadyForNativeBindingsMessage, AddNativeBindings);
 
-			LoadApplication (app);
-        }
+			LoadApplication(app);
+		}
 
-		void AddNativeControls (NestedNativeControlGalleryPage page)
+		void AddNativeControls(NestedNativeControlGalleryPage page)
 		{
-			if (page.NativeControlsAdded) {
+			if (page.NativeControlsAdded)
+			{
 				return;
 			}
 
@@ -43,54 +68,59 @@ namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 
 			// Create and add a native TextBlock
 			var originalText = "I am a native TextBlock";
-			var textBlock = new TextBlock {
+			var textBlock = new TextBlock
+			{
 				Text = originalText,
 				FontSize = 14,
-				FontFamily = new FontFamily ("HelveticaNeue")
+				FontFamily = new FontFamily("HelveticaNeue")
 			};
 
-			sl?.Children.Add (textBlock);
+			sl?.Children.Add(textBlock);
 
 			// Create and add a native Button 
 			var button = new Windows.UI.Xaml.Controls.Button { Content = "Toggle Font Size", Height = 80 };
 			button.Click += (sender, args) => { textBlock.FontSize = textBlock.FontSize == 14 ? 24 : 14; };
 
-			sl?.Children.Add (button.ToView ());
+			sl?.Children.Add(button.ToView());
 
 			// Create a control which we know doesn't behave correctly with regard to measurement
-			var difficultControl = new BrokenNativeControl {
+			var difficultControl = new BrokenNativeControl
+			{
 				Text = "Not Sized/Arranged Properly"
 			};
 
-			var difficultControl2 = new BrokenNativeControl {
+			var difficultControl2 = new BrokenNativeControl
+			{
 				Text = "Fixed"
 			};
 
 			// Add the misbehaving controls, one with a custom delegate for ArrangeOverrideDelegate
-			sl?.Children.Add (difficultControl);
-			sl?.Children.Add (difficultControl2,
-				arrangeOverrideDelegate: (renderer, finalSize) => {
-					if (finalSize.Width <= 0 || double.IsInfinity (finalSize.Width)) {
+			sl?.Children.Add(difficultControl);
+			sl?.Children.Add(difficultControl2,
+				arrangeOverrideDelegate: (renderer, finalSize) =>
+				{
+					if (finalSize.Width <= 0 || double.IsInfinity(finalSize.Width))
+					{
 						return null;
 					}
 
 					FrameworkElement frameworkElement = renderer.Control;
 
-					frameworkElement.Measure (finalSize);
+					frameworkElement.Measure(finalSize);
 
 					// The broken control always tries to size itself to the screen width
 					// So figure that out and we'll know how far off it's laying itself out
-					Rect bounds = ApplicationView.GetForCurrentView ().VisibleBounds;
-					double scaleFactor = DisplayInformation.GetForCurrentView ().RawPixelsPerViewPixel;
-					var screenWidth = new Size (bounds.Width * scaleFactor, bounds.Height * scaleFactor);
-					
+					Rect bounds = ApplicationView.GetForCurrentView().VisibleBounds;
+					double scaleFactor = DisplayInformation.GetForCurrentView().RawPixelsPerViewPixel;
+					var screenWidth = new Size(bounds.Width * scaleFactor, bounds.Height * scaleFactor);
+
 					// We can re-center it by offsetting it during the Arrange call
 					double diff = Math.Abs(screenWidth.Width - finalSize.Width) / -2;
-					frameworkElement.Arrange (new Rect (diff, 0, finalSize.Width - diff, finalSize.Height));
+					frameworkElement.Arrange(new Rect(diff, 0, finalSize.Width - diff, finalSize.Height));
 
 					// Arranging the control to the left will make it show up past the edge of the stack layout
 					// We can fix that by clipping it manually
-					var clip = new RectangleGeometry { Rect = new Rect (-diff, 0, finalSize.Width, finalSize.Height) };
+					var clip = new RectangleGeometry { Rect = new Rect(-diff, 0, finalSize.Width, finalSize.Height) };
 					frameworkElement.Clip = clip;
 
 					return finalSize;
@@ -107,12 +137,14 @@ namespace Xamarin.Forms.ControlGallery.WindowsUniversal
 
 			StackLayout sl = page.Layout;
 
-			var txbLabel = new TextBlock {
+			var txbLabel = new TextBlock
+			{
 				FontSize = 14,
 				FontFamily = new FontFamily("HelveticaNeue")
 			};
 
-			var txbBox = new TextBox {
+			var txbBox = new TextBox
+			{
 				FontSize = 14,
 				FontFamily = new FontFamily("HelveticaNeue")
 			};

Xamarin.Forms.ControlGallery.iOS/AppDelegate.cs

@@ -17,8 +17,22 @@ using Xamarin.Forms.Platform.iOS;
 [assembly: Dependency(typeof(CacheService))]
 [assembly: ExportRenderer(typeof(DisposePage), typeof(DisposePageRenderer))]
 [assembly: ExportRenderer(typeof(DisposeLabel), typeof(DisposeLabelRenderer))]
+[assembly: ExportEffect(typeof(BorderEffect), "BorderEffect")]
 namespace Xamarin.Forms.ControlGallery.iOS
 {
+	public class BorderEffect : PlatformEffect
+	{
+		protected override void OnAttached()
+		{
+			Control.BackgroundColor = UIColor.Blue;
+		}
+
+		protected override void OnDetached()
+		{
+			Control.BackgroundColor = UIColor.Brown;
+		}
+	}
+
 	public class CacheService : ICacheService
 	{
 		public void ClearImageCache()

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla51505.cs

@@ -0,0 +1,56 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+// Apply the default category of "Issues" to all of the tests in this assembly
+// We use this as a catch-all for tests which haven't been individually categorized
+#if UITEST
+[assembly: NUnit.Framework.Category("Issues")]
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Bugzilla, 51505, "ObjectDisposedException On Effect detachment.", PlatformAffected.Android)]
+	public class Bugzilla51505 : TestContentPage
+	{
+		const string ButtonId = "button";
+
+		protected override void Init()
+		{
+			var effect = Effect.Resolve("XamControl.BorderEffect");
+
+			var button = new Button { Text = "Click me", AutomationId = ButtonId };
+			button.Clicked += async (sender, e) =>
+			{
+				await Navigation.PopAsync();
+			};
+			button.Effects.Add(effect);
+
+			Content = new StackLayout
+			{
+				Children = 
+				{
+					new Label 
+					{
+						Text = "The following Button has an Effect applied to it that should attempt to access the Control when it is Detached. When you click the Button, this page should be popped. If the app crashes, this test has failed."
+					},
+					button
+				}
+			};
+		}
+
+#if UITEST
+		[Test]
+		public void Bugzilla51505Test()
+		{
+			RunningApp.WaitForElement(q => q.Marked(ButtonId));
+			Assert.DoesNotThrow(() => RunningApp.Tap(q => q.Marked(ButtonId)), "Accessing the Control when an Effect is detached should not throw");
+		}
+#endif
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -247,6 +247,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla37431.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla44777.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51503.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla51505.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla52533.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)_Template.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1028.cs" />

Xamarin.Forms.Platform.Android/VisualElementRenderer.cs

@@ -280,8 +280,6 @@ namespace Xamarin.Forms.Platform.Android
 					if (Platform.GetRenderer(Element) == this)
 						Platform.SetRenderer(Element, null);
 
-					(Element as IElementController).EffectControlProvider = null;
-
 					Element = null;
 				}
 			}