From 7a82fe3b08cc52f95e4a5b11128bad5fa92b01b6 Mon Sep 17 00:00:00 2001
From: Emily McDonough <emcdonough@mozilla.com>
Date: Tue, 11 Aug 2020 00:25:18 +0000
Subject: [PATCH] Bug 1658457 - Use constexpr static pointers for nsCUPSShim on
 OS X r=emilio

This uses an additional define to control this, as ideally we would have a
configure option to link directly to CUPS which would be set by default on
OS X.

This single define also makes it easier to test either configuration on OS X
and on other Unix.

Differential Revision: https://phabricator.services.mozilla.com/D86642
---
 widget/nsCUPSShim.cpp | 40 +++++++++++++++++++++++++---------------
 widget/nsCUPSShim.h   | 17 ++++++++++++++++-
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/widget/nsCUPSShim.cpp b/widget/nsCUPSShim.cpp
index 703ac61c025e..a45799ab3491 100644
--- a/widget/nsCUPSShim.cpp
+++ b/widget/nsCUPSShim.cpp
@@ -10,33 +10,34 @@
 #include "mozilla/ArrayUtils.h"
 #include "prlink.h"
 
-#ifdef XP_MACOSX
-// TODO: On OS X we are guaranteed to have CUPS, so it would be nice to just
-// assign the members from the header directly instead of dlopen'ing.
-// Alternatively, we could just do some #define's on OS X, but we don't use
-// CUPS all that much, so there really isn't too much overhead in storing this
-// table of functions even on OS X.
+#ifdef CUPS_SHIM_RUNTIME_LINK
+
+// TODO: This is currently pointless as we always use the compile-time linked
+// version of CUPS, but in the future this may become a configure option.
+// We also cannot use NSPR's library suffix support, since that cannot handle
+// version number suffixes.
+#  ifdef XP_MACOSX
 static const char gCUPSLibraryName[] = "libcups.2.dylib";
-#else
+#  else
 static const char gCUPSLibraryName[] = "libcups.so.2";
-#endif
+#  endif
 
 template <typename FuncT>
 static bool LoadCupsFunc(PRLibrary*& lib, FuncT*& dest,
                          const char* const name) {
   dest = (FuncT*)PR_FindSymbol(lib, name);
   if (MOZ_UNLIKELY(!dest)) {
-#ifdef DEBUG
+#  ifdef DEBUG
     nsAutoCString msg(name);
     msg.AppendLiteral(" not found in CUPS library");
     NS_WARNING(msg.get());
-#endif
-#ifndef MOZ_TSAN
+#  endif
+#  ifndef MOZ_TSAN
     // With TSan, we cannot unload libcups once we have loaded it because
     // TSan does not support unloading libraries that are matched from its
     // suppression list. Hence we just keep the library loaded in TSan builds.
     PR_UnloadLibrary(lib);
-#endif
+#  endif
     lib = nullptr;
     return false;
   }
@@ -56,10 +57,19 @@ bool nsCUPSShim::Init() {
 
 // This is a macro so that it could also load from libcups if we are configured
 // to use it as a compile-time dependency.
-#define CUPS_SHIM_LOAD(NAME) \
-  if (!LoadCupsFunc(mCupsLib, NAME, #NAME)) return false;
+#  define CUPS_SHIM_LOAD(NAME) \
+    if (!LoadCupsFunc(mCupsLib, NAME, #NAME)) return false;
   CUPS_SHIM_ALL_FUNCS(CUPS_SHIM_LOAD)
-#undef CUPS_SHIM_LOAD
+#  undef CUPS_SHIM_LOAD
   mInited = true;
   return true;
 }
+
+#else  // CUPS_SHIM_RUNTIME_LINK
+
+bool nsCUPSShim::Init() {
+  mInited = true;
+  return true;
+}
+
+#endif
diff --git a/widget/nsCUPSShim.h b/widget/nsCUPSShim.h
index a53d22526735..657f0480a192 100644
--- a/widget/nsCUPSShim.h
+++ b/widget/nsCUPSShim.h
@@ -11,6 +11,11 @@
 #include "mozilla/Atomics.h"
 #include "mozilla/Mutex.h"
 
+// TODO: This should be a configure option, ideally.
+#ifndef XP_MACOSX
+#  define CUPS_SHIM_RUNTIME_LINK
+#endif
+
 struct PRLibrary;
 
 /* Note: this class relies on static initialization. */
@@ -43,7 +48,15 @@ class nsCUPSShim {
   X(cupsTempFd)                \
   X(httpClose)
 
-#define CUPS_SHIM_FUNC_DECL(X) decltype(::X)* X;
+#ifdef CUPS_SHIM_RUNTIME_LINK
+  // Define a single field which holds a function pointer.
+#  define CUPS_SHIM_FUNC_DECL(X) decltype(::X)* X;
+#else
+  // Define a static constexpr function pointer. GCC can sometimes optimize
+  // away the pointer fetch for this.
+#  define CUPS_SHIM_FUNC_DECL(X) static constexpr decltype(::X)* const X = ::X;
+#endif
+
   CUPS_SHIM_ALL_FUNCS(CUPS_SHIM_FUNC_DECL)
 #undef CUPS_SHIM_FUNC_DECL
 
@@ -63,8 +76,10 @@ class nsCUPSShim {
   //
   // The boolean can't be Relaxed, because it guards our function pointers.
   mozilla::Atomic<bool, mozilla::ReleaseAcquire> mInited {false};
+#ifdef CUPS_SHIM_RUNTIME_LINK
   mozilla::OffTheBooksMutex mInitMutex {"nsCUPSShim::mInitMutex"};
   PRLibrary* mCupsLib;
+#endif
 };
 
 #endif /* nsCUPSShim_h___ */