[Mono.Android] PeekObject() should consider target type (#717)

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=58405

When using `Android.OS.Bundle` to retrieve a collection for a specific
key one can either use the untyped `Get` method or one of the
generic `Get*ArrayList` methods to retrieve the list. In the first
case we will get back an `IList` instance, in the second case we'll
get an `IList<T>` instance. However, if one calls `Get` first and then
one of the generic methods next, for the same key, the result will be
a thrown `InvalidCastException` because we attempt to cast the `IList`
obtained from `Get` to `IList<T>` needed by the generic `Get*ArrayList`
method. This happens because the same native handle is used in both cases
and the first call to `Get` causes `Java.Lang.Object` to cache the `IList`
instance so that the second call to `Get*ArrayList` gets the cached object
instead of a new, properly typed, one.

The solution is to extend `Java.Lang.Object.PeekObject` to allow specifying
the desired type of the object corresponding to the native handle and, if
the requirement isn't met, evict the entry from the cache returning `null`
to the caller, thus letting it do the right thing by creating an instance
of a class with correct type.

Author: grendello

src/Mono.Android/Android.Runtime/JavaList.cs

@@ -550,7 +550,7 @@ public static IList<T> FromJniHandle (IntPtr handle, JniHandleOwnership transfer
 			if (handle == IntPtr.Zero)
 				return null;
 
-			IJavaObject inst = Java.Lang.Object.PeekObject (handle);
+			IJavaObject inst = Java.Lang.Object.PeekObject (handle, typeof (IList<T>));
 			if (inst == null)
 				inst = new JavaList<T> (handle, transfer);
 			else

src/Mono.Android/Java.Lang/Object.cs

@@ -390,7 +390,7 @@ internal static List<WeakReference> GetSurfacedObjects_ForDiagnosticsOnly ()
 			}
 		}
 
-		internal static IJavaObject PeekObject (IntPtr handle)
+		internal static IJavaObject PeekObject (IntPtr handle, Type requiredType = null)
 		{
 			if (handle == IntPtr.Zero)
 				return null;
@@ -401,14 +401,22 @@ internal static IJavaObject PeekObject (IntPtr handle)
 					for (int i  = 0; i < wrefs.Count; ++i) {
 						var wref  = wrefs [i];
 						IJavaObject res = wref.Target as IJavaObject;
-						if (res != null && res.Handle != IntPtr.Zero && JNIEnv.IsSameObject (handle, res.Handle))
+						if (res != null && res.Handle != IntPtr.Zero && JNIEnv.IsSameObject (handle, res.Handle)) {
+							if (requiredType != null && !requiredType.IsAssignableFrom (res.GetType ()))
+								return null;
 							return res;
+						}
 					}
 				}
 			}
 			return null;
 		}
 
+		internal static T PeekObject <T> (IntPtr handle)
+		{
+			return (T)PeekObject (handle, typeof (T));
+		}
+
 		public static T GetObject<T> (IntPtr jnienv, IntPtr handle, JniHandleOwnership transfer)
 			where T : class, IJavaObject
 		{

src/Mono.Android/Test/Android.OS/BundleTest.cs

@@ -0,0 +1,45 @@
+using System;
+using System.Collections.Generic;
+using System.Threading;
+using Android.OS;
+
+using NUnit.Framework;
+
+namespace Xamarin.Android.RuntimeTests {
+
+	[TestFixture]
+	public class BundleTest 
+	{
+		[Test]
+		public void TestBundleIntegerArrayList()
+		{
+			var b = new Bundle();
+			b.PutIntegerArrayList("key", new List<Java.Lang.Integer>() { Java.Lang.Integer.ValueOf(1) });
+			var list = b.GetIntegerArrayList ("key");
+			Assert.NotNull (list, "'key' doesn't refer to a list of integers");
+			var obj = b.Get ("key");
+			Assert.NotNull (obj, "Missing 'key' in bundle");
+			try {
+				list = b.GetIntegerArrayList ("key");
+				Assert.NotNull (list, "'key' doesn't refer to a list of integers after non-generic call");
+			} catch (Exception e) {
+				Assert.Fail ("Java.Lang.Object caches too aggresively");
+			}
+		}
+
+		[Test]
+		public void TestBundleIntegerArrayList2 ()
+		{
+			var b = new Bundle();
+			b.PutIntegerArrayList ("key", new List<Java.Lang.Integer> () { Java.Lang.Integer.ValueOf (1) });
+			var obj = b.Get ("key");
+			Assert.NotNull (obj, "Missing 'key' in bundle");
+			try {
+				var list = b.GetIntegerArrayList ("key");
+				Assert.NotNull (list, "'key' doesn't refer to a list of integers");
+			} catch (Exception e) {
+				Assert.Fail ("Java.Lang.Object caches too aggresively");
+			}
+		}
+	}
+}

src/Mono.Android/Test/Mono.Android-Tests.csproj

@@ -84,6 +84,7 @@
     <Compile Include="Xamarin.Android.RuntimeTests\MyIntent.cs" />
     <Compile Include="Xamarin.Android.RuntimeTests\NonJavaObject.cs" />
     <Compile Include="Xamarin.Android.RuntimeTests\TestInstrumentation.cs" />
+    <Compile Include="Android.OS\BundleTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="Resources\AboutResources.txt" />