Bug 1413811 (part 1) - Avoid PrefCallback for pref callbacks registered with Preferences::RegisterCallback(). r=glandium.
Pref callbacks registered via Preferences::Add*VarCache() are wrapped in a
ValueObserver -- which groups all callback requests that have the same prefname
and callback function -- and the ValueObserver is put into gObserverTable. This
is reasonable.
Observers registered via nsPrefBranch::Add{Weak,Strong}Observer() are wrapped
in a PrefCallback, and the PrefCallback is put into sRootBranch->mObservers.
This is also reasonable.
Pref callbacks registered via Preferences::RegisterCallback() are conceptually
similar to those registered via Preferences::Add*VarCache(). However, they are
implemented by using *both* of the above mechanisms: they are wrapped in a
ValueObserver which is put into gObserverTable, *and* that ValueObserver is then
wrapped in a PrefCallback which is put into sRootBranch->mObservers.
Using both mechanisms isn't necessary, so this patch removes the
PrefCallback/mObservers part. This makes Preferences::RegisterCallback() work
in much the same way as Preferences::Add*VarCache().
Specifically:
- Preferences::RegisterCallback() now calls PREF_RegisterCallback() instead of
Preferences::AddStrongObserver(). This makes it more similar to
RegisterPriorityCallback().
- Preferences::UnregisterCallback() now explicitly calls
PREF_UnregisterCallback() instead of Preferences::RemoveObserver (which
previously happened via ~ValueObserver() when the ValueObserver was removed
from gObserverTable and its refcount dropped to zerod).
MozReview-Commit-ID: 1tEQNeYrBUU
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -3271,17 +3271,17 @@ public:
PrefChangedFunc mCallback;
Preferences::MatchKind mMatchKind;
};
class ValueObserver final
: public nsIObserver
, public ValueObserverHashKey
{
- ~ValueObserver() { Preferences::RemoveObserver(this, mPrefName.get()); }
+ ~ValueObserver() = default;
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIOBSERVER
ValueObserver(const char* aPref,
PrefChangedFunc aCallback,
Preferences::MatchKind aMatchKind)
@@ -5064,19 +5064,20 @@ Preferences::RegisterCallback(PrefChange
gObserverTable->Get(&hashKey, getter_AddRefs(observer));
if (observer) {
observer->AppendClosure(aClosure);
return NS_OK;
}
observer = new ValueObserver(aPref, aCallback, aMatchKind);
observer->AppendClosure(aClosure);
- nsresult rv = AddStrongObserver(observer, aPref);
- NS_ENSURE_SUCCESS(rv, rv);
-
+ PREF_RegisterCallback(aPref,
+ NotifyObserver,
+ static_cast<nsIObserver*>(observer),
+ /* isPriority */ false);
gObserverTable->Put(observer, observer);
return NS_OK;
}
/* static */ nsresult
Preferences::RegisterCallbackAndCall(PrefChangedFunc aCallback,
const char* aPref,
void* aClosure,
@@ -5108,16 +5109,19 @@ Preferences::UnregisterCallback(PrefChan
gObserverTable->Get(&hashKey, getter_AddRefs(observer));
if (!observer) {
return NS_OK;
}
observer->RemoveClosure(aClosure);
if (observer->HasNoClosures()) {
// Delete the callback since its list of closures is empty.
+ MOZ_ALWAYS_SUCCEEDS(
+ PREF_UnregisterCallback(aPref, NotifyObserver, observer));
+
gObserverTable->Remove(observer);
}
return NS_OK;
}
// We insert cache observers using RegisterPriorityCallback to ensure they are
// called prior to ordinary pref observers. Doing this ensures that ordinary
// observers will never get stale values from cache variables.