Fix border on android buttons (#941)

* [Android] Add a button background tracker to handle background and border properties and be shared between button renderers

* [Android]Fix border radius dependency

* [Android] Make sure we always create or updated the background button tracker

7 files changed, 176 insertions, 158 deletions

diff --git a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
index 394f3325..81357fa6 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs
@@ -1,14 +1,9 @@
 using System;
 using System.ComponentModel;
 using Android.Content;
-using Android.Content.Res;
 using Android.Graphics;
-using Android.Graphics.Drawables;
-using Android.Support.V4.Content;
 using Android.Support.V7.Widget;
 using Android.Util;
-using Xamarin.Forms.Internals;
-using GlobalResource = Android.Resource;
 using Object = Java.Lang.Object;
 using AView = Android.Views.View;
 using AMotionEvent = Android.Views.MotionEvent;
@@ -19,6 +14,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 {
     public class ButtonRenderer : ViewRenderer<Button, AppCompatButton>, AView.IOnAttachStateChangeListener
 	{
+		ButtonBackgroundTracker _backgroundTracker;
 		TextColorSwitcher _textColorSwitcher;
 		float _defaultFontSize;
 		Typeface _defaultTypeface;
@@ -27,6 +23,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 		public ButtonRenderer()
 		{
+			System.Diagnostics.Debug.WriteLine("Slow Button!");
 			AutoPackage = false;
 		}
 
@@ -83,6 +80,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 					Control.Tag = null;
 					_textColorSwitcher = null;
 				}
+				_backgroundTracker?.Dispose();
 			}
 
 			base.Dispose(disposing);
@@ -111,8 +109,12 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 					button.AddOnAttachStateChangeListener(this);
 				}
 
+				if (_backgroundTracker == null)
+					_backgroundTracker = new ButtonBackgroundTracker(Element, Control);
+				else
+					_backgroundTracker.Button = e.NewElement;
+
 				UpdateAll();
-				UpdateBackgroundColor();
 			}
 		}
 
@@ -139,42 +141,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			if (Element == null || Control == null)
 				return;
 
-			Color backgroundColor = Element.BackgroundColor;
-			if (backgroundColor.IsDefault)
-			{
-				if (Control.SupportBackgroundTintList != null)
-				{
-					Context context = Context;
-					int id = GlobalResource.Attribute.ButtonTint;
-					unchecked
-					{
-						using (var value = new TypedValue())
-						{
-							try
-							{
-								Resources.Theme theme = context.Theme;
-								if (theme != null && theme.ResolveAttribute(id, value, true))
-#pragma warning disable 618
-									Control.SupportBackgroundTintList = Resources.GetColorStateList(value.Data);
-#pragma warning restore 618
-								else
-									Control.SupportBackgroundTintList = new ColorStateList(ColorExtensions.States, new[] { (int)0xffd7d6d6, 0x7fd7d6d6 });
-							}
-							catch (Exception ex)
-							{
-								Internals.Log.Warning("Xamarin.Forms.Platform.Android.ButtonRenderer", "Could not retrieve button background resource: {0}", ex);
-								Control.SupportBackgroundTintList = new ColorStateList(ColorExtensions.States, new[] { (int)0xffd7d6d6, 0x7fd7d6d6 });
-							}
-						}
-					}
-				}
-			}
-			else
-			{
-				int intColor = backgroundColor.ToAndroid().ToArgb();
-				int disableColor = backgroundColor.MultiplyAlpha(0.5).ToAndroid().ToArgb();
-				Control.SupportBackgroundTintList = new ColorStateList(ColorExtensions.States, new[] { intColor, disableColor });
-			}
+			_backgroundTracker.UpdateBackgroundColor();
 		}
 
 		void UpdateAll()
@@ -184,6 +151,16 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			UpdateBitmap();
 			UpdateTextColor();
 			UpdateEnabled();
+			UpdateBackgroundColor();
+			UpdateDrawable();
+		}
+
+		void UpdateDrawable()
+		{
+			if (Element == null || Control == null)
+				return;
+
+			_backgroundTracker?.UpdateDrawable();
 		}
 
 		void UpdateBitmap()
diff --git a/Xamarin.Forms.Platform.Android/ButtonBackgroundTracker.cs b/Xamarin.Forms.Platform.Android/ButtonBackgroundTracker.cs
new file mode 100644
index 00000000..e8b9b49c
--- /dev/null
+++ b/Xamarin.Forms.Platform.Android/ButtonBackgroundTracker.cs
@@ -0,0 +1,125 @@
+using System;
+using System.ComponentModel;
+using Android.Graphics.Drawables;
+using AButton = Android.Widget.Button;
+
+namespace Xamarin.Forms.Platform.Android
+{
+	internal class ButtonBackgroundTracker : IDisposable
+	{
+		Drawable _defaultDrawable;
+		ButtonDrawable _backgroundDrawable;
+		Button _button;
+		AButton _nativeButton;
+		bool _drawableEnabled;
+		bool _disposed;
+
+		public ButtonBackgroundTracker(Button button, AButton nativeButton)
+		{
+			Button = button;
+			_nativeButton = nativeButton;
+		}
+
+		public Button Button
+		{
+			get { return _button; }
+			set
+			{
+				if (_button == value)
+					return;
+				if (_button != null)
+					_button.PropertyChanged -= ButtonPropertyChanged;
+				_button = value;
+				_button.PropertyChanged += ButtonPropertyChanged;
+			}
+		}
+
+		public void UpdateDrawable()
+		{
+			if (_button.BackgroundColor == Color.Default)
+			{
+				if (!_drawableEnabled)
+					return;
+
+				if (_defaultDrawable != null)
+					_nativeButton.SetBackground(_defaultDrawable);
+
+				_drawableEnabled = false;
+			}
+			else
+			{
+				if (_backgroundDrawable == null)
+					_backgroundDrawable = new ButtonDrawable();
+
+				_backgroundDrawable.Button = _button;
+
+				if (_drawableEnabled)
+					return;
+
+				if (_defaultDrawable == null)
+					_defaultDrawable = _nativeButton.Background;
+
+				_nativeButton.SetBackground(_backgroundDrawable);
+				_drawableEnabled = true;
+			}
+
+			_nativeButton.Invalidate();
+		}
+
+		public void Reset()
+		{
+			if (_drawableEnabled)
+			{
+				_drawableEnabled = false;
+				_backgroundDrawable.Reset();
+				_backgroundDrawable = null;
+			}
+		}
+
+		public void UpdateBackgroundColor()
+		{
+			if (_button == null)
+				return;
+			UpdateDrawable();
+		}
+
+		public void Dispose()
+		{
+			Dispose(true);
+		}
+
+		protected virtual void Dispose(bool disposing)
+		{
+			if (!_disposed)
+			{
+				if (disposing)
+				{
+					_backgroundDrawable?.Dispose();
+					_backgroundDrawable = null;
+					_defaultDrawable?.Dispose();
+					_defaultDrawable = null;
+					if (_button != null)
+					{
+						_button.PropertyChanged -= ButtonPropertyChanged;
+						_button = null;
+					}
+					_nativeButton = null;
+				}
+				_disposed = true;
+			}
+		}
+
+		void ButtonPropertyChanged(object sender, PropertyChangedEventArgs e)
+		{
+			if (e.PropertyName.Equals(Button.BorderColorProperty.PropertyName) ||
+				e.PropertyName.Equals(Button.BorderWidthProperty.PropertyName) ||
+				e.PropertyName.Equals(Button.BorderRadiusProperty.PropertyName) ||
+				e.PropertyName.Equals(VisualElement.BackgroundColorProperty.PropertyName))
+			{
+				Reset();
+				UpdateDrawable();
+			}
+		}
+
+	}
+}
\ No newline at end of file
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs
index 3c08f4bb..3248a5f5 100644
--- a/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs
@@ -1,19 +1,15 @@
 using System;
 using System.ComponentModel;
 using Android.Content;
-using Android.Content.Res;
 using Android.Graphics;
 using Android.Graphics.Drawables;
 using Android.Support.V7.Widget;
 using Android.Util;
 using Android.Views;
 using Xamarin.Forms.Internals;
-using GlobalResource = Android.Resource;
 using AView = Android.Views.View;
-using AMotionEvent = Android.Views.MotionEvent;
 using AMotionEventActions = Android.Views.MotionEventActions;
 using static System.String;
-using Object = Java.Lang.Object;
 
 namespace Xamarin.Forms.Platform.Android.FastRenderers
 {
@@ -30,6 +26,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
         readonly AutomationPropertiesProvider _automationPropertiesProvider;
 		readonly EffectControlProvider _effectControlProvider;
 		VisualElementTracker _tracker;
+		ButtonBackgroundTracker _backgroundTracker;
 
 		public event EventHandler<VisualElementChangedEventArgs> ElementChanged;
 		public event EventHandler<PropertyChangedEventArgs> ElementPropertyChanged;
@@ -124,6 +121,11 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 				oldElement.PropertyChanged -= OnElementPropertyChanged;
 			}
 
+			if (_backgroundTracker == null)
+				_backgroundTracker = new ButtonBackgroundTracker(Button, this);
+			else
+				_backgroundTracker.Button = Button;
+
 			Color currentColor = oldElement?.BackgroundColor ?? Color.Default;
 			if (element.BackgroundColor != currentColor)
 			{
@@ -182,6 +184,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 				_automationPropertiesProvider?.Dispose();
 				_tracker?.Dispose();
 
+				_backgroundTracker?.Dispose();
+		
 				if (Element != null)
 				{
 					Element.PropertyChanged -= OnElementPropertyChanged;
@@ -206,11 +210,14 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 
 		protected virtual void OnElementChanged(ElementChangedEventArgs<Button> e)
 		{
+			if (e.OldElement != null)
+			{
+				_backgroundTracker?.Reset();
+			}
+			
 			if (e.NewElement != null && !_isDisposed)
 			{
-
 				this.EnsureId();
-
 				UpdateFont();
 				UpdateText();
 				UpdateBitmap();
@@ -218,6 +225,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 				UpdateIsEnabled();
 				UpdateInputTransparent();
 				UpdateBackgroundColor();
+				UpdateDrawable();
 			}
 
 			ElementChanged?.Invoke(this, new VisualElementChangedEventArgs(e.OldElement, e.NewElement));
@@ -252,10 +260,6 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 			else if (e.PropertyName == VisualElement.InputTransparentProperty.PropertyName)
 			{
 				UpdateInputTransparent();
-            }
-			else if (e.PropertyName == VisualElement.BackgroundColorProperty.PropertyName)
-			{
-				UpdateBackgroundColor();
 			}
 
 			ElementPropertyChanged?.Invoke(this, e);
@@ -287,54 +291,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 
 		protected void UpdateBackgroundColor()
 		{
-			if (Element == null)
-			{
-				return;
-			}
-
-			Color backgroundColor = Element.BackgroundColor;
-			if (backgroundColor.IsDefault)
-			{
-				if (SupportBackgroundTintList != null)
-				{
-					Context context = Context;
-					int id = GlobalResource.Attribute.ButtonTint;
-					unchecked
-					{
-						using (var value = new TypedValue())
-						{
-							try
-							{
-								Resources.Theme theme = context.Theme;
-								if (theme != null && theme.ResolveAttribute(id, value, true))
-#pragma warning disable 618
-								{
-									SupportBackgroundTintList = Resources.GetColorStateList(value.Data);
-								}
-#pragma warning restore 618
-								else
-								{
-									SupportBackgroundTintList = new ColorStateList(ColorExtensions.States,
-										new[] { (int)0xffd7d6d6, 0x7fd7d6d6 });
-								}
-							}
-							catch (Exception ex)
-							{
-								Internals.Log.Warning("Xamarin.Forms.Platform.Android.ButtonRenderer",
-									"Could not retrieve button background resource: {0}", ex);
-								SupportBackgroundTintList = new ColorStateList(ColorExtensions.States,
-									new[] { (int)0xffd7d6d6, 0x7fd7d6d6 });
-							}
-						}
-					}
-				}
-			}
-			else
-			{
-				int intColor = backgroundColor.ToAndroid().ToArgb();
-				int disableColor = backgroundColor.MultiplyAlpha(0.5).ToAndroid().ToArgb();
-				SupportBackgroundTintList = new ColorStateList(ColorExtensions.States, new[] { intColor, disableColor });
-			}
+			_backgroundTracker.UpdateBackgroundColor();
 		}
 
 		internal virtual void OnNativeFocusChanged(bool hasFocus)
@@ -490,5 +447,11 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 
 			_textColorSwitcher.Value.UpdateTextColor(this, Button.TextColor);
 		}
+
+		void UpdateDrawable()
+		{
+			_backgroundTracker.UpdateDrawable();
+		}
+
 	}
 }
diff --git a/Xamarin.Forms.Platform.Android/Properties/AssemblyInfo.cs b/Xamarin.Forms.Platform.Android/Properties/AssemblyInfo.cs
index 2b046100..9fee726a 100644
--- a/Xamarin.Forms.Platform.Android/Properties/AssemblyInfo.cs
+++ b/Xamarin.Forms.Platform.Android/Properties/AssemblyInfo.cs
@@ -38,7 +38,6 @@ using Xamarin.Forms.Platform.Android;
 [assembly: ExportRenderer (typeof (Stepper), typeof (StepperRenderer))]
 [assembly: ExportRenderer (typeof (ProgressBar), typeof (ProgressBarRenderer))]
 [assembly: ExportRenderer (typeof (ScrollView), typeof (ScrollViewRenderer))]
-[assembly: ExportRenderer (typeof (Toolbar), typeof (ToolbarRenderer))]
 [assembly: ExportRenderer (typeof (ActivityIndicator), typeof (ActivityIndicatorRenderer))]
 [assembly: ExportRenderer (typeof (Frame), typeof (FrameRenderer))]
 [assembly: ExportRenderer (typeof (NavigationMenu), typeof (NavigationMenuRenderer))]
diff --git a/Xamarin.Forms.Platform.Android/Renderers/ButtonDrawable.cs b/Xamarin.Forms.Platform.Android/Renderers/ButtonDrawable.cs
index 62eaecbf..5852f47b 100644
--- a/Xamarin.Forms.Platform.Android/Renderers/ButtonDrawable.cs
+++ b/Xamarin.Forms.Platform.Android/Renderers/ButtonDrawable.cs
@@ -1,6 +1,7 @@
 using System.Linq;
 using Android.Graphics;
 using Android.Graphics.Drawables;
+using System;
 
 namespace Xamarin.Forms.Platform.Android
 {
@@ -136,7 +137,7 @@ namespace Xamarin.Forms.Platform.Android
 				float inset = borderWidth / 2;
 
 				// adjust border radius so outer edge of stroke is same radius as border radius of background
-				float borderRadius = Forms.Context.ToPixels(Button.BorderRadius) - inset;
+				float borderRadius = Math.Max(Forms.Context.ToPixels(Button.BorderRadius) - inset, 0);
 
 				path.AddRoundRect(new RectF(inset, inset, width - inset, height - inset), borderRadius, borderRadius, Path.Direction.Cw);
 				paint.StrokeWidth = borderWidth;
diff --git a/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs
index e07a2be5..da4ec0ba 100644
--- a/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/Renderers/ButtonRenderer.cs
@@ -1,8 +1,6 @@
 using System;
 using System.ComponentModel;
-using Android.Content.Res;
 using Android.Graphics;
-using Android.Graphics.Drawables;
 using Android.Util;
 using static System.String;
 using AButton = Android.Widget.Button;
@@ -15,12 +13,10 @@ namespace Xamarin.Forms.Platform.Android
 {
 	public class ButtonRenderer : ViewRenderer<Button, AButton>, AView.IOnAttachStateChangeListener
 	{
-		ButtonDrawable _backgroundDrawable;
+		ButtonBackgroundTracker _backgroundTracker;
 		TextColorSwitcher _textColorSwitcher;
-		Drawable _defaultDrawable;
 		float _defaultFontSize;
 		Typeface _defaultTypeface;
-		bool _drawableEnabled;
 		bool _isDisposed;
 		int _imageHeight = -1;
 
@@ -72,11 +68,7 @@ namespace Xamarin.Forms.Platform.Android
 
 			if (disposing)
 			{
-				if (_backgroundDrawable != null)
-				{
-					_backgroundDrawable.Dispose();
-					_backgroundDrawable = null;
-				}
+				_backgroundTracker?.Dispose();
 			}
 
 			base.Dispose(disposing);
@@ -104,15 +96,12 @@ namespace Xamarin.Forms.Platform.Android
 					_textColorSwitcher = new TextColorSwitcher(button.TextColors);
 					button.AddOnAttachStateChangeListener(this);
 				}
+				if (_backgroundTracker == null)
+					_backgroundTracker = new ButtonBackgroundTracker(Element, Control);
 			}
 			else
 			{
-				if (_drawableEnabled)
-				{
-					_drawableEnabled = false;
-					_backgroundDrawable.Reset();
-					_backgroundDrawable = null;
-				}
+				_backgroundTracker.Button = e.NewElement;
 			}
 
 			UpdateAll();
@@ -128,27 +117,17 @@ namespace Xamarin.Forms.Platform.Android
 				UpdateEnabled();
 			else if (e.PropertyName == Button.FontProperty.PropertyName)
 				UpdateFont();
-			else if (e.PropertyName == VisualElement.BackgroundColorProperty.PropertyName)
-				UpdateDrawable();
 			else if (e.PropertyName == Button.ImageProperty.PropertyName)
 				UpdateBitmap();
 			else if (e.PropertyName == VisualElement.IsVisibleProperty.PropertyName)
 				UpdateText();
-
-			if (_drawableEnabled &&
-				(e.PropertyName == VisualElement.BackgroundColorProperty.PropertyName || e.PropertyName == Button.BorderColorProperty.PropertyName || e.PropertyName == Button.BorderRadiusProperty.PropertyName ||
-				 e.PropertyName == Button.BorderWidthProperty.PropertyName))
-			{
-				_backgroundDrawable.Reset();
-				Control.Invalidate();
-			}
-
+			
 			base.OnElementPropertyChanged(sender, e);
 		}
 
 		protected override void UpdateBackgroundColor()
 		{
-			// Do nothing, the drawable handles this now
+			_backgroundTracker.UpdateBackgroundColor();
 		}
 
 		void UpdateAll()
@@ -219,34 +198,7 @@ namespace Xamarin.Forms.Platform.Android
 
 		void UpdateDrawable()
 		{
-			if (Element.BackgroundColor == Color.Default)
-			{
-				if (!_drawableEnabled)
-					return;
-
-				if (_defaultDrawable != null)
-					Control.SetBackground(_defaultDrawable);
-
-				_drawableEnabled = false;
-			}
-			else
-			{
-				if (_backgroundDrawable == null)
-					_backgroundDrawable = new ButtonDrawable();
-
-				_backgroundDrawable.Button = Element;
-
-				if (_drawableEnabled)
-					return;
-
-				if (_defaultDrawable == null)
-					_defaultDrawable = Control.Background;
-
-				Control.SetBackground(_backgroundDrawable);
-				_drawableEnabled = true;
-			}
-
-			Control.Invalidate();
+			_backgroundTracker.UpdateDrawable();
 		}
 
 		void UpdateEnabled()
diff --git a/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj b/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj
index 5d43f6c1..d4671f7a 100644
--- a/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj
+++ b/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj
@@ -103,6 +103,7 @@
     <Compile Include="AndroidApplicationLifecycleState.cs" />
     <Compile Include="AndroidTitleBarVisibility.cs" />
     <Compile Include="AppCompat\FrameRenderer.cs" />
+    <Compile Include="ButtonBackgroundTracker.cs" />
     <Compile Include="Extensions\JavaObjectExtensions.cs" />
     <Compile Include="FastRenderers\AutomationPropertiesProvider.cs" />
     <Compile Include="FastRenderers\ButtonRenderer.cs" />