Bug 1472523: Part 4 - Avoid unnecessary domain string duplication in preference observers. r?njn
MozReview-Commit-ID: EMCgMRTDqDn
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -78,16 +78,20 @@
#include "nsXPCOM.h"
#include "nsXULAppAPI.h"
#include "nsZipArchive.h"
#include "plbase64.h"
#include "PLDHashTable.h"
#include "plstr.h"
#include "prlink.h"
+#ifdef MOZ_MEMORY
+#include "mozmemory.h"
+#endif
+
#ifdef XP_WIN
#include "windows.h"
#endif
using namespace mozilla;
#ifdef DEBUG
@@ -1681,17 +1685,22 @@ private:
const nsAString& aValue);
nsresult CheckSanityOfStringLength(const char* aPrefName,
const nsACString& aValue);
nsresult CheckSanityOfStringLength(const char* aPrefName,
const uint32_t aLength);
void RemoveExpiredCallback(PrefCallback* aCallback);
- PrefName GetPrefName(const char* aPrefName) const;
+ PrefName GetPrefName(const char* aPrefName) const
+ {
+ return GetPrefName(nsDependentCString(aPrefName));
+ }
+
+ PrefName GetPrefName(const nsACString& aPrefName) const;
void FreeObserverList(void);
const nsCString mPrefRoot;
PrefValueKind mKind;
bool mFreeingObserverList;
nsClassHashtable<PrefCallback, PrefCallback> mObservers;
@@ -2375,23 +2384,22 @@ nsPrefBranch::GetChildList(const char* a
*aChildArray = outArray;
}
*aCount = numPrefs;
return NS_OK;
}
NS_IMETHODIMP
-nsPrefBranch::AddObserver(const char* aDomain,
- nsIObserver* aObserver,
- bool aHoldWeak)
+nsPrefBranch::AddObserverImpl(const nsACString& aDomain,
+ nsIObserver* aObserver,
+ bool aHoldWeak)
{
PrefCallback* pCallback;
- NS_ENSURE_ARG(aDomain);
NS_ENSURE_ARG(aObserver);
nsCString prefName;
GetPrefName(aDomain).get(prefName);
// Hold a weak reference to the observer if so requested.
if (aHoldWeak) {
nsCOMPtr<nsISupportsWeakReference> weakRefFactory =
@@ -2427,19 +2435,19 @@ nsPrefBranch::AddObserver(const char* aD
pCallback,
Preferences::PrefixMatch,
/* isPriority */ false);
return NS_OK;
}
NS_IMETHODIMP
-nsPrefBranch::RemoveObserver(const char* aDomain, nsIObserver* aObserver)
+nsPrefBranch::RemoveObserverImpl(const nsACString& aDomain,
+ nsIObserver* aObserver)
{
- NS_ENSURE_ARG(aDomain);
NS_ENSURE_ARG(aObserver);
nsresult rv = NS_OK;
// If we're in the middle of a call to FreeObserverList, don't process this
// RemoveObserver call -- the observer in question will be removed soon, if
// it hasn't been already.
//
@@ -2568,26 +2576,25 @@ nsPrefBranch::GetDefaultFromPropertiesFi
if (NS_FAILED(rv)) {
return rv;
}
return bundle->GetStringFromName(aPrefName, aReturn);
}
nsPrefBranch::PrefName
-nsPrefBranch::GetPrefName(const char* aPrefName) const
+nsPrefBranch::GetPrefName(const nsACString& aPrefName) const
{
MOZ_ASSERT(aPrefName);
- // For speed, avoid strcpy if we can.
if (mPrefRoot.IsEmpty()) {
- return PrefName(aPrefName);
- }
-
- return PrefName(mPrefRoot + nsDependentCString(aPrefName));
+ return PrefName(PromiseFlatCString(aPrefName));
+ }
+
+ return PrefName(mPrefRoot + aPrefName);
}
//----------------------------------------------------------------------------
// nsPrefLocalizedString
//----------------------------------------------------------------------------
nsPrefLocalizedString::nsPrefLocalizedString() = default;
@@ -4577,77 +4584,96 @@ Preferences::GetType(const char* aPrefNa
return PREF_BOOL;
default:
MOZ_CRASH();
}
}
/* static */ nsresult
-Preferences::AddStrongObserver(nsIObserver* aObserver, const char* aPref)
+Preferences::AddStrongObserver(nsIObserver* aObserver, const nsACString& aPref)
{
MOZ_ASSERT(aObserver);
NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE);
return sPreferences->mRootBranch->AddObserver(aPref, aObserver, false);
}
/* static */ nsresult
-Preferences::AddWeakObserver(nsIObserver* aObserver, const char* aPref)
+Preferences::AddWeakObserver(nsIObserver* aObserver, const nsACString& aPref)
{
MOZ_ASSERT(aObserver);
NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE);
return sPreferences->mRootBranch->AddObserver(aPref, aObserver, true);
}
/* static */ nsresult
-Preferences::RemoveObserver(nsIObserver* aObserver, const char* aPref)
+Preferences::RemoveObserver(nsIObserver* aObserver, const nsACString& aPref)
{
MOZ_ASSERT(aObserver);
if (sShutdown) {
MOZ_ASSERT(!sPreferences);
return NS_OK; // Observers have been released automatically.
}
NS_ENSURE_TRUE(sPreferences, NS_ERROR_NOT_AVAILABLE);
return sPreferences->mRootBranch->RemoveObserver(aPref, aObserver);
}
+template<typename T>
+static void
+AssertNotMallocAllocated(T* aPtr)
+{
+#if defined(DEBUG) && defined(MOZ_MEMORY)
+ jemalloc_ptr_info_t info;
+ jemalloc_ptr_info((void*)aPtr, &info);
+ MOZ_ASSERT(info.tag == TagUnknown);
+#endif
+}
+
/* static */ nsresult
Preferences::AddStrongObservers(nsIObserver* aObserver, const char** aPrefs)
{
MOZ_ASSERT(aObserver);
for (uint32_t i = 0; aPrefs[i]; i++) {
- nsresult rv = AddStrongObserver(aObserver, aPrefs[i]);
+ AssertNotMallocAllocated(aPrefs[i]);
+
+ nsCString pref;
+ pref.AssignLiteral(aPrefs[i], strlen(aPrefs[i]));
+ nsresult rv = AddStrongObserver(aObserver, pref);
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
/* static */ nsresult
Preferences::AddWeakObservers(nsIObserver* aObserver, const char** aPrefs)
{
MOZ_ASSERT(aObserver);
for (uint32_t i = 0; aPrefs[i]; i++) {
- nsresult rv = AddWeakObserver(aObserver, aPrefs[i]);
+ AssertNotMallocAllocated(aPrefs[i]);
+
+ nsCString pref;
+ pref.AssignLiteral(aPrefs[i], strlen(aPrefs[i]));
+ nsresult rv = AddWeakObserver(aObserver, pref);
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
/* static */ nsresult
Preferences::RemoveObservers(nsIObserver* aObserver, const char** aPrefs)
{
MOZ_ASSERT(aObserver);
if (sShutdown) {
MOZ_ASSERT(!sPreferences);
return NS_OK; // Observers have been released automatically.
}
NS_ENSURE_TRUE(sPreferences, NS_ERROR_NOT_AVAILABLE);
for (uint32_t i = 0; aPrefs[i]; i++) {
- nsresult rv = RemoveObserver(aObserver, aPrefs[i]);
+ nsresult rv = RemoveObserver(aObserver, nsDependentCString(aPrefs[i]));
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
/* static */ nsresult
Preferences::RegisterCallback(PrefChangedFunc aCallback,
const nsACString& aPrefNode,
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -232,22 +232,45 @@ public:
// Clears user set pref. Fails if run outside the parent process.
static nsresult ClearUser(const char* aPrefName);
// Whether the pref has a user value or not.
static bool HasUserValue(const char* aPref);
// Adds/Removes the observer for the root pref branch. See nsIPrefBranch.idl
// for details.
- static nsresult AddStrongObserver(nsIObserver* aObserver, const char* aPref);
- static nsresult AddWeakObserver(nsIObserver* aObserver, const char* aPref);
- static nsresult RemoveObserver(nsIObserver* aObserver, const char* aPref);
+ static nsresult AddStrongObserver(nsIObserver* aObserver,
+ const nsACString& aPref);
+ static nsresult AddWeakObserver(nsIObserver* aObserver,
+ const nsACString& aPref);
+ static nsresult RemoveObserver(nsIObserver* aObserver,
+ const nsACString& aPref);
+
+ template<int N>
+ static nsresult AddStrongObserver(nsIObserver* aObserver,
+ const char (&aPref)[N])
+ {
+ return AddStrongObserver(aObserver, nsLiteralCString(aPref));
+ }
+ template<int N>
+ static nsresult AddWeakObserver(nsIObserver* aObserver,
+ const char (&aPref)[N])
+ {
+ return AddWeakObserver(aObserver, nsLiteralCString(aPref));
+ }
+ template<int N>
+ static nsresult RemoveObserver(nsIObserver* aObserver, const char (&aPref)[N])
+ {
+ return RemoveObserver(aObserver, nsLiteralCString(aPref));
+ }
// Adds/Removes two or more observers for the root pref branch. Pass to
// aPrefs an array of const char* whose last item is nullptr.
+ // Note: All preference strings *must* be statically-allocated string
+ // literals.
static nsresult AddStrongObservers(nsIObserver* aObserver,
const char** aPrefs);
static nsresult AddWeakObservers(nsIObserver* aObserver, const char** aPrefs);
static nsresult RemoveObservers(nsIObserver* aObserver, const char** aPrefs);
// Registers/Unregisters the callback function for the aPref.
static nsresult RegisterCallback(PrefChangedFunc aCallback,
const nsACString& aPref,
--- a/modules/libpref/nsIPrefBranch.idl
+++ b/modules/libpref/nsIPrefBranch.idl
@@ -1,15 +1,19 @@
/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* 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/. */
#include "nsISupports.idl"
+%{C++
+#include "nsLiteralString.h"
+%}
+
interface nsIObserver;
/**
* The nsIPrefBranch interface is used to manipulate the preferences data. This
* object may be obtained from the preferences service (nsIPrefService) and
* used to get and set default and/or user preferences across the application.
*
* This object is created with a "root" value which describes the base point in
@@ -18,17 +22,17 @@ interface nsIObserver;
* For example, if this object is created with the root "browser.startup.",
* the preferences "browser.startup.page", "browser.startup.homepage",
* and "browser.startup.homepage_override" can be accessed by simply passing
* "page", "homepage", or "homepage_override" to the various Get/Set methods.
*
* @see nsIPrefService
*/
-[scriptable, uuid(55d25e49-793f-4727-a69f-de8b15f4b985)]
+[scriptable, builtinclass, uuid(55d25e49-793f-4727-a69f-de8b15f4b985)]
interface nsIPrefBranch : nsISupports
{
/**
* Values describing the basic preference types.
*
* @see getPrefType
*/
@@ -416,34 +420,62 @@ interface nsIPrefBranch : nsISupports
* @note
* It is not safe to change observers during this callback in Gecko
* releases before 1.9. If you want a safe way to remove a pref observer,
* please use an nsITimer.
*
* @see nsIObserver
* @see removeObserver
*/
- void addObserver(in string aDomain, in nsIObserver aObserver,
+ [binaryname(AddObserverImpl)]
+ void addObserver(in ACString aDomain, in nsIObserver aObserver,
[optional] in boolean aHoldWeak);
/**
* Remove a preference change observer.
*
* @param aDomain The preference which is being observed for changes.
* @param aObserver An observer previously registered with addObserver().
*
* @note
* Note that you must call removeObserver() on the same nsIPrefBranch
* instance on which you called addObserver() in order to remove aObserver;
* otherwise, the observer will not be removed.
*
* @see nsIObserver
* @see addObserver
*/
- void removeObserver(in string aDomain, in nsIObserver aObserver);
+ [binaryname(RemoveObserverImpl)]
+ void removeObserver(in ACString aDomain, in nsIObserver aObserver);
+
+ %{C++
+ nsresult AddObserver(const nsACString& aDomain, nsIObserver* aObserver,
+ bool aHoldWeak = false)
+ {
+ return AddObserverImpl(aDomain, aObserver, aHoldWeak);
+ }
+
+ template <int N>
+ nsresult AddObserver(const char (&aDomain)[N], nsIObserver* aObserver,
+ bool aHoldWeak = false)
+ {
+ return AddObserverImpl(nsLiteralCString(aDomain), aObserver, aHoldWeak);
+ }
+
+ nsresult RemoveObserver(const nsACString& aDomain, nsIObserver* aObserver)
+ {
+ return RemoveObserverImpl(aDomain, aObserver);
+ }
+
+ template <int N>
+ nsresult RemoveObserver(const char (&aDomain)[N], nsIObserver* aObserver)
+ {
+ return RemoveObserverImpl(nsLiteralCString(aDomain), aObserver);
+ }
+ %}
};
%{C++
#define NS_PREFBRANCH_CONTRACTID "@mozilla.org/preferencesbranch;1"
/**
* Notification sent when a preference changes.
--- a/netwerk/cache/nsCacheService.cpp
+++ b/netwerk/cache/nsCacheService.cpp
@@ -334,17 +334,19 @@ nsCacheProfilePrefObserver::Install()
rv2 = rv;
}
// install preferences observer
nsCOMPtr<nsIPrefBranch> branch = do_GetService(NS_PREFSERVICE_CONTRACTID);
if (!branch) return NS_ERROR_FAILURE;
for (auto& pref : prefList) {
- rv = branch->AddObserver(pref, this, false);
+ nsCString prefStr;
+ prefStr.AssignLiteral(pref, strlen(pref));
+ rv = branch->AddObserver(prefStr, this, false);
if (NS_FAILED(rv))
rv2 = rv;
}
// Determine if we have a profile already
// Install() is called *after* the profile-after-change notification
// when there is only a single profile, or it is specified on the
// commandline at startup.
@@ -377,17 +379,17 @@ nsCacheProfilePrefObserver::Remove()
}
// remove Pref Service observers
nsCOMPtr<nsIPrefBranch> prefs =
do_GetService(NS_PREFSERVICE_CONTRACTID);
if (!prefs)
return;
for (auto& pref : prefList)
- prefs->RemoveObserver(pref, this); // remove cache pref observers
+ prefs->RemoveObserver(nsDependentCString(pref), this); // remove cache pref observers
}
void
nsCacheProfilePrefObserver::SetDiskCacheCapacity(int32_t capacity)
{
mDiskCacheCapacity = std::max(0, capacity);
}
--- a/toolkit/components/extensions/WebExtensionPolicy.cpp
+++ b/toolkit/components/extensions/WebExtensionPolicy.cpp
@@ -277,19 +277,19 @@ namespace {
class AtomSetPref : public nsIObserver
, public nsSupportsWeakReference
{
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIOBSERVER
static already_AddRefed<AtomSetPref>
- Create(const char* aPref)
+ Create(const nsCString& aPref)
{
- RefPtr<AtomSetPref> self = new AtomSetPref(aPref);
+ RefPtr<AtomSetPref> self = new AtomSetPref(aPref.get());
Preferences::AddWeakObserver(self, aPref);
return self.forget();
}
const AtomSet& Get() const;
bool Contains(const nsAtom* aAtom) const
{
@@ -349,17 +349,17 @@ WebExtensionPolicy::IsRestrictedDoc(cons
return IsRestrictedURI(aDoc.PrincipalURL());
}
/* static */ bool
WebExtensionPolicy::IsRestrictedURI(const URLInfo &aURI)
{
static RefPtr<AtomSetPref> domains;
if (!domains) {
- domains = AtomSetPref::Create(kRestrictedDomainPref);
+ domains = AtomSetPref::Create(nsLiteralCString(kRestrictedDomainPref));
ClearOnShutdown(&domains);
}
if (domains->Contains(aURI.HostAtom())) {
return true;
}
if (AddonManagerWebAPI::IsValidSite(aURI.URI())) {
--- a/toolkit/components/reputationservice/LoginReputation.cpp
+++ b/toolkit/components/reputationservice/LoginReputation.cpp
@@ -25,16 +25,17 @@ static bool sPasswordProtectionEnabled =
LazyLogModule gLoginReputationLogModule("LoginReputation");
#define LR_LOG(args) MOZ_LOG(gLoginReputationLogModule, mozilla::LogLevel::Debug, args)
#define LR_LOG_ENABLED() MOZ_LOG_TEST(gLoginReputationLogModule, mozilla::LogLevel::Debug)
static Atomic<bool> gShuttingDown(false);
static const char* kObservedPrefs[] = {
PREF_PASSWORD_ALLOW_TABLE,
+ nullptr,
};
// -------------------------------------------------------------------------
// ReputationQueryParam
//
// Concrete class for nsILoginReputationQuery to hold query parameters
//
class ReputationQueryParam final : public nsILoginReputationQuery
@@ -268,19 +269,17 @@ LoginReputationService::Enable()
MOZ_ASSERT(XRE_IsParentProcess());
MOZ_ASSERT(sPasswordProtectionEnabled);
LR_LOG(("Enable login reputation service"));
nsresult rv = mLoginWhitelist->Init();
Unused << NS_WARN_IF(NS_FAILED(rv));
- for (const char* pref : kObservedPrefs) {
- Preferences::AddStrongObserver(this, pref);
- }
+ Preferences::AddStrongObservers(this, kObservedPrefs);
return NS_OK;
}
nsresult
LoginReputationService::Disable()
{
MOZ_ASSERT(XRE_IsParentProcess());
@@ -289,19 +288,17 @@ LoginReputationService::Disable()
nsresult rv = mLoginWhitelist->Uninit();
Unused << NS_WARN_IF(NS_FAILED(rv));
mQueryRequests.Clear();
nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
if (prefs) {
- for (const char* pref : kObservedPrefs) {
- prefs->RemoveObserver(pref, this);
- }
+ Preferences::RemoveObservers(this, kObservedPrefs);
}
return NS_OK;
}
nsresult
LoginReputationService::Shutdown()
{
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -1668,17 +1668,17 @@ nsUrlClassifierDBService::Init()
// XXX: Do we *really* need to be able to change all of these at runtime?
// Note: These observers should only be added when everything else above has
// succeeded. Failing to do so can cause long shutdown times in certain
// situations. See Bug 1247798 and Bug 1244803.
Preferences::AddUintVarCache(&sGethashNoise, GETHASH_NOISE_PREF,
GETHASH_NOISE_DEFAULT);
for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
- Preferences::AddStrongObserver(this, kObservedPrefs[i].get());
+ Preferences::AddStrongObserver(this, kObservedPrefs[i]);
}
return NS_OK;
}
// nsChannelClassifier is the only consumer of this interface.
NS_IMETHODIMP
nsUrlClassifierDBService::Classify(nsIPrincipal* aPrincipal,
@@ -2491,17 +2491,17 @@ nsUrlClassifierDBService::Shutdown()
Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_SHUTDOWN_TIME> timer;
mCompleters.Clear();
nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
if (prefs) {
for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
- prefs->RemoveObserver(kObservedPrefs[i].get(), this);
+ prefs->RemoveObserver(kObservedPrefs[i], this);
}
}
// 1. Synchronize with worker thread and update thread by
// *synchronously* dispatching an event to worker thread
// for shutting down the update thread. The reason not
// shutting down update thread directly from main thread
// is to avoid racing for Classifier::mUpdateThread
--- a/widget/android/PrefsHelper.h
+++ b/widget/android/PrefsHelper.h
@@ -259,32 +259,32 @@ public:
nsTArray<jni::Object::LocalRef> nameRefArray(
aPrefsToObserve->GetElements());
nsAppShell* const appShell = nsAppShell::Get();
MOZ_ASSERT(appShell);
for (jni::Object::LocalRef& nameRef : nameRefArray) {
jni::String::LocalRef nameStr(std::move(nameRef));
MOZ_ALWAYS_SUCCEEDS(Preferences::AddStrongObserver(
- appShell, nameStr->ToCString().get()));
+ appShell, nameStr->ToCString()));
}
}
static void RemoveObserver(const jni::Class::LocalRef& aCls,
jni::ObjectArray::Param aPrefsToUnobserve)
{
nsTArray<jni::Object::LocalRef> nameRefArray(
aPrefsToUnobserve->GetElements());
nsAppShell* const appShell = nsAppShell::Get();
MOZ_ASSERT(appShell);
for (jni::Object::LocalRef& nameRef : nameRefArray) {
jni::String::LocalRef nameStr(std::move(nameRef));
MOZ_ALWAYS_SUCCEEDS(Preferences::RemoveObserver(
- appShell, nameStr->ToCString().get()));
+ appShell, nameStr->ToCString()));
}
}
static void OnPrefChange(const char16_t* aData)
{
const nsCString& name = NS_LossyConvertUTF16toASCII(aData);
int32_t type = -1;