Bug 1477904: Correctly handle static var caches with changed default values. r?njn
MozReview-Commit-ID: H2ImQrmrtAV
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -461,16 +461,17 @@ class PrefWrapper;
class Pref
{
public:
explicit Pref(const char* aName)
: mName(ArenaStrdup(aName, gPrefNameArena))
, mType(static_cast<uint32_t>(PrefType::None))
, mIsSticky(false)
, mIsLocked(false)
+ , mDefaultChanged(false)
, mHasDefaultValue(false)
, mHasUserValue(false)
, mDefaultValue()
, mUserValue()
{
}
~Pref()
@@ -496,26 +497,32 @@ public:
bool IsTypeInt() const { return IsType(PrefType::Int); }
bool IsTypeBool() const { return IsType(PrefType::Bool); }
// Other properties.
bool IsLocked() const { return mIsLocked; }
void SetIsLocked(bool aValue) { mIsLocked = aValue; }
+ bool DefaultChanged() const { return mDefaultChanged; }
+
bool IsSticky() const { return mIsSticky; }
bool HasDefaultValue() const { return mHasDefaultValue; }
bool HasUserValue() const { return mHasUserValue; }
template<typename T>
void AddToMap(SharedPrefMapBuilder& aMap)
{
aMap.Add(Name(),
- { HasDefaultValue(), HasUserValue(), IsSticky(), IsLocked() },
+ { HasDefaultValue(),
+ HasUserValue(),
+ IsSticky(),
+ IsLocked(),
+ DefaultChanged() },
HasDefaultValue() ? mDefaultValue.Get<T>() : T(),
HasUserValue() ? mUserValue.Get<T>() : T());
}
void AddToMap(SharedPrefMapBuilder& aMap)
{
if (IsTypeBool()) {
AddToMap<bool>(aMap);
@@ -708,16 +715,19 @@ public:
// Should we set the default value? Only if the pref is not locked, and
// doing so would change the default value.
if (!IsLocked()) {
if (aIsLocked) {
SetIsLocked(true);
}
if (!ValueMatches(PrefValueKind::Default, aType, aValue)) {
mDefaultValue.Replace(mHasDefaultValue, Type(), aType, aValue);
+ if (mHasDefaultValue) {
+ mDefaultChanged = true;
+ }
mHasDefaultValue = true;
if (aIsSticky) {
mIsSticky = true;
}
if (!mHasUserValue) {
*aValueChanged = true;
}
// What if we change the default to be the same as the user value?
@@ -931,16 +941,17 @@ public:
}
private:
const char* mName; // allocated in gPrefNameArena
uint32_t mType : 2;
uint32_t mIsSticky : 1;
uint32_t mIsLocked : 1;
+ uint32_t mDefaultChanged : 1;
uint32_t mHasDefaultValue : 1;
uint32_t mHasUserValue : 1;
PrefValue mDefaultValue;
PrefValue mUserValue;
};
class PrefEntry : public PLDHashEntryHdr
@@ -1003,16 +1014,17 @@ public:
struct Matcher \
{ \
retType match(const Pref* aPref) { return aPref->method(); } \
retType match(SharedPref& aPref) { return aPref.method(); } \
}; \
return match(Matcher()); \
}
+ FORWARD(bool, DefaultChanged)
FORWARD(bool, IsLocked)
FORWARD(bool, IsSticky)
FORWARD(bool, HasDefaultValue)
FORWARD(bool, HasUserValue)
FORWARD(const char*, Name)
FORWARD(nsCString, NameString)
FORWARD(PrefType, Type)
#undef FORWARD
@@ -4825,33 +4837,29 @@ Preferences::InitInitialObjects(bool aIs
if (!XRE_IsParentProcess()) {
MOZ_ASSERT(gSharedMap);
// We got our initial preference values from the content process, so we
// don't need to add them to the DB. For static var caches, though, the
// current preference values may differ from their static defaults. So we
// still need to notify callbacks for each of our shared prefs which have
- // user values.
- //
- // While it is technically also possible for the default values to have
- // changed at runtime, and therefore not match the static defaults, we don't
- // support that for static preferences in this configuration, and therefore
- // ignore the possibility.
+ // user values, of whose default values have changed since they were
+ // initialized.
for (auto& pref : gSharedMap->Iter()) {
- if (pref.HasUserValue() || pref.IsLocked()) {
+ if (pref.HasUserValue() || pref.DefaultChanged()) {
NotifyCallbacks(pref.Name(), PrefWrapper(pref));
}
}
#ifdef DEBUG
- // Check that all varcache preferences match their current values. This can
- // currently fail if the default value of a static varcache preference is
- // changed in a preference file or at runtime, rather than in
- // StaticPrefList.h.
+ // Check that all varcache preferences match their current values. This
+ // can currently fail if the default value of a static varcache preference
+ // is changed in a preference file or at runtime, rather than in
+ // StaticPrefList.h.
#define PREF(name, cpp_type, value)
#define VARCACHE_PREF(name, id, cpp_type, value) \
MOZ_ASSERT(GetPref<StripAtomic<cpp_type>>(name, value) == StaticPrefs::id(), \
"Incorrect cached value for " name);
#include "mozilla/StaticPrefList.h"
#undef PREF
#undef VARCACHE_PREF
--- a/modules/libpref/SharedPrefMap.cpp
+++ b/modules/libpref/SharedPrefMap.cpp
@@ -93,16 +93,17 @@ SharedPrefMapBuilder::Add(const char* aK
aKey,
mKeyTable.Add(aKey),
{ aDefaultValue, aUserValue },
uint8_t(PrefType::Bool),
aFlags.mHasDefaultValue,
aFlags.mHasUserValue,
aFlags.mIsSticky,
aFlags.mIsLocked,
+ aFlags.mDefaultChanged,
});
}
void
SharedPrefMapBuilder::Add(const char* aKey,
const Flags& aFlags,
int32_t aDefaultValue,
int32_t aUserValue)
@@ -118,16 +119,17 @@ SharedPrefMapBuilder::Add(const char* aK
aKey,
mKeyTable.Add(aKey),
{ index },
uint8_t(PrefType::Int),
aFlags.mHasDefaultValue,
aFlags.mHasUserValue,
aFlags.mIsSticky,
aFlags.mIsLocked,
+ aFlags.mDefaultChanged,
});
}
void
SharedPrefMapBuilder::Add(const char* aKey,
const Flags& aFlags,
const nsCString& aDefaultValue,
const nsCString& aUserValue)
@@ -145,16 +147,17 @@ SharedPrefMapBuilder::Add(const char* aK
aKey,
mKeyTable.Add(aKey),
{ index },
uint8_t(PrefType::String),
aFlags.mHasDefaultValue,
aFlags.mHasUserValue,
aFlags.mIsSticky,
aFlags.mIsLocked,
+ aFlags.mDefaultChanged,
});
}
Result<Ok, nsresult>
SharedPrefMapBuilder::Finalize(loader::AutoMemMap& aMap)
{
using Header = SharedPrefMap::Header;
@@ -211,17 +214,17 @@ SharedPrefMapBuilder::Finalize(loader::A
headerPtr[0] = header;
auto* entryPtr = reinterpret_cast<SharedPrefMap::Entry*>(&headerPtr[1]);
for (auto* entry : entries) {
*entryPtr = {
entry->mKey, GetValue(*entry),
entry->mType, entry->mHasDefaultValue,
entry->mHasUserValue, entry->mIsSticky,
- entry->mIsLocked,
+ entry->mIsLocked, entry->mDefaultChanged,
};
entryPtr++;
}
auto ptr = mem.Get<uint8_t>();
mKeyTable.Write(
{ &ptr[header.mKeyStrings.mOffset], header.mKeyStrings.mSize });
--- a/modules/libpref/SharedPrefMap.h
+++ b/modules/libpref/SharedPrefMap.h
@@ -313,16 +313,18 @@ class SharedPrefMap
uint8_t mHasDefaultValue : 1;
// True if the preference has a user value. Callers must not attempt to
// access the entry's user value if this is false.
uint8_t mHasUserValue : 1;
// True if the preference is sticky, as defined by the preference service.
uint8_t mIsSticky : 1;
// True if the preference is locked, as defined by the preference service.
uint8_t mIsLocked : 1;
+ // True if the preference's default value has changed since it was first set.
+ uint8_t mDefaultChanged : 1;
};
public:
NS_INLINE_DECL_REFCOUNTING(SharedPrefMap)
// A temporary wrapper class for accessing entries in the array. Instances of
// this class are valid as long as SharedPrefMap instance is alive, but
// generally should not be stored long term, or allocated on the heap.
@@ -340,16 +342,17 @@ public:
nsCString NameString() const { return mMap->KeyTable().Get(mEntry->mKey); }
PrefType Type() const
{
MOZ_ASSERT(PrefType(mEntry->mType) != PrefType::None);
return PrefType(mEntry->mType);
}
+ bool DefaultChanged() const { return mEntry->mDefaultChanged; }
bool HasDefaultValue() const { return mEntry->mHasDefaultValue; }
bool HasUserValue() const { return mEntry->mHasUserValue; }
bool IsLocked() const { return mEntry->mIsLocked; }
bool IsSticky() const { return mEntry->mIsSticky; }
bool GetBoolValue(PrefValueKind aKind = PrefValueKind::User) const
{
MOZ_ASSERT(Type() == PrefType::Bool);
@@ -584,16 +587,17 @@ public:
// The set of flags for the preference, as documented in SharedPrefMap::Entry.
struct Flags
{
uint8_t mHasDefaultValue : 1;
uint8_t mHasUserValue : 1;
uint8_t mIsSticky : 1;
uint8_t mIsLocked : 1;
+ uint8_t mDefaultChanged : 1;
};
void Add(const char* aKey,
const Flags& aFlags,
bool aDefaultValue,
bool aUserValue);
void Add(const char* aKey,
@@ -855,16 +859,17 @@ private:
StringTableEntry mKey;
Value mValue;
uint8_t mType : 2;
uint8_t mHasDefaultValue : 1;
uint8_t mHasUserValue : 1;
uint8_t mIsSticky : 1;
uint8_t mIsLocked : 1;
+ uint8_t mDefaultChanged : 1;
};
// Converts a builder Value struct to a SharedPrefMap::Value struct for
// serialization. This must not be called before callers have finished adding
// entries to the value array builders.
SharedPrefMap::Value GetValue(const Entry& aEntry) const
{
switch (PrefType(aEntry.mType)) {
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -144,24 +144,28 @@ VARCACHE_PREF(
#endif
VARCACHE_PREF(
"dom.animations-api.timelines.enabled",
dom_animations_api_timelines_enabled,
bool, PREF_VALUE
)
#undef PREF_VALUE
+// NOTE: This preference is used in unit tests. If it is removed or its default
+// value changes, please update test_sharedMap_var_caches.js accordingly.
VARCACHE_PREF(
"dom.webcomponents.shadowdom.report_usage",
dom_webcomponents_shadowdom_report_usage,
bool, false
)
// Whether we disable triggering mutation events for changes to style
// attribute via CSSOM.
+// NOTE: This preference is used in unit tests. If it is removed or its default
+// value changes, please update test_sharedMap_var_caches.js accordingly.
VARCACHE_PREF(
"dom.mutation-events.cssom.disabled",
dom_mutation_events_cssom_disabled,
bool, true
)
VARCACHE_PREF(
"dom.performance.enable_scheduler_timing",
new file mode 100644
--- /dev/null
+++ b/modules/libpref/test/unit_ipc/test_sharedMap_var_caches.js
@@ -0,0 +1,66 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+// Tests that static preference variable caches in the content process
+// correctly handle values which are different from their
+// statically-defined defaults.
+//
+// Since we can't access variable cache values from JS, this tests
+// relies on assertions in debug builds to detect mismatches. The
+// default and user values of two preferences are changed (respectively)
+// before a content process is started. Once the content process is
+// launched, the preference service asserts that the values stored in
+// all var caches match their current values as known to the preference
+// service. If there's a mismatch, the shell will crash, and the test
+// will fail.
+//
+// For sanity, we also check that the dynamically retrieved preference
+// values in the content process match our expectations, though this is
+// not strictly part of the test.
+
+const PREF1_NAME = "dom.webcomponents.shadowdom.report_usage";
+const PREF1_VALUE = false;
+
+const PREF2_NAME = "dom.mutation-events.cssom.disabled"
+const PREF2_VALUE = true;
+
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.import("resource://testing-common/ExtensionXPCShellUtils.jsm");
+
+ExtensionTestUtils.init(this);
+
+let contentPage;
+
+const {prefs} = Services;
+const defaultPrefs = prefs.getDefaultBranch("");
+
+add_task(async function test_sharedMap_var_caches() {
+ equal(prefs.getBoolPref(PREF1_NAME), PREF1_VALUE,
+ `Expected initial value for ${PREF1_NAME}`);
+ equal(prefs.getBoolPref(PREF2_NAME), PREF2_VALUE,
+ `Expected initial value for ${PREF2_NAME}`);
+
+ defaultPrefs.setBoolPref(PREF1_NAME, !PREF1_VALUE);
+ prefs.setBoolPref(PREF2_NAME, !PREF2_VALUE);
+
+ equal(prefs.getBoolPref(PREF1_NAME), !PREF1_VALUE,
+ `Expected updated value for ${PREF1_NAME}`);
+ equal(prefs.getBoolPref(PREF2_NAME), !PREF2_VALUE,
+ `Expected updated value for ${PREF2_NAME}`);
+
+ let contentPage = await ExtensionTestUtils.loadContentPage("about:blank", {remote: true});
+ registerCleanupFunction(() => contentPage.close());
+
+ let values = await contentPage.spawn([PREF1_NAME, PREF2_NAME], (prefs) => {
+ ChromeUtils.import("resource://gre/modules/Services.jsm");
+ return prefs.map(pref => Services.prefs.getBoolPref(pref));
+ })
+
+ equal(values[0], !PREF1_VALUE,
+ `Expected content value for ${PREF1_NAME}`);
+ equal(values[1], !PREF2_VALUE,
+ `Expected content value for ${PREF2_NAME}`);
+});
+
--- a/modules/libpref/test/unit_ipc/xpcshell.ini
+++ b/modules/libpref/test/unit_ipc/xpcshell.ini
@@ -4,9 +4,11 @@ skip-if = toolkit == 'android'
[test_existing_prefs.js]
[test_initial_prefs.js]
[test_large_pref.js]
[test_locked_prefs.js]
[test_observed_prefs.js]
[test_update_prefs.js]
[test_sharedMap.js]
+[test_sharedMap_var_caches.js]
+skip-if = !debug # Relies on debug assertions to catch failure cases.
[test_user_default_prefs.js]