[ABRecord] Subclass NativeObject + numerous other code updates (#13085)

* Subclass NativeObject to reuse object lifetime code. * Enable nullability and fix code accordingly. * Use 'is' and 'is not' instead of '==' and '!=' for object identity. * Use CFString.CreateNative/ReleaseNative instead of other means to create native strings (the fastest and least memory hungry option). * Use the null-safe NativeObjectExtensions.GetHandle extension method to get the handle instead of checking for null (avoids some code duplication). * Use 'nameof (parameter)' instead of string constants.
xamarin · Oct 25, 2021 · 0ede53d · 0ede53d · vs-mobiletools-engineering-service2 · Oct 25, 2021
1 parent 22b8c38
commit 0ede53d
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 63 deletions.
diff --git a/src/AddressBook/ABPerson.cs b/src/AddressBook/ABPerson.cs
@@ -944,8 +944,8 @@ public void SetDates (ABMultiValue<NSDate>? value)
 		}
 
 		public ABPersonKind PersonKind {
-			get {return ABPersonKindId.ToPersonKind (PropertyTo<NSNumber> (ABPersonPropertyId.Kind));}
-			set {SetValue (ABPersonPropertyId.Kind, ABPersonKindId.FromPersonKind (value));}
+			get { return ABPersonKindId.ToPersonKind (PropertyTo<NSNumber> (ABPersonPropertyId.Kind!)!); }
+			set { SetValue (ABPersonPropertyId.Kind!, ABPersonKindId.FromPersonKind (value)); }
 		}
 
 		public ABMultiValue<string>? GetPhones ()

diff --git a/src/AddressBook/ABRecord.cs b/src/AddressBook/ABRecord.cs
@@ -27,6 +27,8 @@
 //
 //
 
+#nullable enable
+
 #if !MONOMAC
 
 using System;
@@ -53,32 +55,28 @@ namespace AddressBook {
 	[Obsolete ("Starting with maccatalyst14.0 use the 'Contacts' API instead.", DiagnosticId = "BI1234", UrlFormat = "https://github.com/xamarin/xamarin-macios/wiki/Obsolete")]
 #endif
 #endif
-	public class ABRecord : INativeObject, IDisposable {
+	public class ABRecord : NativeObject {
 
 		public const int InvalidRecordId = -1;
 		public const int InvalidPropertyId = -1;
 
-		IntPtr handle;
-
 		[Preserve (Conditional = true)]
 		internal ABRecord (IntPtr handle, bool owns)
+			: base (handle, owns)
 		{
-			if (!owns)
-				CFObject.CFRetain (handle);
-			this.handle = handle;
 		}
 
-		public static ABRecord FromHandle (IntPtr handle)
+		public static ABRecord? FromHandle (IntPtr handle)
 		{
 			if (handle == IntPtr.Zero)
 				return null;
 			return FromHandle (handle, null, false);
 		}
 
-		internal static ABRecord FromHandle (IntPtr handle, ABAddressBook addressbook, bool owns = true)
+		internal static ABRecord FromHandle (IntPtr handle, ABAddressBook? addressbook, bool owns = true)
 		{
 			if (handle == IntPtr.Zero)
-				throw new ArgumentNullException ("handle");
+				throw new ArgumentNullException (nameof (handle));
 			// TODO: does ABGroupCopyArrayOfAllMembers() have Create or Get
 			// semantics for the array elements?
 			var type = ABRecordGetRecordType (handle);
@@ -102,45 +100,16 @@ internal static ABRecord FromHandle (IntPtr handle, ABAddressBook addressbook, b
 			return rec;
 		}
 
-		~ABRecord ()
-		{
-			Dispose (false);
-		}
-
-		public void Dispose ()
+		protected override void Dispose (bool disposing)
 		{
-			Dispose (true);
-			GC.SuppressFinalize (this);
-		}
-
-		protected virtual void Dispose (bool disposing)
-		{
-			if (handle != IntPtr.Zero)
-				CFObject.CFRelease (handle);
-			handle = IntPtr.Zero;
 			AddressBook = null;
+			base.Dispose (disposing);
 		}
 
-		void AssertValid ()
-		{
-			if (handle == IntPtr.Zero)
-				throw new ObjectDisposedException ("");
-		}
-
-		internal ABAddressBook AddressBook {
+		internal ABAddressBook? AddressBook {
 			get; set;
 		}
 
-		public IntPtr Handle {
-			get {
-				AssertValid ();
-				return handle;
-			}
-			internal set {
-				handle = value;				
-			}
-		}
-
 		[DllImport (Constants.AddressBookLibrary)]
 		extern static int ABRecordGetRecordID (IntPtr record);
 		public int Id {
@@ -155,10 +124,9 @@ public ABRecordType Type {
 
 		[DllImport (Constants.AddressBookLibrary)]
 		extern static IntPtr ABRecordCopyCompositeName (IntPtr record);
-		public override string ToString ()
+		public override string? ToString ()
 		{
-			using (var s = new NSString (ABRecordCopyCompositeName (Handle)))
-				return s.ToString ();
+			return CFString.FromHandle (ABRecordCopyCompositeName (Handle));
 		}
 
 		// TODO: Should SetValue/CopyValue/RemoveValue be public?
@@ -173,15 +141,19 @@ internal void SetValue (int property, IntPtr value)
 				throw CFException.FromCFError (error);
 		}
 
-		internal void SetValue (int property, NSObject value)
+		internal void SetValue (int property, NSObject? value)
 		{
-			SetValue (property, value == null ? IntPtr.Zero : value.Handle);
+			SetValue (property, value.GetHandle ());
 		}
 
-		internal void SetValue (int property, string value)
+		internal void SetValue (int property, string? value)
 		{
-			using (NSObject v = value == null ? null : new NSString (value))
-				SetValue (property, v);
+			var valueHandle = CFString.CreateNative (value);
+			try {
+				SetValue (property, valueHandle);
+			} finally {
+				CFString.ReleaseNative (valueHandle);
+			}
 		}
 
 		[DllImport (Constants.AddressBookLibrary)]
@@ -202,22 +174,16 @@ internal void RemoveValue (int property)
 				throw CFException.FromCFError (error);
 		}
 
-		internal T PropertyTo<T> (int id)
+		internal T? PropertyTo<T> (int id)
 			where T : NSObject
 		{
 			IntPtr value = CopyValue (id);
-			if (value == IntPtr.Zero)
-				return null;
-			return (T) Runtime.GetNSObject (value);
+			return (T?) Runtime.GetNSObject (value);
 		}
 
-		internal string PropertyToString (int id)
+		internal string? PropertyToString (int id)
 		{
-			IntPtr value = CopyValue (id);
-			if (value == IntPtr.Zero)
-				return null;
-			using (var s = new NSString (value))
-				return s.ToString ();
+			return CFString.FromHandle (CopyValue (id));
 		}
 	}
 }

diff --git a/src/generator.cs b/src/generator.cs
@@ -2356,8 +2356,8 @@ public void Go ()
 			marshal_types.Add (new MarshalType (TypeManager.MTAudioProcessingTap, create: "MediaToolbox.MTAudioProcessingTap.FromHandle("));
 		if (Frameworks.HaveAddressBook) {
 			marshal_types.Add (TypeManager.ABAddressBook);
-			marshal_types.Add (new MarshalType (TypeManager.ABPerson, create: "(ABPerson) ABRecord.FromHandle("));
-			marshal_types.Add (new MarshalType (TypeManager.ABRecord, create: "ABRecord.FromHandle("));
+			marshal_types.Add (new MarshalType (TypeManager.ABPerson, create: "(ABPerson) ABRecord.FromHandle (", closingCreate: ")!"));
+			marshal_types.Add (new MarshalType (TypeManager.ABRecord, create: "ABRecord.FromHandle (", closingCreate: ")!"));
 		}
 		if (Frameworks.HaveCoreVideo) {
 			// owns `false` like ptr ctor https://github.com/xamarin/xamarin-macios/blob/6f68ab6f79c5f1d96d2cbb1e697330623164e46d/src/CoreVideo/CVBuffer.cs#L74-L90