Skip to content

Code Quality: Removed the MMI reference for AOT compatibility#18644

Open
0x5bfa wants to merge 5 commits into
files-community:mainfrom
0x5bfa:mmi
Open

Code Quality: Removed the MMI reference for AOT compatibility#18644
0x5bfa wants to merge 5 commits into
files-community:mainfrom
0x5bfa:mmi

Conversation

@0x5bfa

@0x5bfa 0x5bfa commented Jun 25, 2026

Copy link
Copy Markdown
Member

Resolved / Related Issues

This project has been abandoned and replaced by System.Management but this is also AOT unsafe. Plus, we no longer have to use this safe code after we migrated to WASDK. I've replaced the MMI references with COM APIs.

Steps used to test these changes

  1. Open Files app
  2. Insert/eject a removable media
  3. See the drive list is updated

Copilot AI review requested due to automatic review settings June 25, 2026 03:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the legacy Microsoft.Management.Infrastructure (MMI) / WMI-based drive detection code from the WinUI app and replaces it with a CsWin32-backed Shell notification manager to improve AOT compatibility and reduce dependency surface area.

Changes:

  • Removed MMI package references and eliminated WMI watcher/helper types that depended on MMI.
  • Replaced Win32 drive insertion/ejection monitoring with a new WindowsDriveManager using SHChangeNotifyRegister + a hidden window WndProc.
  • Cleaned up About page third-party references accordingly.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Files.App/ViewModels/Settings/AboutViewModel.cs Removes the MMI attribution/link from the About page.
src/Files.App/Utils/Storage/Helpers/DriveHelpers.cs Drops the MMI-backed GetVolumeId helper and its import.
src/Files.App/Utils/Global/WindowsStorageDeviceWatcher.cs Switches from DeviceManager to WindowsDriveManager and starts/stops the new manager.
src/Files.App/Utils/Storage/Helpers/DeviceManager.cs Deletes the old WMI/MMI-based device watcher implementation.
src/Files.App/Helpers/WMI/WqlEventQuery.cs Deletes WMI query helper type tied to the old watcher.
src/Files.App/Helpers/WMI/ManagementEventWatcher.cs Deletes MMI-based event watcher implementation.
src/Files.App/Data/EventArguments/EventArrivedEventArgs.cs Deletes event args type used only by the old MMI watcher.
src/Files.App/Files.App.csproj Removes MMI package references from the app project.
src/Files.App.Storage/Windows/Managers/WindowsDriveManager.cs Adds a new Shell notification-based drive manager (CsWin32).
src/Files.App.CsWin32/NativeMethods.txt Adds required Shell APIs/types for the new drive manager.
src/Files.App.CsWin32/ManualGuid.cs Adds FOLDERID_ComputerFolder GUID used to scope Shell notifications.
Directory.Packages.props Removes MMI package versions from central package management.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Files.App.Storage/Windows/Managers/WindowsDriveManager.cs Outdated
@yair100

yair100 commented Jun 26, 2026

Copy link
Copy Markdown
Member

This needs some testing.

@yair100 yair100 added the ready for review Pull requests that are ready for review label Jun 29, 2026
@yair100 yair100 requested review from Josh65-2201 and hez2010 June 29, 2026 19:13
Josh65-2201
Josh65-2201 previously approved these changes Jun 29, 2026
{
public unsafe sealed class WindowsDriveManager : IDisposable
{
private static readonly Lazy<WindowsDriveManager> lazy = new(() => new WindowsDriveManager());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to use a lazy here. static fields are already only initializing on the first access.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've removed Lazy and just used new. In the future, I'd like to put this to the root app model and avoid Default but we can't do it but do this.

Comment on lines +73 to +74
wndClass.lpfnWndProc = (delegate* unmanaged[Stdcall]<HWND, uint, WPARAM, LPARAM, LRESULT>)
Marshal.GetFunctionPointerForDelegate(_wndProc);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make WndProc static, marking it as UnmanagedCallersOnly so that we can take a function pointer directly instead of acquiring a function pointer from an instance delegate?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since the window proc accesses instance-only FolderChanged event.

@hez2010 hez2010 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Pull requests that are ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants