-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Core - Update CefErrorCode and PermissionRequestType #5213
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe pull request updates two CefSharp enum files: CefErrorCode now includes seven new error codes (DNS errors, incognito blocking, HTTP engine placeholder) while removing eight obsolete codes; PermissionRequestType adds two new permission flags for local and loopback network access. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🤖 Fix all issues with AI agents
In `@CefSharp/Enums/CefErrorCode.cs`:
- Around line 1364-1376: The enum member name HttpengineProviderInUse in the
CefErrorCode enum uses incorrect PascalCase; rename it to
HttpEngineProviderInUse to match the file's naming conventions (e.g.,
Http2ProtocolError, Http11Required) and keep the same underlying value (-508)
and existing XML doc comment unchanged; update any references/usages of
CefErrorCode.HttpengineProviderInUse throughout the codebase to
CefErrorCode.HttpEngineProviderInUse to avoid build errors.
| /// <summary> | ||
| /// This is a placeholder value that should never be used within //net. | ||
| /// | ||
| /// When Cronet APIs are being backed by HttpEngine (i.e., HttpEngineProvider is | ||
| /// being used), org.chromium.net.NetworkException#getCronetInternalErrorCode is | ||
| /// not supported (android.net.http.NetworkException#getCronetInternalErrorCode | ||
| /// does not exist). In this scenario, getCronetInternalErrorCode will always | ||
| /// return this error. This is a first step towards the deprecation of | ||
| /// getCronetInternalErrorCode. | ||
| /// | ||
| /// Temporarily terminate, then restart, ITTT to avoid unsupported nesting. | ||
| /// </summary> | ||
| HttpengineProviderInUse = -508, |
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.
🧩 Analysis chain
🏁 Script executed:
# Search for Http2* and Http11* enum values in the CefErrorCode.cs file to understand the naming pattern
cat CefSharp/Enums/CefErrorCode.cs | grep -E "(Http2|Http11)" | head -20Repository: cefsharp/CefSharp
Length of output: 515
🌐 Web query:
Chromium error code -508 net_error_list.h constant name
💡 Result:
Chromium net error code -508 corresponds to:
- Constant in
net_error_list.h:HTTPENGINE_PROVIDER_IN_USE - Enum / error name used in code:
net::ERR_HTTPENGINE_PROVIDER_IN_USE[1]
🏁 Script executed:
# Also search for other HttpEngine references in the file to understand context
rg "HttpEngine" CefSharp/Enums/CefErrorCode.csRepository: cefsharp/CefSharp
Length of output: 149
Rename to HttpEngineProviderInUse for PascalCase consistency.
The enum violates the established naming pattern used throughout the file (e.g., Http2ProtocolError, Http11Required). The acronym "Engine" should be capitalized as part of the compound term.
🔧 Proposed fix
- HttpengineProviderInUse = -508,
+ HttpEngineProviderInUse = -508,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// <summary> | |
| /// This is a placeholder value that should never be used within //net. | |
| /// | |
| /// When Cronet APIs are being backed by HttpEngine (i.e., HttpEngineProvider is | |
| /// being used), org.chromium.net.NetworkException#getCronetInternalErrorCode is | |
| /// not supported (android.net.http.NetworkException#getCronetInternalErrorCode | |
| /// does not exist). In this scenario, getCronetInternalErrorCode will always | |
| /// return this error. This is a first step towards the deprecation of | |
| /// getCronetInternalErrorCode. | |
| /// | |
| /// Temporarily terminate, then restart, ITTT to avoid unsupported nesting. | |
| /// </summary> | |
| HttpengineProviderInUse = -508, | |
| /// <summary> | |
| /// This is a placeholder value that should never be used within //net. | |
| /// | |
| /// When Cronet APIs are being backed by HttpEngine (i.e., HttpEngineProvider is | |
| /// being used), org.chromium.net.NetworkException#getCronetInternalErrorCode is | |
| /// not supported (android.net.http.NetworkException#getCronetInternalErrorCode | |
| /// does not exist). In this scenario, getCronetInternalErrorCode will always | |
| /// return this error. This is a first step towards the deprecation of | |
| /// getCronetInternalErrorCode. | |
| /// | |
| /// Temporarily terminate, then restart, ITTT to avoid unsupported nesting. | |
| /// </summary> | |
| HttpEngineProviderInUse = -508, |
🤖 Prompt for AI Agents
In `@CefSharp/Enums/CefErrorCode.cs` around lines 1364 - 1376, The enum member
name HttpengineProviderInUse in the CefErrorCode enum uses incorrect PascalCase;
rename it to HttpEngineProviderInUse to match the file's naming conventions
(e.g., Http2ProtocolError, Http11Required) and keep the same underlying value
(-508) and existing XML doc comment unchanged; update any references/usages of
CefErrorCode.HttpengineProviderInUse throughout the codebase to
CefErrorCode.HttpEngineProviderInUse to avoid build errors.
|
✅ Build CefSharp 144.0.120-CI5413 completed (commit 904e1bac29 by @campersau) |
PermissinRequestType got updated again in 145.0.7632.0
Summary:
Updated
CefErrorCodeusing https://raw.githubusercontent.com/chromium/chromium/145.0.7632.0/net/base/net_error_list.h and steps described in #3785Updated
PermissionRequestTypeusing https://github.com/chromiumembedded/cef/blob/bfb74aa1a42355203a43fa89f38e7737e954f67f/include/internal/cef_types.h#L3822-L3856Changes:
How Has This Been Tested?
Build works.
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.