Set the Id field for Android Views created by Forms #1004

16 files changed, 73 insertions, 35 deletions

diff --git a/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs
index 766ee225..f50815d2 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs
@@ -94,7 +94,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 						LayoutParameters = new ViewGroup.LayoutParams(ViewGroup.LayoutParams.MatchParent, ViewGroup.LayoutParams.MatchParent),
 						Adapter = new FormsFragmentPagerAdapter<ContentPage>(e.NewElement, activity.SupportFragmentManager) { CountOverride = e.NewElement.Children.Count }
 					};
-				pager.Id = FormsAppCompatActivity.GetUniqueId();
+				pager.Id = Platform.GenerateViewId();
 				pager.AddOnPageChangeListener(this);
 
 				ViewGroup.AddView(pager);
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs b/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs
index 8bba063b..a121cfff 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs
@@ -549,23 +549,6 @@ namespace Xamarin.Forms.Platform.Android
 
 		public static int ToolbarResource { get; set; }
 
-		internal static int GetUniqueId()
-		{
-			// getting unique Id's is an art, and I consider myself the Jackson Pollock of the field
-			if ((int)Build.VERSION.SdkInt >= 17)
-				return global::Android.Views.View.GenerateViewId();
-
-			// Numbers higher than this range reserved for xml
-			// If we roll over, it can be exceptionally problematic for the user if they are still retaining things, android's internal implementation is
-			// basically identical to this except they do a lot of locking we don't have to because we know we only do this
-			// from the UI thread
-			if (s_id >= 0x00ffffff)
-				s_id = 0x00000400;
-			return s_id++;
-		}
-
-		static int s_id = 0x00000400;
-
 		#endregion
 	}
 }
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
index 50840926..faaf7462 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
@@ -19,7 +19,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 
 		public MasterDetailContainer(MasterDetailPage parent, bool isMaster, Context context) : base(parent, isMaster, context)
 		{
-			Id = FormsAppCompatActivity.GetUniqueId();
+			Id = Platform.GenerateViewId();
 			_parent = parent;
 			_isMaster = isMaster;
 		}
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs
index d405b42b..69e1c13b 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs
@@ -50,7 +50,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 		public NavigationPageRenderer()
 		{
 			AutoPackage = false;
-			Id = FormsAppCompatActivity.GetUniqueId();
+			Id = Platform.GenerateViewId();
 			Device.Info.PropertyChanged += DeviceInfoPropertyChanged;
 		}
 
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs b/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs
index 872bac77..0081d5a1 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs
@@ -353,6 +353,8 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				Android.Platform.SetRenderer(modal, _renderer);
 
 				AddView(_renderer.View);
+
+				Id = Platform.GenerateViewId();
 			}
 
 			protected override void Dispose(bool disposing)
@@ -399,6 +401,11 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			}
 		}
 
+		internal static int GenerateViewId()
+		{
+			return Android.Platform.GenerateViewId();
+		}
+
 		#region Statics
 
 		public static implicit operator ViewGroup(Platform canvas)
diff --git a/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs
index bb14100d..78fc19cb 100644
--- a/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs
@@ -166,7 +166,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 							LayoutParameters = new LayoutParams(LayoutParams.MatchParent, LayoutParams.MatchParent),
 							Adapter = new FormsFragmentPagerAdapter<Page>(e.NewElement, FragmentManager) { CountOverride = e.NewElement.Children.Count }
 						};
-					pager.Id = FormsAppCompatActivity.GetUniqueId();
+					pager.Id = Platform.GenerateViewId();
 					pager.AddOnPageChangeListener(this);
 
 					AddView(pager);
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
index 6cd158ff..e4118ac4 100644
--- a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
+++ b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
@@ -162,8 +162,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 			if (elemValue != null)
 			{
 				var id = Control.Id;
-				if (id == -1)
-					id = Control.Id = FormsAppCompatActivity.GetUniqueId();
+				if (id == global::Android.Views.View.NoId)
+					id = Control.Id = Platform.GenerateViewId();
 
 				var renderer = elemValue?.GetRenderer();
 				renderer?.SetLabelFor(id);
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs
index 6d19ffe6..3c08f4bb 100644
--- a/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs
@@ -40,7 +40,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 			_effectControlProvider = new EffectControlProvider(this);
             _textColorSwitcher = new Lazy<TextColorSwitcher>(() => new TextColorSwitcher(TextColors));
 
-            Initialize();
+			Initialize();
 		}
 
 		public VisualElement Element => Button;
@@ -208,13 +208,16 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 		{
 			if (e.NewElement != null && !_isDisposed)
 			{
-                UpdateFont();
-                UpdateText();
-                UpdateBitmap();
-                UpdateTextColor();
-                UpdateIsEnabled();
-			    UpdateInputTransparent();
-                UpdateBackgroundColor();
+
+				this.EnsureId();
+
+				UpdateFont();
+				UpdateText();
+				UpdateBitmap();
+				UpdateTextColor();
+				UpdateIsEnabled();
+				UpdateInputTransparent();
+				UpdateBackgroundColor();
 			}
 
 			ElementChanged?.Invoke(this, new VisualElementChangedEventArgs(e.OldElement, e.NewElement));
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs
index d3583cce..82637a3b 100644
--- a/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs
@@ -145,6 +145,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 
 			if (e.NewElement != null)
 			{
+				this.EnsureId();
+
 				if (_visualElementTracker == null)
 				{
 					_visualElementTracker = new VisualElementTracker(this);
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs
index 76901021..28aa11ad 100644
--- a/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs
@@ -62,11 +62,12 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 		{
 			await TryUpdateBitmap(e.OldElement);
 			UpdateAspect();
+			this.EnsureId();
 
 			ElementChanged?.Invoke(this, new VisualElementChangedEventArgs(e.OldElement, e.NewElement));
 		}
 
-        public override bool OnTouchEvent(MotionEvent e)
+		public override bool OnTouchEvent(MotionEvent e)
         {
             bool handled;
             var result = _visualElementRenderer.OnTouchEvent(e, Parent, out handled);
@@ -74,7 +75,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
             return handled ? result : base.OnTouchEvent(e);
         }
 
-        protected virtual Size MinimumSize()
+		protected virtual Size MinimumSize()
 		{
 			return new Size();
 		}
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs
index aa1f06e0..f4b11bbd 100644
--- a/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs
@@ -177,6 +177,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers
 
 			if (e.NewElement != null)
 			{
+				this.EnsureId();
+
 				if (_visualElementTracker == null)
 				{
 					_visualElementTracker = new VisualElementTracker(this);
diff --git a/Xamarin.Forms.Platform.Android/Platform.cs b/Xamarin.Forms.Platform.Android/Platform.cs
index fe588eb8..bbbdf953 100644
--- a/Xamarin.Forms.Platform.Android/Platform.cs
+++ b/Xamarin.Forms.Platform.Android/Platform.cs
@@ -1035,6 +1035,23 @@ namespace Xamarin.Forms.Platform.Android
 			}
 		}
 
+		internal static int GenerateViewId()
+		{
+			// getting unique Id's is an art, and I consider myself the Jackson Pollock of the field
+			if ((int)Build.VERSION.SdkInt >= 17)
+				return global::Android.Views.View.GenerateViewId();
+
+			// Numbers higher than this range reserved for xml
+			// If we roll over, it can be exceptionally problematic for the user if they are still retaining things, android's internal implementation is
+			// basically identical to this except they do a lot of locking we don't have to because we know we only do this
+			// from the UI thread
+			if (s_id >= 0x00ffffff)
+				s_id = 0x00000400;
+			return s_id++;
+		}
+
+		static int s_id = 0x00000400;
+
 		internal class DefaultRenderer : VisualElementRenderer<View>
 		{
 			bool _notReallyHandled;
diff --git a/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs b/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs
index a4e77361..435ee7cf 100644
--- a/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs
+++ b/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs
@@ -10,6 +10,7 @@ namespace Xamarin.Forms.Platform.Android
 			AddView(child.View);
 			Child = child;
 			IsInFragment = inFragment;
+			Id = Platform.GenerateViewId();
 		}
 
 		public IVisualElementRenderer Child { get; set; }
diff --git a/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs
index 7db4a4ad..3474abca 100644
--- a/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs
@@ -43,6 +43,11 @@ namespace Xamarin.Forms.Platform.Android
 			Page view = e.NewElement;
 			base.OnElementChanged(e);
 
+			if (Id == NoId)
+			{
+				Id = Platform.GenerateViewId();
+			}
+
 			UpdateBackgroundColor(view);
 			UpdateBackgroundImage(view);
 
diff --git a/Xamarin.Forms.Platform.Android/ViewExtensions.cs b/Xamarin.Forms.Platform.Android/ViewExtensions.cs
index 6afcf5c2..89146705 100644
--- a/Xamarin.Forms.Platform.Android/ViewExtensions.cs
+++ b/Xamarin.Forms.Platform.Android/ViewExtensions.cs
@@ -64,5 +64,18 @@ namespace Xamarin.Forms.Platform.Android
 				}
 			}
 		}
+
+		public static void EnsureId(this AView view)
+		{
+			if (view.IsDisposed())
+			{
+				return;
+			}
+
+			if (view.Id == AView.NoId)
+			{
+				view.Id = Platform.GenerateViewId();
+			}
+		}
 	}
 }
\ No newline at end of file
diff --git a/Xamarin.Forms.Platform.Android/ViewRenderer.cs b/Xamarin.Forms.Platform.Android/ViewRenderer.cs
index 1cf77594..dbd930b8 100644
--- a/Xamarin.Forms.Platform.Android/ViewRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/ViewRenderer.cs
@@ -290,6 +290,10 @@ namespace Xamarin.Forms.Platform.Android
 			_container = container;
 
 			Control = control;
+			if (Control.Id == NoId)
+			{
+				Control.Id = Platform.GenerateViewId();
+			}
 
 			AView toAdd = container == this ? control : (AView)container;
 			AddView(toAdd, LayoutParams.MatchParent);
@@ -310,8 +314,8 @@ namespace Xamarin.Forms.Platform.Android
 			if (elemValue != null)
 			{
 				var id = Control.Id;
-				if (id == -1)
-					id = Control.Id = FormsAppCompatActivity.GetUniqueId();
+				if (id == NoId)
+					id = Control.Id = Platform.GenerateViewId();
 
 				var renderer = elemValue?.GetRenderer();
 				renderer?.SetLabelFor(id);