-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Core - Update CefErrorCode and PermissionRequestType #5205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Core - Update CefErrorCode and PermissionRequestType #5205
Conversation
WalkthroughThis PR updates two enum files in CefSharp: CefErrorCode adds proxy, DNS cache invalidation, and blob-related error codes while removing H2OrQuicRequired and renaming local network access policy codes; PermissionRequestType removes AccessibilityEvents, renames Identity_Provider to IdentityProvider, and adds new permission types with bit-flag reordering. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CefSharp/Enums/CefErrorCode.cs(6 hunks)CefSharp/Enums/PermissionRequestType.cs(2 hunks)
🔇 Additional comments (8)
CefSharp/Enums/PermissionRequestType.cs (2)
40-43: Bit position shifts are a subtle but significant breaking change.
WindowManagementmoved from1<<22to1<<23andFileSystemAccessmoved from1<<23to1<<24. This means any persisted/serialized permission values or bitwise comparisons using the old positions will now be misinterpreted.
WebAppInstallation(1<<22) andLocalNetworkAccess(1<<25) are correctly added per the PR objective.If permission flags are persisted anywhere (databases, configs), consumers will need a migration strategy.
17-28: Breaking changes: member removal and renaming require documentation.PermissionRequestType should be kept in sync with Chromium's permissions::RequestType type. The changes indicate
AccessibilityEventshas been removed andIdentity_Providerhas been renamed toIdentityProvider. These are breaking changes for consumers of this enum.Ensure release notes document these breaking changes and provide migration guidance for code using the removed or renamed members.
CefSharp/Enums/CefErrorCode.cs (6)
32-32: LGTM! Range documentation added.The addition of the blob error range (900-999) appropriately documents the new error codes added later in the file.
189-190: LGTM! Proper documentation of removed error code.Documenting the removal of
H2_OR_QUIC_REQUIREDwhile preserving the value slot is good practice for maintaining backward compatibility and preventing value reuse.
267-268: LGTM! Improved cross-reference documentation.The updated documentation properly references
PROXY_UNABLE_TO_CONNECT_TO_DESTINATION, which helps clarify the relationship between these related error codes.
1547-1551: LGTM! Well-documented DNS error addition.The
DnsCacheInvalidationInProgresserror code is properly documented with clear guidance for callers about its transient nature and retry semantics.
1553-1594: LGTM! Comprehensive blob error code additions.The blob error codes are well-documented and provide comprehensive coverage of blob-related failure scenarios. The mapping from
storage::BlobStatusis clearly indicated, and each error code has descriptive documentation explaining the specific failure condition.
1236-1236: Breaking change: Enum members renamed to align with Chromium terminology.The following error codes have been renamed:
CachedIpAddressSpaceBlockedByPrivateNetworkAccessPolicy→CachedIpAddressSpaceBlockedByLocalNetworkAccessPolicyBlockedByPrivateNetworkAccessChecks→BlockedByLocalNetworkAccessChecksNo internal references to the old names exist in the codebase. The breaking change is isolated to external consumers of this enum.
|
✅ Build CefSharp 143.0.90-CI5404 completed (commit ef4dea3d83 by @campersau) |
|
Thanks, will look at merging shortly. |
I needed the new
PermissionRequestType.LocalNetworkAccessbut it was missing, so I synced it and also updatedCefErrorCodeagain.Summary:
Updated
CefErrorCodeusing https://raw.githubusercontent.com/chromium/chromium/143.0.7499.40/net/base/net_error_list.h and steps described in #3785Updated
PermissionRequestTypeusing https://github.com/chromiumembedded/cef/blob/8aed01b55e1b242b2b3df82a87f321a5cc2b6762/include/internal/cef_types.h#L3812-L3847Changes:
How Has This Been Tested?
Build works.
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.