Add support to ARM browsers #2937
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds ARM64 support by extending platform detection and adjusting browser data paths accordingly.
- Bumped package version to 20.2.2
- Introduced
Platform.LinuxArm64and updatedBrowserFetcherto detect Windows ARM64 on Windows 11 - Updated executable paths and archive names for Firefox, Chromium, ChromeHeadlessShell, and Chrome to include Linux ARM64
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/PuppeteerSharp/PuppeteerSharp.csproj | Bumped version from 20.2.1 to 20.2.2 |
| lib/PuppeteerSharp/Platform.cs | Added new Platform.LinuxArm64 enum |
| lib/PuppeteerSharp/BrowserFetcher.cs | Updated GetCurrentPlatform and added IsWindows11 helper |
| lib/PuppeteerSharp/BrowserData/Firefox.cs | Extended RelativeExecutablePath, GetPlatformNameForUrl, and archive methods to support LinuxArm64 |
| lib/PuppeteerSharp/BrowserData/Chromium.cs | Updated RelativeExecutablePath, ResolveDownloadPath, and archive methods for LinuxArm64 |
| lib/PuppeteerSharp/BrowserData/ChromeHeadlessShell.cs | Adjusted RelativeExecutablePath, ResolveDownloadPath, and folder logic for LinuxArm64 |
| lib/PuppeteerSharp/BrowserData/Chrome.cs | Added LinuxArm64 in system executable resolution and download path logic |
Comments suppressed due to low confidence (2)
lib/PuppeteerSharp/BrowserFetcher.cs:170
- [nitpick] Add an XML doc comment explaining the Windows 11 detection logic and why build number >= 22000 is used.
internal static bool IsWindows11()
lib/PuppeteerSharp/BrowserData/Firefox.cs:93
- Add unit tests covering the new
Platform.LinuxArm64branch inRelativeExecutablePathto verify correct executable paths for ARM64.
Platform.Linux or Platform.LinuxArm64 => Path.Combine("firefox", "firefox"),
| @@ -160,12 +160,16 @@ internal static Platform GetCurrentPlatform() | |||
|
|
|||
| if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | |||
| { | |||
| return RuntimeInformation.OSArchitecture == Architecture.X64 ? Platform.Win64 : Platform.Win32; | |||
| return RuntimeInformation.OSArchitecture == Architecture.X64 || | |||
There was a problem hiding this comment.
[nitpick] Consider introducing a dedicated Platform.WinArm64 enum or a helper to explicitly represent Windows ARM64 rather than mapping it to Win64 for clarity in ARM support.
Copilot uses AI. Check for mistakes.
| @@ -21,23 +21,23 @@ internal static string RelativeExecutablePath(Platform platform, string builId) | |||
| "Contents", | |||
| "MacOS", | |||
| "Chromium"), | |||
| Platform.Linux => Path.Combine("chrome-linux", "chrome"), | |||
| Platform.Linux or Platform.LinuxArm64 => Path.Combine("chrome-linux", "chrome"), | |||
There was a problem hiding this comment.
[nitpick] There's repeated grouping of Platform.Linux and Platform.LinuxArm64; consider extracting a helper (e.g., IsLinuxPlatform) or shared method to reduce duplication across switch statements.
| Platform.Linux or Platform.LinuxArm64 => Path.Combine("chrome-linux", "chrome"), | |
| _ when IsLinuxPlatform(platform) => Path.Combine("chrome-linux", "chrome"), |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
closes #2929