From bdb3eac1dd46f3d0b63fb559a5b3812a7e483691 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Sun, 1 Jul 2018 23:23:48 -0700 Subject: [PATCH] Bug 1471025: Part 4 - Add a wrapper class that can access either static or dynamic prefs. r=njn The in-memory format of shared-memory preferences is significantly different from the format used by dynamic preferences, which means that we need different classes to access their properties. Virtual classes would be a potential solution to this problem, but I don't think the performance characteristics would be acceptable for preferences code. And since the wrapper classes used for static prefs are temporary, they would add the additional snag of figuring out how to keep a valid pointer alive. So, instead, this patch adds a wrapper class that can access either type of preference, based on known type flags in a Variant. It also moves some of the logic for deciding which preference value to return to the wrapper, so that it doesn't need to be duplicated for each representation. MozReview-Commit-ID: LameIIbYcD3 --HG-- extra : source : 83d98ea5ebaccded8a20929c0f3316e5618f1f76 extra : histedit_source : b3fc33ea04357e15323c7bcb1d02e73c1a9b0c76 --- modules/libpref/Preferences.cpp | 236 +++++++++++++++++++++++--------- 1 file changed, 171 insertions(+), 65 deletions(-) diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index c339bac86aa1..82705c40e5e8 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -462,7 +462,8 @@ public: mUserValue.Clear(Type()); } - const char* Name() { return mName; } + const char* Name() const { return mName; } + nsDependentCString NameString() const { return nsDependentCString(mName); } // Types. @@ -484,17 +485,18 @@ public: mHasChangedSinceInit = true; } + bool IsSticky() const { return mIsSticky; } + bool HasDefaultValue() const { return mHasDefaultValue; } bool HasUserValue() const { return mHasUserValue; } template void AddToMap(SharedPrefMapBuilder& aMap) { - aMap.Add( - Name(), - { HasDefaultValue(), HasUserValue(), uint8_t(mIsSticky), IsLocked() }, - HasDefaultValue() ? mDefaultValue.Get() : T(), - HasUserValue() ? mUserValue.Get() : T()); + aMap.Add(Name(), + { HasDefaultValue(), HasUserValue(), IsSticky(), IsLocked() }, + HasDefaultValue() ? mDefaultValue.Get() : T(), + HasUserValue() ? mUserValue.Get() : T()); } void AddToMap(SharedPrefMapBuilder& aMap) @@ -547,63 +549,41 @@ public: return strcmp(mName, aPrefName) == 0; } - nsresult GetBoolValue(PrefValueKind aKind, bool* aResult) + bool GetBoolValue(PrefValueKind aKind = PrefValueKind::User) const { - if (!IsTypeBool()) { - return NS_ERROR_UNEXPECTED; - } + MOZ_ASSERT(IsTypeBool()); + MOZ_ASSERT(aKind == PrefValueKind::Default ? HasDefaultValue() + : HasUserValue()); - if (aKind == PrefValueKind::Default || IsLocked() || !mHasUserValue) { - // Do we have a default? - if (!mHasDefaultValue) { - return NS_ERROR_UNEXPECTED; - } - *aResult = mDefaultValue.mBoolVal; - } else { - *aResult = mUserValue.mBoolVal; - } - - return NS_OK; + return aKind == PrefValueKind::Default ? mDefaultValue.mBoolVal + : mUserValue.mBoolVal; } - nsresult GetIntValue(PrefValueKind aKind, int32_t* aResult) + int32_t GetIntValue(PrefValueKind aKind = PrefValueKind::User) const { - if (!IsTypeInt()) { - return NS_ERROR_UNEXPECTED; - } + MOZ_ASSERT(IsTypeInt()); + MOZ_ASSERT(aKind == PrefValueKind::Default ? HasDefaultValue() + : HasUserValue()); - if (aKind == PrefValueKind::Default || IsLocked() || !mHasUserValue) { - // Do we have a default? - if (!mHasDefaultValue) { - return NS_ERROR_UNEXPECTED; - } - *aResult = mDefaultValue.mIntVal; - } else { - *aResult = mUserValue.mIntVal; - } - - return NS_OK; + return aKind == PrefValueKind::Default ? mDefaultValue.mIntVal + : mUserValue.mIntVal; } - nsresult GetCStringValue(PrefValueKind aKind, nsACString& aResult) + const char* GetBareStringValue( + PrefValueKind aKind = PrefValueKind::User) const { - if (!IsTypeString()) { - return NS_ERROR_UNEXPECTED; - } + MOZ_ASSERT(IsTypeString()); + MOZ_ASSERT(aKind == PrefValueKind::Default ? HasDefaultValue() + : HasUserValue()); - if (aKind == PrefValueKind::Default || IsLocked() || !mHasUserValue) { - // Do we have a default? - if (!mHasDefaultValue) { - return NS_ERROR_UNEXPECTED; - } - MOZ_ASSERT(mDefaultValue.mStringVal); - aResult = mDefaultValue.mStringVal; - } else { - MOZ_ASSERT(mUserValue.mStringVal); - aResult = mUserValue.mStringVal; - } + return aKind == PrefValueKind::Default ? mDefaultValue.mStringVal + : mUserValue.mStringVal; + } - return NS_OK; + nsDependentCString GetStringValue( + PrefValueKind aKind = PrefValueKind::User) const + { + return nsDependentCString(GetBareStringValue(aKind)); } void ToDomPref(dom::Pref* aDomPref) @@ -1036,6 +1016,113 @@ public: } }; +using PrefWrapperBase = Variant; +class MOZ_STACK_CLASS PrefWrapper : public PrefWrapperBase +{ + using SharedPref = const SharedPrefMap::Pref; + +public: + MOZ_IMPLICIT PrefWrapper(Pref* aPref) + : PrefWrapperBase(AsVariant(aPref)) + { + } + + MOZ_IMPLICIT PrefWrapper(const SharedPrefMap::Pref& aPref) + : PrefWrapperBase(AsVariant(aPref)) + { + } + + // Types. + + bool IsType(PrefType aType) const { return Type() == aType; } + bool IsTypeNone() const { return IsType(PrefType::None); } + bool IsTypeString() const { return IsType(PrefType::String); } + bool IsTypeInt() const { return IsType(PrefType::Int); } + bool IsTypeBool() const { return IsType(PrefType::Bool); } + +#define FORWARD(retType, method) \ + retType method() const \ + { \ + struct Matcher \ + { \ + retType match(const Pref* aPref) { return aPref->method(); } \ + retType match(SharedPref& aPref) { return aPref.method(); } \ + }; \ + return match(Matcher()); \ + } + + FORWARD(bool, IsLocked) + FORWARD(bool, IsSticky) + FORWARD(bool, HasDefaultValue) + FORWARD(bool, HasUserValue) + FORWARD(const char*, Name) + FORWARD(nsCString, NameString) + FORWARD(PrefType, Type) +#undef FORWARD + +#define FORWARD(retType, method) \ + retType method(PrefValueKind aKind = PrefValueKind::User) const \ + { \ + struct Matcher \ + { \ + PrefValueKind mKind; \ + \ + retType match(const Pref* aPref) { return aPref->method(mKind); } \ + retType match(SharedPref& aPref) { return aPref.method(mKind); } \ + }; \ + return match(Matcher{ aKind }); \ + } + + FORWARD(bool, GetBoolValue) + FORWARD(int32_t, GetIntValue) + FORWARD(nsCString, GetStringValue) + FORWARD(const char*, GetBareStringValue) +#undef FORWARD + + Result WantValueKind(PrefType aType, + PrefValueKind aKind) const + { + if (Type() != aType) { + return Err(NS_ERROR_UNEXPECTED); + } + + if (aKind == PrefValueKind::Default || IsLocked() || !HasUserValue()) { + if (!HasDefaultValue()) { + return Err(NS_ERROR_UNEXPECTED); + } + return PrefValueKind::Default; + } + return PrefValueKind::User; + } + + nsresult GetBoolValue(PrefValueKind aKind, bool* aResult) const + { + PrefValueKind kind; + MOZ_TRY_VAR(kind, WantValueKind(PrefType::Bool, aKind)); + + *aResult = GetBoolValue(kind); + return NS_OK; + } + + nsresult GetIntValue(PrefValueKind aKind, int32_t* aResult) const + { + PrefValueKind kind; + MOZ_TRY_VAR(kind, WantValueKind(PrefType::Int, aKind)); + + *aResult = GetIntValue(kind); + return NS_OK; + } + + nsresult GetCStringValue(PrefValueKind aKind, nsACString& aResult) const + { + PrefValueKind kind; + MOZ_TRY_VAR(kind, WantValueKind(PrefType::String, aKind)); + + aResult = GetStringValue(kind); + return NS_OK; + } +}; + class CallbackNode { public: @@ -1221,9 +1308,21 @@ pref_HashTableLookup(const char* aPrefName) return nullptr; } + return entry->mPref; +} + +Maybe +pref_Lookup(const char* aPrefName) +{ + Maybe result; + AddAccessCount(aPrefName); - return entry->mPref; + if (Pref* pref = pref_HashTableLookup(aPrefName)) { + result.emplace(pref); + } + + return result; } static nsresult @@ -4441,8 +4540,9 @@ Preferences::GetBool(const char* aPrefName, bool* aResult, PrefValueKind aKind) MOZ_ASSERT(aResult); NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE); - Pref* pref = pref_HashTableLookup(aPrefName); - return pref ? pref->GetBoolValue(aKind, aResult) : NS_ERROR_UNEXPECTED; + Maybe pref = pref_Lookup(aPrefName); + return pref.isSome() ? pref->GetBoolValue(aKind, aResult) + : NS_ERROR_UNEXPECTED; } /* static */ nsresult @@ -4453,8 +4553,9 @@ Preferences::GetInt(const char* aPrefName, MOZ_ASSERT(aResult); NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE); - Pref* pref = pref_HashTableLookup(aPrefName); - return pref ? pref->GetIntValue(aKind, aResult) : NS_ERROR_UNEXPECTED; + Maybe pref = pref_Lookup(aPrefName); + return pref.isSome() ? pref->GetIntValue(aKind, aResult) + : NS_ERROR_UNEXPECTED; } /* static */ nsresult @@ -4481,8 +4582,9 @@ Preferences::GetCString(const char* aPrefName, aResult.SetIsVoid(true); - Pref* pref = pref_HashTableLookup(aPrefName); - return pref ? pref->GetCStringValue(aKind, aResult) : NS_ERROR_UNEXPECTED; + Maybe pref = pref_Lookup(aPrefName); + return pref.isSome() ? pref->GetCStringValue(aKind, aResult) + : NS_ERROR_UNEXPECTED; } /* static */ nsresult @@ -4652,8 +4754,8 @@ Preferences::IsLocked(const char* aPrefName) { NS_ENSURE_TRUE(InitStaticMembers(), false); - Pref* pref = pref_HashTableLookup(aPrefName); - return pref && pref->IsLocked(); + Maybe pref = pref_Lookup(aPrefName); + return pref.isSome() && pref->IsLocked(); } /* static */ nsresult @@ -4682,8 +4784,8 @@ Preferences::HasUserValue(const char* aPrefName) { NS_ENSURE_TRUE(InitStaticMembers(), false); - Pref* pref = pref_HashTableLookup(aPrefName); - return pref && pref->HasUserValue(); + Maybe pref = pref_Lookup(aPrefName); + return pref.isSome() && pref->HasUserValue(); } /* static */ int32_t @@ -4691,8 +4793,12 @@ Preferences::GetType(const char* aPrefName) { NS_ENSURE_TRUE(InitStaticMembers(), nsIPrefBranch::PREF_INVALID); - Pref* pref; - if (!gHashTable || !(pref = pref_HashTableLookup(aPrefName))) { + if (!gHashTable) { + return PREF_INVALID; + } + + Maybe pref = pref_Lookup(aPrefName); + if (!pref.isSome()) { return PREF_INVALID; }