-
Notifications
You must be signed in to change notification settings - Fork 868
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
Wired up BraveVpnService with BraveVPNOSConnectionAPI #9836
Conversation
fix brave/brave-browser#17658 Two cmd switches are added. Use "--brave-vpn-test-credentials=connection-name:host-name:user-name:password" to pass testable credentials for connecting to guardian service. This will be useful before we can get real credentials from payment service. Use "--brave-vpn-simulation" to simulate os vpn connection instead of call os' vpn api. This will be useful when it's difficult to call os'vpn api such as macOS.
components/brave_vpn/switches.h
Outdated
// Value should be "connection-name:host-name:user-name:password". | ||
constexpr char kBraveVPNTestCredentials[] = "brave-vpn-test-credentials"; | ||
// Use for simulation instead of calling os platform apis. | ||
constexpr char kBraveVPNSimulation[] = "brave-vpn-simulation"; |
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.
not sure if we're using "simulation" in other modules but i'd think grammatically this would be ideal:
--brave-vpn-simulate
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.
Updated.
Made BraveVpnService common on all platform and added desktop specific impls to BraveVpnServiceDesktop.
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.
LGTM
Set skipped because last change was just switch name string. |
fix brave/brave-browser#17658
Two cmd switches are added.
Use "--brave-vpn-test-credentials=connection-name:host-name:user-name:password" to pass
testable credentials for connecting to guardian service.
This will be useful before we can get real credentials from payment service.
Use "--brave-vpn-simulate" to simulate os vpn connection instead of call os' vpn api.
This will be useful when it's difficult to call os'vpn api such as macOS.
and Introduced
BraveVpnServiceDesktop
instead of adding desktop's impl toBraveVpnService
because android also uses
BraveVpnService
.Resolves
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: