-
Notifications
You must be signed in to change notification settings - Fork 869
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
fix incorrect and missing defaults for content settings #13426
Conversation
@boocmp fyi |
25548d0
to
5d6f742
Compare
browser/content_settings/brave_content_settings_registry_unittest.cc
Outdated
Show resolved
Hide resolved
1887103
to
836ef6f
Compare
@@ -29,6 +31,9 @@ class CookiePrefService : public KeyedService, | |||
CookiePrefService& operator=(const CookiePrefService&) = delete; | |||
~CookiePrefService() override; | |||
|
|||
static ControlType CookieControlsModeToControlType( |
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.
why is it here now? can't find where it is used outside of .cc
components/content_settings/core/browser/brave_content_settings_default_provider.cc
Show resolved
Hide resolved
browser/browsing_data/brave_browsing_data_remover_delegate_unittest.cc
Outdated
Show resolved
Hide resolved
chromium_src/components/content_settings/core/common/content_settings.cc
Outdated
Show resolved
Hide resolved
// TODO(bridiver) - DEFAULT is not a valid setting, but leaving it for | ||
// now because this requires more extensive changes to fix correctly | ||
// { | ||
// SCOPED_TRACE("Content setting: BRAVE_FINGERPRINTING_V2"); |
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.
i'd rather keep checks that assert "not valid" things then keep something commented and not even compiled
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.
uncommenting these causes a crash because GetInitialDefaultSetting DCHECKs for invalid values
20b7649
to
f11ff89
Compare
unrelated test failures on macos |
Resolves brave/brave-browser#23031
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: