Bug 1409249: Require singleton constructors to return explicit already_AddRefed. r=froydnj

Right now, NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR expects singleton
constructors to return already-addrefed raw pointers, and while it accepts
constructors that return already_AddRefed, most existing don't do so.

Meanwhile, the convention elsewhere is that a raw pointer return value is
owned by the callee, and that the caller needs to addref it if it wants to
keep its own reference to it.

The difference in convention makes it easy to leak (I've definitely caused
more than one shutdown leak this way), so it would be better if we required
the singleton getters to return an explicit already_AddRefed, which would
behave the same for all callers.


This also cleans up several singleton constructors that left a dangling
pointer to their singletons when their initialization methods failed, when
they released their references without clearing their global raw pointers.

MozReview-Commit-ID: 9peyG4pRYcr

--HG--
extra : rebase_source : 2f5bd89c17cb554541be38444672a827c1392f3f

caps/nsScriptSecurityManager.cpp

@@ -1437,13 +1437,12 @@ nsScriptSecurityManager::InitStatics()
 // Currently this nsGenericFactory constructor is used only from FastLoad
 // (XPCOM object deserialization) code, when "creating" the system principal
 // singleton.
-SystemPrincipal *
+already_AddRefed<SystemPrincipal>
 nsScriptSecurityManager::SystemPrincipalSingletonConstructor()
 {
-    nsIPrincipal *sysprin = nullptr;
     if (gScriptSecMan)
-        NS_ADDREF(sysprin = gScriptSecMan->mSystemPrincipal);
-    return static_cast<SystemPrincipal*>(sysprin);
+        return do_AddRef(gScriptSecMan->mSystemPrincipal.get()).downcast<SystemPrincipal>();
+    return nullptr;
 }
 
 struct IsWhitespace {

caps/nsScriptSecurityManager.h

@@ -53,7 +53,7 @@ public:
     // Invoked exactly once, by XPConnect.
     static void InitStatics();
 
-    static SystemPrincipal*
+    static already_AddRefed<SystemPrincipal>
     SystemPrincipalSingletonConstructor();
 
     /**

dom/base/DOMRequest.h

@@ -103,12 +103,10 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIDOMREQUESTSERVICE
 
-  // Returns an owning reference! No one should call this but the factory.
-  static DOMRequestService* FactoryCreate()
+  // No one should call this but the factory.
+  static already_AddRefed<DOMRequestService> FactoryCreate()
   {
-    DOMRequestService* res = new DOMRequestService;
-    NS_ADDREF(res);
-    return res;
+    return MakeAndAddRef<DOMRequestService>();
   }
 };
 

dom/quota/QuotaManagerService.cpp

@@ -225,14 +225,11 @@ QuotaManagerService::Get()
 }
 
 // static
-QuotaManagerService*
+already_AddRefed<QuotaManagerService>
 QuotaManagerService::FactoryCreate()
 {
-  // Returns a raw pointer that carries an owning reference! Lame, but the
-  // singleton factory macros force this.
-  QuotaManagerService* quotaManagerService = GetOrCreate();
-  NS_IF_ADDREF(quotaManagerService);
-  return quotaManagerService;
+  RefPtr<QuotaManagerService> quotaManagerService = GetOrCreate();
+  return quotaManagerService.forget();
 }
 
 void

dom/quota/QuotaManagerService.h

@@ -63,8 +63,8 @@ public:
   static QuotaManagerService*
   Get();
 
-  // Returns an owning reference! No one should call this but the factory.
-  static QuotaManagerService*
+  // No one should call this but the factory.
+  static already_AddRefed<QuotaManagerService>
   FactoryCreate();
 
   void

extensions/cookie/nsPermissionManager.cpp

@@ -926,12 +926,11 @@ nsPermissionManager::~nsPermissionManager()
 }
 
 // static
-nsIPermissionManager*
+already_AddRefed<nsIPermissionManager>
 nsPermissionManager::GetXPCOMSingleton()
 {
   if (gPermissionManager) {
-    NS_ADDREF(gPermissionManager);
-    return gPermissionManager;
+    return do_AddRef(gPermissionManager);
   }
 
   // Create a new singleton nsPermissionManager.
@@ -940,15 +939,13 @@ nsPermissionManager::GetXPCOMSingleton()
   // Release our members, since GC cycles have already been completed and
   // would result in serious leaks.
   // See bug 209571.
-  gPermissionManager = new nsPermissionManager();
-  if (gPermissionManager) {
-    NS_ADDREF(gPermissionManager);
-    if (NS_FAILED(gPermissionManager->Init())) {
-      NS_RELEASE(gPermissionManager);
-    }
+  auto permManager = MakeRefPtr<nsPermissionManager>();
+  if (NS_SUCCEEDED(permManager->Init())) {
+    gPermissionManager = permManager.get();
+    return permManager.forget();
   }
 
-  return gPermissionManager;
+  return nullptr;;
 }
 
 nsresult

extensions/cookie/nsPermissionManager.h

@@ -167,7 +167,7 @@ public:
   NS_DECL_NSIOBSERVER
 
   nsPermissionManager();
-  static nsIPermissionManager* GetXPCOMSingleton();
+  static already_AddRefed<nsIPermissionManager> GetXPCOMSingleton();
   nsresult Init();
 
   // enums for AddInternal()

js/xpconnect/src/nsXPConnect.cpp

@@ -141,12 +141,10 @@ nsXPConnect::InitStatics()
     gSelf->mRuntime->InitSingletonScopes();
 }
 
-nsXPConnect*
+already_AddRefed<nsXPConnect>
 nsXPConnect::GetSingleton()
 {
-    nsXPConnect* xpc = nsXPConnect::XPConnect();
-    NS_IF_ADDREF(xpc);
-    return xpc;
+    return do_AddRef(nsXPConnect::XPConnect());
 }
 
 // static

js/xpconnect/src/xpcprivate.h

@@ -257,10 +257,7 @@ public:
         return gSystemPrincipal;
     }
 
-    // This returns an AddRef'd pointer. It does not do this with an 'out' param
-    // only because this form is required by the generic module macro:
-    // NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR
-    static nsXPConnect* GetSingleton();
+    static already_AddRefed<nsXPConnect> GetSingleton();
 
     // Called by module code in dll startup
     static void InitStatics();

modules/libjar/nsJARProtocolHandler.cpp

@@ -63,23 +63,19 @@ NS_IMPL_ISUPPORTS(nsJARProtocolHandler,
                   nsIProtocolHandler,
                   nsISupportsWeakReference)
 
-nsJARProtocolHandler*
+already_AddRefed<nsJARProtocolHandler>
 nsJARProtocolHandler::GetSingleton()
 {
     if (!gJarHandler) {
-        gJarHandler = new nsJARProtocolHandler();
-        if (!gJarHandler)
-            return nullptr;
-
-        NS_ADDREF(gJarHandler);
-        nsresult rv = gJarHandler->Init();
-        if (NS_FAILED(rv)) {
-            NS_RELEASE(gJarHandler);
+        auto jar = MakeRefPtr<nsJARProtocolHandler>();
+        gJarHandler = jar.get();
+        if (NS_FAILED(jar->Init())) {
+            gJarHandler = nullptr;
             return nullptr;
         }
+        return jar.forget();
     }
-    NS_ADDREF(gJarHandler);
-    return gJarHandler;
+    return do_AddRef(gJarHandler);
 }
 
 NS_IMETHODIMP

modules/libjar/nsJARProtocolHandler.h

@@ -25,7 +25,7 @@ public:
     // nsJARProtocolHandler methods:
     nsJARProtocolHandler();
 
-    static nsJARProtocolHandler *GetSingleton();
+    static already_AddRefed<nsJARProtocolHandler> GetSingleton();
 
     nsresult Init();
 

modules/libpref/Preferences.cpp

@@ -3826,12 +3826,11 @@ public:
 
 } // namespace
 
-/* static */ Preferences*
+/* static */ already_AddRefed<Preferences>
 Preferences::GetInstanceForService()
 {
   if (sPreferences) {
-    NS_ADDREF(sPreferences);
-    return sPreferences;
+    return do_AddRef(sPreferences);
   }
 
   if (sShutdown) {
@@ -3868,8 +3867,7 @@ Preferences::GetInstanceForService()
     new AddPreferencesMemoryReporterRunnable();
   NS_DispatchToMainThread(runnable);
 
-  NS_ADDREF(sPreferences);
-  return sPreferences;
+  return do_AddRef(sPreferences);
 }
 
 /* static */ bool

modules/libpref/Preferences.h

@@ -78,7 +78,7 @@ public:
   static void InitializeUserPrefs();
 
   // Returns the singleton instance which is addreffed.
-  static Preferences* GetInstanceForService();
+  static already_AddRefed<Preferences> GetInstanceForService();
 
   // Finallizes global members.
   static void Shutdown();

netwerk/base/nsIOService.cpp

@@ -351,23 +351,19 @@ nsIOService::InitializeProtocolProxyService()
     return rv;
 }
 
-nsIOService*
+already_AddRefed<nsIOService>
 nsIOService::GetInstance() {
     if (!gIOService) {
-        gIOService = new nsIOService();
-        if (!gIOService)
-            return nullptr;
-        NS_ADDREF(gIOService);
-
-        nsresult rv = gIOService->Init();
-        if (NS_FAILED(rv)) {
-            NS_RELEASE(gIOService);
+        RefPtr<nsIOService> ios = new nsIOService();
+        gIOService = ios.get();
+        if (NS_FAILED(ios->Init())) {
+            gIOService = nullptr;
             return nullptr;
         }
-        return gIOService;
+
+        return ios.forget();
     }
-    NS_ADDREF(gIOService);
-    return gIOService;
+    return do_AddRef(gIOService);
 }
 
 NS_IMPL_ISUPPORTS(nsIOService,

netwerk/base/nsIOService.h

@@ -64,8 +64,7 @@ public:
 
     // Gets the singleton instance of the IO Service, creating it as needed
     // Returns nullptr on out of memory or failure to initialize.
-    // Returns an addrefed pointer.
-    static nsIOService* GetInstance();
+    static already_AddRefed<nsIOService> GetInstance();
 
     nsresult Init();
     nsresult NewURI(const char* aSpec, nsIURI* aBaseURI,

netwerk/dns/ChildDNSService.cpp

@@ -26,7 +26,7 @@ namespace net {
 static ChildDNSService *gChildDNSService;
 static const char kPrefNameDisablePrefetch[] = "network.dns.disablePrefetch";
 
-ChildDNSService* ChildDNSService::GetSingleton()
+already_AddRefed<ChildDNSService> ChildDNSService::GetSingleton()
 {
   MOZ_ASSERT(IsNeckoChild());
 
@@ -34,8 +34,7 @@ ChildDNSService* ChildDNSService::GetSingleton()
     gChildDNSService = new ChildDNSService();
   }
 
-  NS_ADDREF(gChildDNSService);
-  return gChildDNSService;
+  return do_AddRef(gChildDNSService);
 }
 
 NS_IMPL_ISUPPORTS(ChildDNSService,

netwerk/dns/ChildDNSService.h

@@ -32,7 +32,7 @@ public:
 
   ChildDNSService();
 
-  static ChildDNSService* GetSingleton();
+  static already_AddRefed<ChildDNSService> GetSingleton();
 
   void NotifyRequestDone(DNSRequestChild *aDnsRequest);
 

netwerk/dns/nsDNSService2.cpp

@@ -499,7 +499,7 @@ NS_IMPL_ISUPPORTS(nsDNSService, nsIDNSService, nsPIDNSService, nsIObserver,
  ******************************************************************************/
 static nsDNSService *gDNSService;
 
-nsIDNSService*
+already_AddRefed<nsIDNSService>
 nsDNSService::GetXPCOMSingleton()
 {
     if (IsNeckoChild()) {
@@ -509,25 +509,23 @@ nsDNSService::GetXPCOMSingleton()
     return GetSingleton();
 }
 
-nsDNSService*
+already_AddRefed<nsDNSService>
 nsDNSService::GetSingleton()
 {
     NS_ASSERTION(!IsNeckoChild(), "not a parent process");
 
     if (gDNSService) {
-        NS_ADDREF(gDNSService);
-        return gDNSService;
+        return do_AddRef(gDNSService);
     }
 
-    gDNSService = new nsDNSService();
-    if (gDNSService) {
-        NS_ADDREF(gDNSService);
-        if (NS_FAILED(gDNSService->Init())) {
-              NS_RELEASE(gDNSService);
-        }
+    auto dns = MakeRefPtr<nsDNSService>();
+    gDNSService = dns.get();
+    if (NS_FAILED(dns->Init())) {
+        gDNSService = nullptr;
+        return nullptr;
     }
 
-    return gDNSService;
+    return dns.forget();
 }
 
 NS_IMETHODIMP

netwerk/dns/nsDNSService2.h

@@ -34,7 +34,7 @@ public:
 
     nsDNSService();
 
-    static nsIDNSService* GetXPCOMSingleton();
+    static already_AddRefed<nsIDNSService> GetXPCOMSingleton();
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
@@ -51,7 +51,7 @@ protected:
 private:
     ~nsDNSService();
 
-    static nsDNSService* GetSingleton();
+    static already_AddRefed<nsDNSService> GetSingleton();
 
     uint16_t GetAFForLookup(const nsACString &host, uint32_t flags);
 

startupcache/StartupCache.cpp

@@ -700,13 +700,12 @@ StartupCacheWrapper* StartupCacheWrapper::gStartupCacheWrapper = nullptr;
 
 NS_IMPL_ISUPPORTS(StartupCacheWrapper, nsIStartupCache)
 
-StartupCacheWrapper* StartupCacheWrapper::GetSingleton()
+already_AddRefed<StartupCacheWrapper> StartupCacheWrapper::GetSingleton()
 {
   if (!gStartupCacheWrapper)
     gStartupCacheWrapper = new StartupCacheWrapper();
 
-  NS_ADDREF(gStartupCacheWrapper);
-  return gStartupCacheWrapper;
+  return do_AddRef(gStartupCacheWrapper);
 }
 
 nsresult

startupcache/StartupCache.h

@@ -214,7 +214,7 @@ class StartupCacheWrapper final
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSISTARTUPCACHE
 
-  static StartupCacheWrapper* GetSingleton();
+  static already_AddRefed<StartupCacheWrapper> GetSingleton();
   static StartupCacheWrapper *gStartupCacheWrapper;
 };
 

storage/VacuumManager.cpp

@@ -311,7 +311,7 @@ NS_IMPL_ISUPPORTS(
 VacuumManager *
 VacuumManager::gVacuumManager = nullptr;
 
-VacuumManager *
+already_AddRefed<VacuumManager>
 VacuumManager::getSingleton()
 {
   //Don't allocate it in the child Process.
@@ -319,15 +319,10 @@ VacuumManager::getSingleton()
     return nullptr;
   }
 
-  if (gVacuumManager) {
-    NS_ADDREF(gVacuumManager);
-    return gVacuumManager;
+  if (!gVacuumManager) {
+    gVacuumManager = new VacuumManager();
   }
-  gVacuumManager = new VacuumManager();
-  if (gVacuumManager) {
-    NS_ADDREF(gVacuumManager);
-  }
-  return gVacuumManager;
+  return do_AddRef(gVacuumManager);
 }
 
 VacuumManager::VacuumManager()

storage/VacuumManager.h

@@ -28,7 +28,7 @@ public:
   /**
    * Obtains the VacuumManager object.
    */
-  static VacuumManager * getSingleton();
+  static already_AddRefed<VacuumManager> getSingleton();
 
 private:
   ~VacuumManager();

storage/mozStorageService.cpp

@@ -193,12 +193,11 @@ NS_IMPL_ISUPPORTS(
 
 Service *Service::gService = nullptr;
 
-Service *
+already_AddRefed<Service>
 Service::getSingleton()
 {
   if (gService) {
-    NS_ADDREF(gService);
-    return gService;
+    return do_AddRef(gService);
   }
 
   // Ensure that we are using the same version of SQLite that we compiled with
@@ -222,14 +221,14 @@ Service::getSingleton()
   // The first reference to the storage service must be obtained on the
   // main thread.
   NS_ENSURE_TRUE(NS_IsMainThread(), nullptr);
-  gService = new Service();
-  if (gService) {
-    NS_ADDREF(gService);
-    if (NS_FAILED(gService->initialize()))
-      NS_RELEASE(gService);
+  RefPtr<Service> service = new Service();
+  gService = service.get();
+  if (NS_FAILED(service->initialize())) {
+    gService = nullptr;
+    return nullptr;
   }
 
-  return gService;
+  return service.forget();
 }
 
 nsIXPConnect *Service::sXPConnect = nullptr;

storage/mozStorageService.h

@@ -52,7 +52,7 @@ public:
                            const nsAString &aStr2,
                            int32_t aComparisonStrength);
 
-  static Service *getSingleton();
+  static already_AddRefed<Service> getSingleton();
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_MOZISTORAGESERVICE

toolkit/components/downloads/ApplicationReputation.cpp

@@ -1552,21 +1552,13 @@ NS_IMPL_ISUPPORTS(ApplicationReputationService,
 ApplicationReputationService*
   ApplicationReputationService::gApplicationReputationService = nullptr;
 
-ApplicationReputationService*
+already_AddRefed<ApplicationReputationService>
 ApplicationReputationService::GetSingleton()
 {
-  if (gApplicationReputationService) {
-    NS_ADDREF(gApplicationReputationService);
-    return gApplicationReputationService;
+  if (!gApplicationReputationService) {
+    gApplicationReputationService = new ApplicationReputationService();
   }
-
-  // We're not initialized yet.
-  gApplicationReputationService = new ApplicationReputationService();
-  if (gApplicationReputationService) {
-    NS_ADDREF(gApplicationReputationService);
-  }
-
-  return gApplicationReputationService;
+  return do_AddRef(gApplicationReputationService);
 }
 
 ApplicationReputationService::ApplicationReputationService()

toolkit/components/downloads/ApplicationReputation.h

@@ -27,7 +27,7 @@ public:
   NS_DECL_NSIAPPLICATIONREPUTATIONSERVICE
 
 public:
-  static ApplicationReputationService* GetSingleton();
+  static already_AddRefed<ApplicationReputationService> GetSingleton();
 
 private:
   friend class PendingLookup;

toolkit/components/downloads/nsDownloadManager.cpp

@@ -25,22 +25,20 @@ NS_IMPL_ISUPPORTS(
 
 nsDownloadManager *nsDownloadManager::gDownloadManagerService = nullptr;
 
-nsDownloadManager *
+already_AddRefed<nsDownloadManager>
 nsDownloadManager::GetSingleton()
 {
   if (gDownloadManagerService) {
-    NS_ADDREF(gDownloadManagerService);
-    return gDownloadManagerService;
+    return do_AddRef(gDownloadManagerService);
   }
 
-  gDownloadManagerService = new nsDownloadManager();
-  if (gDownloadManagerService) {
-    NS_ADDREF(gDownloadManagerService);
-    if (NS_FAILED(gDownloadManagerService->Init()))
-      NS_RELEASE(gDownloadManagerService);
+  auto serv = MakeRefPtr<nsDownloadManager>();
+  gDownloadManagerService = serv.get();
+  if (NS_FAILED(serv->Init())) {
+    gDownloadManagerService = nullptr;
+    return nullptr;
   }
-
-  return gDownloadManagerService;
+  return serv.forget();
 }
 
 nsDownloadManager::~nsDownloadManager()

toolkit/components/downloads/nsDownloadManager.h

@@ -21,7 +21,7 @@ public:
 
   nsresult Init();
 
-  static nsDownloadManager *GetSingleton();
+  static already_AddRefed<nsDownloadManager> GetSingleton();
 
   nsDownloadManager()
   {

toolkit/components/places/History.cpp

@@ -2421,7 +2421,7 @@ History::GetService()
 }
 
 /* static */
-History*
+already_AddRefed<History>
 History::GetSingleton()
 {
   if (!gService) {
@@ -2430,8 +2430,7 @@ History::GetSingleton()
     gService->InitMemoryReporter();
   }
 
-  NS_ADDREF(gService);
-  return gService;
+  return do_AddRef(gService);
 }
 
 mozIStorageConnection*

toolkit/components/places/History.h

@@ -107,10 +107,9 @@ public:
   static History* GetService();
 
   /**
-   * Obtains a pointer that has had AddRef called on it.  Used by the service
-   * manager only.
+   * Used by the service manager only.
    */
-  static History* GetSingleton();
+  static already_AddRefed<History> GetSingleton();
 
   template<int N>
   already_AddRefed<mozIStorageStatement>

toolkit/mozapps/extensions/AddonPathService.cpp

@@ -57,14 +57,13 @@ NS_IMPL_ISUPPORTS(AddonPathService, amIAddonPathService)
 
 AddonPathService *AddonPathService::sInstance;
 
-/* static */ AddonPathService*
+/* static */ already_AddRefed<AddonPathService>
 AddonPathService::GetInstance()
 {
   if (!sInstance) {
     sInstance = new AddonPathService();
   }
-  NS_ADDREF(sInstance);
-  return sInstance;
+  return do_AddRef(sInstance);
 }
 
 static JSAddonId*

toolkit/mozapps/extensions/AddonPathService.h

@@ -23,7 +23,7 @@ class AddonPathService final : public amIAddonPathService
 public:
   AddonPathService();
 
-  static AddonPathService* GetInstance();
+  static already_AddRefed<AddonPathService> GetInstance();
 
   JSAddonId* Find(const nsAString& path);
   static JSAddonId* FindAddonId(const nsAString& path);

uriloader/prefetch/nsOfflineCacheUpdate.h

@@ -356,8 +356,7 @@ public:
      */
     static nsOfflineCacheUpdateService *EnsureService();
 
-    /** Addrefs and returns the singleton nsOfflineCacheUpdateService. */
-    static nsOfflineCacheUpdateService *GetInstance();
+    static already_AddRefed<nsOfflineCacheUpdateService> GetInstance();
 
     static nsresult OfflineAppPinnedForURI(nsIURI *aDocumentURI,
                                            nsIPrefBranch *aPrefBranch,

uriloader/prefetch/nsOfflineCacheUpdateService.cpp

@@ -292,25 +292,20 @@ nsOfflineCacheUpdateService::Init()
 }
 
 /* static */
-nsOfflineCacheUpdateService *
+already_AddRefed<nsOfflineCacheUpdateService>
 nsOfflineCacheUpdateService::GetInstance()
 {
     if (!gOfflineCacheUpdateService) {
-        gOfflineCacheUpdateService = new nsOfflineCacheUpdateService();
-        if (!gOfflineCacheUpdateService)
-            return nullptr;
-        NS_ADDREF(gOfflineCacheUpdateService);
-        nsresult rv = gOfflineCacheUpdateService->Init();
-        if (NS_FAILED(rv)) {
-            NS_RELEASE(gOfflineCacheUpdateService);
+        auto serv = MakeRefPtr<nsOfflineCacheUpdateService>();
+        gOfflineCacheUpdateService = serv.get();
+        if (NS_FAILED(serv->Init())) {
+            gOfflineCacheUpdateService = nullptr;
             return nullptr;
         }
-        return gOfflineCacheUpdateService;
+        return serv.forget();
     }
 
-    NS_ADDREF(gOfflineCacheUpdateService);
-
-    return gOfflineCacheUpdateService;
+    return do_AddRef(gOfflineCacheUpdateService);
 }
 
 /* static */

xpcom/components/ModuleUtils.h

@@ -49,8 +49,25 @@ _InstanceClass##Constructor(nsISupports *aOuter, REFNSIID aIID,               \
   return rv;                                                                  \
 }
 
+namespace mozilla {
+namespace detail {
+
+template<typename T>
+struct RemoveAlreadyAddRefed
+{
+  using Type = T;
+};
+
+template<typename T>
+struct RemoveAlreadyAddRefed<already_AddRefed<T>>
+{
+  using Type = T;
+};
+
+} // namespace detail
+} // namespace mozilla
+
 // 'Constructor' that uses an existing getter function that gets a singleton.
-// NOTE: assumes that getter does an AddRef - so additional AddRef is not done.
 #define NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(_InstanceClass, _GetterProc) \
 static nsresult                                                               \
 _InstanceClass##Constructor(nsISupports *aOuter, REFNSIID aIID,               \
@@ -63,7 +80,12 @@ _InstanceClass##Constructor(nsISupports *aOuter, REFNSIID aIID,               \
     return NS_ERROR_NO_AGGREGATION;                                           \
   }                                                                           \
                                                                               \
-  inst = already_AddRefed<_InstanceClass>(_GetterProc());                     \
+  using T = mozilla::detail::RemoveAlreadyAddRefed<decltype(_GetterProc())>::Type; \
+  static_assert(mozilla::IsSame<already_AddRefed<T>, decltype(_GetterProc())>::value, \
+                "Singleton constructor must return already_AddRefed");        \
+  static_assert(mozilla::IsBaseOf<_InstanceClass, T>::value,                  \
+                "Singleton constructor must return correct already_AddRefed");\
+  inst = _GetterProc();                                                       \
   if (nullptr == inst) {                                                      \
     return NS_ERROR_OUT_OF_MEMORY;                                            \
   }                                                                           \