-
Notifications
You must be signed in to change notification settings - Fork 920
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
Use gin to add braveSkus handler #13645
Conversation
84b4af0
to
b9958e0
Compare
dea9eda
to
a2382a7
Compare
native implementation lgtm, note that it's still overrideable if it's part of |
5c1a3c9
to
b22e7bf
Compare
chrome exists at the time of call mostly but I wrapped it to be non-writable too |
thx, i downloaded this but seems |
08e5613
to
8a6fee4
Compare
Need to enable vpn and 2 skus flags, please pay attention I reworked test via the RenderViewTest and it allowed to keep urls hardcoded inside the renderer process as it was initially. Restarted new build with this changes if required https://ci.brave.com/job/test-brave-browser-build-macos-arm64/183/ cc @diracdeltas @bsclifton |
lgtm, what origins do we plan on having |
276c69c
to
bed123f
Compare
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.
++ on changes since I last viewed (ex: InitWithFeatures
adding SKU to test 😄)
Resolves brave/brave-browser#20437
Security review https://github.com/brave/security/issues/907
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: