Register prefs in right places #3031
Conversation
| @@ -908,6 +909,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry, | ||
| #endif | ||
|
|
||
| RegisterProfilePrefsForMigration(registry); |
There was a problem hiding this comment.
do we need a patch here? Can't we just do a chromium_src override for RegisterProfilePrefs?
There was a problem hiding this comment.
that will also override the calling from RegisterUserProfilePrefs and RegisterSigninProfilePrefs to be RegisterProfilePrefs_ChromiumImpl and no one calls our overridden RegisterProfilePrefs
There was a problem hiding this comment.
Also it overrides extensions::ActivityLog::RegisterProfilePrefs => extensions::ActivityLog::RegisterProfilePrefs_ChromiumImpl and bunch of same cases
| LoadPrefsForNormalStartup(async_prefs); | ||
| #endif | ||
|
|
||
| +// Done here instead of browser_prefs.cc so it can override default prefs |
There was a problem hiding this comment.
@darkdh did you verify that this still works correctly?
There was a problem hiding this comment.
yes, if we override default at wrong place, we will get a DCHECK failure saying using unregistered prefs
darkdh
force-pushed
the
reg-pref-rework
branch
2 times, most recently
from
1bb111a to
c6660c6
Compare
1. Prefs used by keyed service must be registered or set default in RegisterProfilePrefs of pref_service_builder_utils 2. RegisterProfilePrefs or RegisterLocalStatePrefs depends on scope
|
this only error https://staging.ci.brave.com/job/brave-browser-build-pr/job/reg-pref-rework/9/execution/node/359/log/ doesn't seem like related to code changes |
Regression of #3031. Each unit test doesn't need to register specific prefs after above PR.
RegisterProfilePrefs of pref_service_builder_utils
fix brave/brave-browser#5414
Submitter Checklist:
npm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.