Code Quality: Fixed a GC hole in the network connection dialog (#16482)

Author: 0x5bfa

src/Files.App/Data/Contracts/ICommonDialogService.cs

@@ -43,6 +43,6 @@ public interface ICommonDialogService
 		/// <param name="remoteNetworkName">The name of the remote network.</param>
 		/// <param name="useMostRecentPath">The value indicating whether to enter the most recently used paths into the combination box.</param>
 		/// <returns>True if the 'OK' button was clicked; otherwise, false.</returns>
-		bool Open_NetworkConnectionDialog(nint hWind, bool hideRestoreConnectionCheckBox = false, bool persistConnectionAtLogon = false, bool readOnlyPath = false, string? remoteNetworkName = null, bool useMostRecentPath = false);
+		bool Open_NetworkConnectionDialog(nint hWnd, bool hideRestoreConnectionCheckBox = false, bool persistConnectionAtLogon = false, bool readOnlyPath = false, string? remoteNetworkName = null, bool useMostRecentPath = false);
 	}
 }

src/Files.App/Services/Windows/WindowsDialogService.cs

@@ -178,57 +178,43 @@ public unsafe bool Open_FileSaveDialog(nint hWnd, bool pickFoldersOnly, string[]
 		}
 
 		/// <inheritdoc/>
-		public unsafe bool Open_NetworkConnectionDialog(nint hWind, bool hideRestoreConnectionCheckBox = false, bool persistConnectionAtLogon = false, bool readOnlyPath = false, string? remoteNetworkName = null, bool useMostRecentPath = false)
+		public unsafe bool Open_NetworkConnectionDialog(nint hWnd, bool hideRestoreConnectionCheckBox = false, bool persistConnectionAtLogon = false, bool readOnlyPath = false, string? remoteNetworkName = null, bool useMostRecentPath = false)
 		{
-			NETRESOURCEW netRes = default;
-			CONNECTDLGSTRUCTW dialogOptions = default;
+			if (useMostRecentPath && !string.IsNullOrEmpty(remoteNetworkName))
+				throw new ArgumentException($"{nameof(useMostRecentPath)} cannot be set to true if {nameof(remoteNetworkName)} has a value.");
 
-			dialogOptions.cbStructure = (uint)Marshal.SizeOf(typeof(CONNECTDLGSTRUCTW));
-			netRes.dwType = NET_RESOURCE_TYPE.RESOURCETYPE_DISK;
+			NETRESOURCEW netResource = default;
+			CONNECTDLGSTRUCTW connectDlgOptions = default;
+			WIN32_ERROR res = default;
 
 			if (hideRestoreConnectionCheckBox)
-				dialogOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_HIDE_BOX;
-			else
-				dialogOptions.dwFlags &= ~CONNECTDLGSTRUCT_FLAGS.CONNDLG_HIDE_BOX;
-
+				connectDlgOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_HIDE_BOX;
 			if (persistConnectionAtLogon)
-			{
-				dialogOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_PERSIST;
-				dialogOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_NOT_PERSIST;
-			}
-			else
-			{
-				dialogOptions.dwFlags &= ~CONNECTDLGSTRUCT_FLAGS.CONNDLG_PERSIST;
-				dialogOptions.dwFlags &= ~CONNECTDLGSTRUCT_FLAGS.CONNDLG_NOT_PERSIST;
-			}
-
-			fixed (char* lpcRemoteName = remoteNetworkName)
-				netRes.lpRemoteName = lpcRemoteName;
-
-			if (useMostRecentPath && !string.IsNullOrEmpty(remoteNetworkName))
-				throw new InvalidOperationException($"{nameof(useMostRecentPath)} cannot be set to true if {nameof(remoteNetworkName)} has a value.");
-
+				connectDlgOptions.dwFlags |= (CONNECTDLGSTRUCT_FLAGS.CONNDLG_PERSIST & CONNECTDLGSTRUCT_FLAGS.CONNDLG_NOT_PERSIST);
 			if (useMostRecentPath)
-				dialogOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_USE_MRU;
-			else
-				dialogOptions.dwFlags &= ~CONNECTDLGSTRUCT_FLAGS.CONNDLG_USE_MRU;
-
-			dialogOptions.hwndOwner = new(hWind);
+				connectDlgOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_USE_MRU;
+			if (readOnlyPath && !string.IsNullOrEmpty(remoteNetworkName))
+				connectDlgOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_RO_PATH;
 
-			dialogOptions.lpConnRes = &netRes;
-
-			if (readOnlyPath && !string.IsNullOrEmpty(netRes.lpRemoteName.ToString()))
-				dialogOptions.dwFlags |= CONNECTDLGSTRUCT_FLAGS.CONNDLG_RO_PATH;
+			fixed (char* pszRemoteName = remoteNetworkName)
+			{
+				netResource.dwType = NET_RESOURCE_TYPE.RESOURCETYPE_DISK;
+				netResource.lpRemoteName = pszRemoteName;
 
-			var result = PInvoke.WNetConnectionDialog1W(ref dialogOptions);
+				connectDlgOptions.cbStructure = (uint)sizeof(CONNECTDLGSTRUCTW);
+				connectDlgOptions.hwndOwner = new(hWnd);
+				connectDlgOptions.lpConnRes = &netResource;
 
-			dialogOptions.lpConnRes = null;
+				res = PInvoke.WNetConnectionDialog1W(ref connectDlgOptions);
+			}
 
-			if ((uint)result == unchecked((uint)-1))
+			// User canceled
+			if ((uint)res == unchecked((uint)-1))
 				return false;
 
-			if (result == 0)
-				throw new Win32Exception("Cannot display dialog");
+			// Unexpected error happened
+			if (res is not WIN32_ERROR.NO_ERROR)
+				throw new Win32Exception("Failed to process the network connection dialog successfully.");
 
 			return true;
 		}