From 8d585408dc4fd18e18be66ea2be9bd7e9e8f092c Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Fri, 13 Jul 2018 18:54:11 -0700 Subject: [PATCH] Bug 1473631: Part 0b - Add helper for registering instance methods as pref callbacks. r=njn I also tried to avoid this change but, again, given the number of times I was repeating the same pattern of defining a static method just to forward a callback to an instance method, decided it was probably necessary. Without an easy way to do this, people are more likely to register observers rather than callbacks, for which we'll wind up paying a continued memory and performance penalty. This patch adds a helper which creates a type-safe preference callback function which forwards calls to an instance method on its closure object. The implementation is somewhat complicated, mainly due to the constraint that unregistering a callback requires passing the exact same function pointer that was used to register it. The patch achieves this by creating the callback function as a template, with the method pointer as a template parameter. As long as the Register and Unregister calls happen in the same translation unit, the same template instance is guaranteed to be used for both. The main difficulty is that, until C++ 17, there's no way match a value as a template parameter unless you know its complete type, or can at least compute its complete type based on earlier template parameters. That means that we need a macro to extract the type of the method, and construct the template with the full set of explicit parameters. MozReview-Commit-ID: 10N3R2SRtPc --HG-- extra : rebase_source : 7d0a8ddeb77e01d4a6f421459514e93bc0875598 --- dom/base/nsDocument.cpp | 15 +++++------ layout/base/nsPresContext.cpp | 27 +++++++++----------- layout/base/nsPresContext.h | 1 - modules/libpref/Preferences.h | 48 +++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 3c918f72fe5b..1a3d3e0a0ae9 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -12592,17 +12592,21 @@ struct PrefStore Preferences::AddBoolVarCache(&mPluginsHttpOnly, "plugins.http_https_only"); - Preferences::RegisterCallbacks(UpdateStringPrefs, gCallbackPrefs, this); + Preferences::RegisterCallbacks( + PREF_CHANGE_METHOD(PrefStore::UpdateStringPrefs), + gCallbackPrefs, this); UpdateStringPrefs(); } ~PrefStore() { - Preferences::UnregisterCallbacks(UpdateStringPrefs, gCallbackPrefs, this); + Preferences::UnregisterCallbacks( + PREF_CHANGE_METHOD(PrefStore::UpdateStringPrefs), + gCallbackPrefs, this); } - void UpdateStringPrefs() + void UpdateStringPrefs(const char* aPref = nullptr) { Preferences::GetCString("urlclassifier.flashAllowTable", mAllowTables); Preferences::GetCString("urlclassifier.flashAllowExceptTable", mAllowExceptionsTables); @@ -12612,11 +12616,6 @@ struct PrefStore Preferences::GetCString("urlclassifier.flashSubDocExceptTable", mSubDocDenyExceptionsTables); } - static void UpdateStringPrefs(const char*, PrefStore* aSelf) - { - aSelf->UpdateStringPrefs(); - } - bool mFlashBlockEnabled; bool mPluginsHttpOnly; diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp index 32eaafeefcae..c593f43cd75c 100644 --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -149,13 +149,6 @@ nsPresContext::IsDOMPaintEventPending() return false; } -void -nsPresContext::PrefChangedCallback(const char* aPrefName, nsPresContext* aPresContext) -{ - NS_ASSERTION(aPresContext, "bad instance data"); - aPresContext->PreferenceChanged(aPrefName); -} - void nsPresContext::ForceReflowForFontInfoUpdate() { @@ -321,10 +314,12 @@ nsPresContext::Destroy() } // Unregister preference callbacks - Preferences::UnregisterPrefixCallbacks(nsPresContext::PrefChangedCallback, - gPrefixCallbackPrefs, this); - Preferences::UnregisterCallbacks(nsPresContext::PrefChangedCallback, - gExactCallbackPrefs, this); + Preferences::UnregisterPrefixCallbacks( + PREF_CHANGE_METHOD(nsPresContext::PreferenceChanged), + gPrefixCallbackPrefs, this); + Preferences::UnregisterCallbacks( + PREF_CHANGE_METHOD(nsPresContext::PreferenceChanged), + gExactCallbackPrefs, this); mRefreshDriver = nullptr; } @@ -858,10 +853,12 @@ nsPresContext::Init(nsDeviceContext* aDeviceContext) } // Register callbacks so we're notified when the preferences change - Preferences::RegisterPrefixCallbacks(nsPresContext::PrefChangedCallback, - gPrefixCallbackPrefs, this); - Preferences::RegisterCallbacks(nsPresContext::PrefChangedCallback, - gExactCallbackPrefs, this); + Preferences::RegisterPrefixCallbacks( + PREF_CHANGE_METHOD(nsPresContext::PreferenceChanged), + gPrefixCallbackPrefs, this); + Preferences::RegisterCallbacks( + PREF_CHANGE_METHOD(nsPresContext::PreferenceChanged), + gExactCallbackPrefs, this); nsresult rv = mEventManager->Init(); NS_ENSURE_SUCCESS(rv, rv); diff --git a/layout/base/nsPresContext.h b/layout/base/nsPresContext.h index ad3e66809169..9a88f4fdddab 100644 --- a/layout/base/nsPresContext.h +++ b/layout/base/nsPresContext.h @@ -1198,7 +1198,6 @@ protected: void GetDocumentColorPreferences(); void PreferenceChanged(const char* aPrefName); - static void PrefChangedCallback(const char*, nsPresContext*); void UpdateAfterPreferencesChanged(); void DispatchPrefChangedRunnableIfNeeded(); diff --git a/modules/libpref/Preferences.h b/modules/libpref/Preferences.h index 2326158a2094..1e31b86c5363 100644 --- a/modules/libpref/Preferences.h +++ b/modules/libpref/Preferences.h @@ -67,6 +67,54 @@ struct TypedPrefChangeFunc CallbackType mCallback; }; +// Similar to PrefChangedFunc, but for use with instance methods. +// +// Any instance method with this signature may be passed to the +// PREF_CHANGE_METHOD macro, which will wrap it into a typesafe preference +// callback function, which accepts a preference name as its first argument, and +// an instance of the appropriate class as the second. +// +// When called, the wrapper will forward the call to the wrapped method on the +// given instance, with the notified preference as its only argument. +typedef void(PrefChangedMethod)(const char* aPref); + +namespace detail { +// Helper to extract the instance type from any instance method. For an instance +// method `Method = U T::*`, InstanceType::Type returns T. +template +struct InstanceType; + +template +struct InstanceType +{ + using Type = T; +}; + +// A wrapper for a PrefChangeMethod instance method which forwards calls to the +// wrapped method on the given instance. +template +void +PrefChangeMethod(const char* aPref, T* aInst) +{ + ((*aInst).*Method)(aPref); +} +} // namespace detail + +// Creates a wrapper around an instance method, with the signature of +// PrefChangedMethod, from an arbitrary class, so that it can be used as a +// preference callback. The closure data passed to RegisterCallback must be an +// instance of this class. +// +// Note: This is implemented as a macro rather than a pure template function +// because, prior to C++17, value template arguments must have their types +// fully-specified. Once all of our supported compilers have C++17 support, we +// can give PrefChangeMethod a single argument, and use +// PrefChangeMethod<&meth> directly. +#define PREF_CHANGE_METHOD(meth) \ + (&::mozilla::detail::PrefChangeMethod< \ + ::mozilla::detail::InstanceType::Type, \ + &meth>) + class PreferenceServiceReporter; namespace dom {