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

@@ -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;
 

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);

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();

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<Method>::Type returns T.
+template<typename T>
+struct InstanceType;
+
+template<typename T, typename U>
+struct InstanceType<U T::*>
+{
+  using Type = T;
+};
+
+// A wrapper for a PrefChangeMethod instance method which forwards calls to the
+// wrapped method on the given instance.
+template<typename T, PrefChangedMethod T::*Method>
+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 <auto Method> argument, and use
+// PrefChangeMethod<&meth> directly.
+#define PREF_CHANGE_METHOD(meth)                                               \
+  (&::mozilla::detail::PrefChangeMethod<                                       \
+    ::mozilla::detail::InstanceType<decltype(&meth)>::Type,                    \
+    &meth>)
+
 class PreferenceServiceReporter;
 
 namespace dom {