Bug 1570575 - Some libpref tidy-ups. r=KrisWright

This commit:

- improves the wording of some comments;

- renames `UpdatePolicy` as `MirrorKind`, to reflect the new terminology we are
  starting to use;

- does a couple of other minor clean-ups.

Differential Revision: https://phabricator.services.mozilla.com/D40161

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Nicholas Nethercote 2019-08-02 06:36:14 +00:00
parent 905c456380
commit 605fd328b4
5 changed files with 83 additions and 89 deletions

View File

@ -1201,8 +1201,8 @@ using PrefsHashTable = HashSet<UniquePtr<Pref>, PrefHasher>;
static PrefsHashTable* gHashTable; static PrefsHashTable* gHashTable;
#ifdef DEBUG #ifdef DEBUG
// This defines the datatype used to store our `Once` StaticPrefs checker. // This defines the type used to store our `once` mirrors checker. We can't use
// We can't use HashMap for now due to alignment restrictions when dealing with // HashMap for now due to alignment restrictions when dealing with
// std::function<void()> (see bug 1557617). // std::function<void()> (see bug 1557617).
typedef std::function<void()> AntiFootgunCallback; typedef std::function<void()> AntiFootgunCallback;
struct CompareStr { struct CompareStr {
@ -1707,9 +1707,9 @@ static void NotifyCallbacks(const char* aPrefName, const PrefWrapper* aPref) {
if (XRE_IsParentProcess() && if (XRE_IsParentProcess() &&
!StaticPrefs::preferences_force_disable_check_once_policy() && !StaticPrefs::preferences_force_disable_check_once_policy() &&
(StaticPrefs::preferences_check_once_policy() || xpc::IsInAutomation())) { (StaticPrefs::preferences_check_once_policy() || xpc::IsInAutomation())) {
// Check that we aren't modifying a `Once` pref using that prefName. // Check that we aren't modifying a `once`-mirrored pref using that pref
// We have about 100 `Once` StaticPrefs defined. std::map performs a search // name. We have about 100 `once`-mirrored prefs. std::map performs a
// in O(log n), so this is fast enough for our case. // search in O(log n), so this is fast enough.
MOZ_ASSERT(gOnceStaticPrefsAntiFootgun); MOZ_ASSERT(gOnceStaticPrefsAntiFootgun);
auto search = gOnceStaticPrefsAntiFootgun->find(aPrefName); auto search = gOnceStaticPrefsAntiFootgun->find(aPrefName);
if (search != gOnceStaticPrefsAntiFootgun->end()) { if (search != gOnceStaticPrefsAntiFootgun->end()) {
@ -3724,8 +3724,8 @@ FileDescriptor Preferences::EnsureSnapshot(size_t* aSize) {
iter.get()->AddToMap(builder); iter.get()->AddToMap(builder);
} }
// Store the current value of Once StaticPrefs. Following this those // Store the current value of `once`-mirrored prefs. After this point they
// StaticPrefs will become immutable. // will be immutable.
StaticPrefs::RegisterOncePrefs(builder); StaticPrefs::RegisterOncePrefs(builder);
gSharedMap = new SharedPrefMap(std::move(builder)); gSharedMap = new SharedPrefMap(std::move(builder));
@ -5371,34 +5371,24 @@ static void InitPref(const char* aName, float aDefaultValue) {
} }
template <typename T> template <typename T>
static void InitVarCachePref(StaticPrefs::UpdatePolicy aPolicy, static void InitMirroredPref(StaticPrefs::MirrorKind aPolicy,
const nsACString& aName, T* aCache, const nsACString& aName, T* aCache,
StripAtomic<T> aDefaultValue, bool aIsStartup, StripAtomic<T> aDefaultValue, bool aIsStartup,
bool aIsParent) { bool aIsParent) {
// aIsStartup will be true when we first initialize the StaticPrefs and false // In the parent process, set/reset the pref value and the `always` mirror (if
// when we want to reset the Preferences/StaticPrefs to their default value. // there is one) to the default value.
// - `once` mirrors will be initialized lazily in InitOncePrefs().
// InitVarCachePref is called under the following scenarios: // - In child processes, the parent sends the correct initial values via
// aIsParent | aIsStartup | Action // shared memory, so we do not re-initialize them here.
// true | true | Set underlying preference and StaticPrefs to
// | | their default value, set callback for Live pref.
// true | false | reset underlying preference and StaticPref to
// | | default value.
// false | true | set callback for Live pref.
// false | false | none.
//
// We only set *aCache if the policy is Live as:
// 1- On startup, `Once` prefs will be initialized lazily in InitOncePrefs(),
// 2- After that, `Once` prefs are immutable.
if (aIsParent) { if (aIsParent) {
InitPref(PromiseFlatCString(aName).get(), aDefaultValue); InitPref(PromiseFlatCString(aName).get(), aDefaultValue);
if (MOZ_LIKELY(aPolicy == StaticPrefs::UpdatePolicy::Live)) { if (MOZ_LIKELY(aPolicy == StaticPrefs::MirrorKind::Always)) {
*aCache = aDefaultValue; *aCache = aDefaultValue;
} }
} }
if (MOZ_LIKELY(aPolicy == StaticPrefs::UpdatePolicy::Live) && // At startup, setup the callback for the `always` mirror (if there is one).
if (MOZ_LIKELY(aPolicy == StaticPrefs::MirrorKind::Always) &&
MOZ_LIKELY(aIsStartup)) { MOZ_LIKELY(aIsStartup)) {
AddVarCacheNoAssignment(aCache, aName, aDefaultValue); AddVarCacheNoAssignment(aCache, aName, aDefaultValue);
} }
@ -5411,7 +5401,8 @@ namespace StaticPrefs {
void MaybeInitOncePrefs() { void MaybeInitOncePrefs() {
if (MOZ_LIKELY(sOncePrefRead)) { if (MOZ_LIKELY(sOncePrefRead)) {
// `Once` StaticPrefs have already been initialized to their default value. // `once`-mirrored prefs have already been initialized to their default
// value.
return; return;
} }
StaticMutexAutoLock lock(sOncePrefMutex); StaticMutexAutoLock lock(sOncePrefMutex);
@ -5429,14 +5420,14 @@ void MaybeInitOncePrefs() {
// For a pref like this: // For a pref like this:
// //
// VARCACHE_PREF($POLICY, "my.pref", my_pref, my_pref, int32_t, 99) // VARCACHE_PREF($MIRROR, "my.pref", my_pref, my_pref, int32_t, 99)
// //
// we generate a variable definition like this: // we generate a variable definition like this:
// //
// int32_t sVarCache_my_pref(99); // int32_t sVarCache_my_pref(99);
// //
#define PREF(name, cpp_type, value) #define PREF(name, cpp_type, value)
#define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, default_value) \ #define VARCACHE_PREF(mirror, name, base_id, full_id, cpp_type, default_value) \
cpp_type sVarCache_##full_id(default_value); cpp_type sVarCache_##full_id(default_value);
#include "mozilla/StaticPrefListAll.h" #include "mozilla/StaticPrefListAll.h"
#undef PREF #undef PREF
@ -5450,19 +5441,19 @@ static void InitAll(bool aIsStartup) {
// For prefs like these: // For prefs like these:
// //
// PREF("foo.bar.baz", bool, true) // PREF("foo.bar.baz", bool, true)
// VARCACHE_PREF($POLICY, "my.pref", my_pref, my_pref, int32_t, 99) // VARCACHE_PREF($MIRROR, "my.pref", my_pref, my_pref, int32_t, 99)
// //
// we generate registration calls like this: // we generate registration calls like this:
// //
// if (isParent) { // if (isParent) {
// InitPref_bool("foo.bar.baz", true); // InitPref_bool("foo.bar.baz", true);
// } // }
// InitVarCachePref(UpdatePolicy::Live, "my.pref", &sVarCache_my_pref, // InitMirroredPref($MIRROR, "my.pref", &sVarCache_my_pref, 99, aIsStartup,
// 99, aIsStartup, isParent); // isParent);
// //
// The InitPref_*() functions have a type suffix to avoid ambiguity between // The InitPref_*() functions have a type suffix to avoid ambiguity between
// prefs having int32_t and float default values. That suffix is not needed // prefs having int32_t and float default values. That suffix is not needed
// for the InitVarCachePref() functions because they take a pointer parameter, // for the InitMirroredPref() functions because they take a pointer parameter,
// which prevents automatic int-to-float coercion. // which prevents automatic int-to-float coercion.
// //
// In content processes, we rely on the parent to send us the correct initial // In content processes, we rely on the parent to send us the correct initial
@ -5471,8 +5462,8 @@ static void InitAll(bool aIsStartup) {
if (isParent) { \ if (isParent) { \
InitPref_##cpp_type(name, value); \ InitPref_##cpp_type(name, value); \
} }
#define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, value) \ #define VARCACHE_PREF(mirror, name, base_id, full_id, cpp_type, value) \
InitVarCachePref(UpdatePolicy::policy, NS_LITERAL_CSTRING(name), \ InitMirroredPref(MirrorKind::mirror, NS_LITERAL_CSTRING(name), \
&sVarCache_##full_id, value, aIsStartup, isParent); &sVarCache_##full_id, value, aIsStartup, isParent);
#include "mozilla/StaticPrefListAll.h" #include "mozilla/StaticPrefListAll.h"
#undef PREF #undef PREF
@ -5482,26 +5473,27 @@ static void InitAll(bool aIsStartup) {
static void InitOncePrefs() { static void InitOncePrefs() {
// For a pref like this: // For a pref like this:
// //
// VARCACHE_PREF($POLICY, "my.pref", my_pref, my_pref, int32_t, 99) // VARCACHE_PREF($MIRROR, "my.pref", my_pref, my_pref, int32_t, 99)
// //
// we generate an initialization (in a non-DEBUG build) like this: // we generate a non-DEBUG initialization like this:
// //
// if (UpdatePolicy::$POLICY == UpdatePolicy::Once) { // if (MirrorKind::$MIRROR == MirrorKind::Once) {
// sVarCache_my_pref = Internals::GetPref("my.pref", 99); // sVarCache_my_pref = Internals::GetPref("my.pref", 99);
// } // }
// //
// This is done to get the potentially updated Preference value as we didn't // This is done in case the pref value was updated when reading pref data
// register a callback method for the `Once` policy. // files. It's necessary because we don't have callbacks registered for
// `once`-mirrored prefs.
// //
// On debug build, we also install a mechanism that allows to check if the // In debug builds, we also install a mechanism that can check if the
// original Preference is being modified once `Once` StaticPrefs have been // preference value is modified after `once`-mirrored prefs are initialized.
// initialized as this would indicate a likely misuse of `Once` StaticPrefs // In tests this would indicate a likely misuse of a `once`-mirrored pref and
// and that maybe instead they should have been made `Live`. // suggest that it should instead be `always`-mirrored.
// //
#define PREF(name, cpp_type, value) #define PREF(name, cpp_type, value)
#ifdef DEBUG #ifdef DEBUG
# define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, value) \ # define VARCACHE_PREF(mirror, name, base_id, full_id, cpp_type, value) \
if (UpdatePolicy::policy == UpdatePolicy::Once) { \ if (MirrorKind::mirror == MirrorKind::Once) { \
MOZ_ASSERT(gOnceStaticPrefsAntiFootgun); \ MOZ_ASSERT(gOnceStaticPrefsAntiFootgun); \
sVarCache_##full_id = \ sVarCache_##full_id = \
Internals::GetPref(name, StripAtomic<cpp_type>(value)); \ Internals::GetPref(name, StripAtomic<cpp_type>(value)); \
@ -5521,8 +5513,8 @@ static void InitOncePrefs() {
std::move(checkPref))); \ std::move(checkPref))); \
} }
#else #else
# define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, value) \ # define VARCACHE_PREF(mirror, name, base_id, full_id, cpp_type, value) \
if (UpdatePolicy::policy == UpdatePolicy::Once) { \ if (MirrorKind::mirror == MirrorKind::Once) { \
sVarCache_##full_id = \ sVarCache_##full_id = \
Internals::GetPref(name, StripAtomic<cpp_type>(value)); \ Internals::GetPref(name, StripAtomic<cpp_type>(value)); \
} }
@ -5596,24 +5588,25 @@ static void RegisterOncePrefs(SharedPrefMapBuilder& aBuilder) {
// For a pref like this: // For a pref like this:
// //
// VARCACHE_PREF($POLICY, "my.pref", my_pref, my_pref, int32_t, 99) // VARCACHE_PREF($MIRROR, "my.pref", my_pref, my_pref, int32_t, 99)
// //
// we generate a save call like this: // we generate a save call like this:
// //
// if (UpdatePolicy::$POLICY == UpdatePolicy::Once) { // if (MirrorKind::$MIRROR == MirrorKind::Once) {
// SaveOncePrefToSharedMap(aBuilder, ONCE_PREF_NAME(my.pref), // SaveOncePrefToSharedMap(aBuilder, ONCE_PREF_NAME(my.pref),
// sVarCache_my_pref); // sVarCache_my_pref);
// } // }
// //
// `Once` StaticPrefs values will be stored in a hidden and locked preferences // This saves the `once`-mirrored value as it was at parent startup. It is
// in the global SharedPreferenceMap. In order for those preferences to be // stored in a special (hidden and locked) entry in the global
// hidden and not appear in about:config nor ever be stored to disk, we add // SharedPreferenceMap. In order for the entry to be hidden and not appear in
// the "$$$" prefix and suffix to the preference name and set the IsVisible // about:config nor ever be stored to disk, we set its IsSkippedByIteration
// flag to false. // flag to true. We also distinguish it by adding a "$$$" prefix and suffix
// to the preference name.
// //
#define PREF(name, cpp_type, value) #define PREF(name, cpp_type, value)
#define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, value) \ #define VARCACHE_PREF(mirror, name, base_id, full_id, cpp_type, value) \
if (UpdatePolicy::policy == UpdatePolicy::Once) { \ if (MirrorKind::mirror == MirrorKind::Once) { \
SaveOncePrefToSharedMap(aBuilder, ONCE_PREF_NAME(name), \ SaveOncePrefToSharedMap(aBuilder, ONCE_PREF_NAME(name), \
StripAtomic<cpp_type>(sVarCache_##full_id)); \ StripAtomic<cpp_type>(sVarCache_##full_id)); \
} }
@ -5629,43 +5622,37 @@ static void InitStaticPrefsFromShared() {
// For a prefs like this: // For a prefs like this:
// //
// VARCACHE_PREF($POLICY, "my.pref", my_pref, my_pref, int32_t, 99) // VARCACHE_PREF($MIRROR, "my.pref", my_pref, my_pref, int32_t, 99)
// //
// we generate an initialization like this: // we generate an initialization like this:
// //
// { // {
// int32_t val; // int32_t val;
// nsresult rv; // nsresult rv = (MirrorKind::$MIRROR == MirrorKind::Once)
// if (UpdatePolicy::$POLICY == UpdatePolicy::Once) { // ? Internals::GetSharedPrefValue("$$$my.pref$$$", &val)
// rv = Internals::GetSharedPrefValue( // : Internals::GetSharedPrefValue("my.pref", &val);
// "$$$my.pref$$$", &val);
// } else if (UpdatePolicy::Once == UpdatePolicy::Live) {
// rv = Internals::GetSharedPrefValue("my.pref", &val);
// }
// MOZ_DIAGNOSTIC_ALWAYS_TRUE(NS_SUCCEEDED(rv)); // MOZ_DIAGNOSTIC_ALWAYS_TRUE(NS_SUCCEEDED(rv));
// sVarCache_my_pref = val; // sVarCache_my_pref = val;
// } // }
// //
#define PREF(name, cpp_type, value) #define PREF(name, cpp_type, value)
#define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, value) \ #define VARCACHE_PREF(mirror, name, base_id, full_id, cpp_type, value) \
{ \ { \
StripAtomic<cpp_type> val; \ StripAtomic<cpp_type> val; \
nsresult rv; \ nsresult rv = \
if (UpdatePolicy::policy == UpdatePolicy::Once) { \ (MirrorKind::mirror == MirrorKind::Once) \
rv = Internals::GetSharedPrefValue(ONCE_PREF_NAME(name), &val); \ ? Internals::GetSharedPrefValue(ONCE_PREF_NAME(name), &val) \
} else { \ : Internals::GetSharedPrefValue(name, &val); \
rv = Internals::GetSharedPrefValue(name, &val); \ MOZ_DIAGNOSTIC_ALWAYS_TRUE(NS_SUCCEEDED(rv)); \
} \ StaticPrefs::sVarCache_##full_id = val; \
MOZ_DIAGNOSTIC_ALWAYS_TRUE(NS_SUCCEEDED(rv)); \
StaticPrefs::sVarCache_##full_id = val; \
} }
#include "mozilla/StaticPrefListAll.h" #include "mozilla/StaticPrefListAll.h"
#undef PREF #undef PREF
#undef VARCACHE_PREF #undef VARCACHE_PREF
// `Once` StaticPrefs have been set to their value in the step above and // `once`-mirrored prefs have been set to their value in the step above and
// outside the parent process they are immutable. So we set sOncePrefRead // outside the parent process they are immutable. We set sOncePrefRead so
// so that we can directly skip any lazy initializations. // that we can directly skip any lazy initializations.
sOncePrefRead = true; sOncePrefRead = true;
} }

View File

@ -64,10 +64,17 @@ struct IsAtomic<std::atomic<T>> : TrueType {};
namespace StaticPrefs { namespace StaticPrefs {
// Undo X11/X.h's definition of `Always` so we can use it in `MirrorKind`.
#undef Always
// Enums for the update policy. // Enums for the update policy.
enum class UpdatePolicy { enum class MirrorKind {
Once, // Evaluate the preference once, unchanged during the session. // Mirror the pref value once, at startup.
Live // Evaluate the preference and set callback so it stays current/live. Once,
// Mirror the pref vale always, with live updating. This would be `Always`,
// but /usr/include/X11/X.h defines a macro with that name.
Always
}; };
void MaybeInitOncePrefs(); void MaybeInitOncePrefs();

View File

@ -16,14 +16,14 @@ namespace StaticPrefs {
// For a VarCache pref like this: // For a VarCache pref like this:
// //
// VARCACHE_PREF($POLICY, "my.pref", my_pref, my_pref, int32_t, 99) // VARCACHE_PREF($MIRROR, "my.pref", my_pref, my_pref, int32_t, 99)
// //
// we generate an extern variable declaration and three getter // we generate an extern variable declaration and three getter
// declarations/definitions. // declarations/definitions.
// //
// extern int32_t sVarCache_my_pref; // extern int32_t sVarCache_my_pref;
// inline int32_t my_pref() { // inline int32_t my_pref() {
// if (UpdatePolicy::$POLICY != UpdatePolicy::Once) { // if (MirrorKind::$MIRROR != MirrorKind::Once) {
// return sVarCache_my_pref; // return sVarCache_my_pref;
// } // }
// MaybeInitOncePrefs(); // MaybeInitOncePrefs();
@ -39,7 +39,7 @@ namespace StaticPrefs {
#define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, default_value) \ #define VARCACHE_PREF(policy, name, base_id, full_id, cpp_type, default_value) \
extern cpp_type sVarCache_##full_id; \ extern cpp_type sVarCache_##full_id; \
inline StripAtomic<cpp_type> full_id() { \ inline StripAtomic<cpp_type> full_id() { \
if (UpdatePolicy::policy != UpdatePolicy::Once) { \ if (MirrorKind::policy != MirrorKind::Once) { \
MOZ_DIAGNOSTIC_ASSERT( \ MOZ_DIAGNOSTIC_ASSERT( \
IsAtomic<cpp_type>::value || NS_IsMainThread(), \ IsAtomic<cpp_type>::value || NS_IsMainThread(), \
"Non-atomic static pref '" name \ "Non-atomic static pref '" name \

View File

@ -65,7 +65,7 @@ VARCACHE_PREF(
'always': '''\ 'always': '''\
VARCACHE_PREF( VARCACHE_PREF(
Live, Always,
"{name}", "{name}",
{base_id}, {base_id},
{full_id}, {full_id},

View File

@ -100,7 +100,7 @@ VARCACHE_PREF(
) )
VARCACHE_PREF( VARCACHE_PREF(
Live, Always,
"my.uint", "my.uint",
my_uint, my_uint,
my_uint, my_uint,
@ -120,7 +120,7 @@ PREF("my.string", String, "foo\\"bar")
PREF("my.string2", String, "foobar") PREF("my.string2", String, "foobar")
VARCACHE_PREF( VARCACHE_PREF(
Live, Always,
"my.atomic.bool", "my.atomic.bool",
my_atomic_bool, my_atomic_bool,
my_atomic_bool, my_atomic_bool,
@ -128,7 +128,7 @@ VARCACHE_PREF(
) )
VARCACHE_PREF( VARCACHE_PREF(
Live, Always,
"my.atomic.int", "my.atomic.int",
my_atomic_int, my_atomic_int,
my_atomic_int_DoNotUseDirectly, my_atomic_int_DoNotUseDirectly,