Bug 1396676: Return already_AddRefed from cookie service GetSingleton() methods. r?jdm
These methods return an addrefed raw pointer, which makes them easy to use in
ways that cause leaks. If they're to continue returning an addrefed pointer,
they should explicitly return an already_AddRefed.
This also switches to StaticRefPtr with ClearOnShutdown for the cached
pointers for the sake of sanity.
MozReview-Commit-ID: D0lDpU8Hqug
--- a/netwerk/cookie/CookieServiceChild.cpp
+++ b/netwerk/cookie/CookieServiceChild.cpp
@@ -2,16 +2,17 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "mozilla/net/CookieServiceChild.h"
#include "mozilla/net/NeckoChannelParams.h"
#include "mozilla/LoadInfo.h"
#include "mozilla/BasePrincipal.h"
+#include "mozilla/ClearOnShutdown.h"
#include "mozilla/dom/ContentChild.h"
#include "mozilla/ipc/URIUtils.h"
#include "mozilla/net/NeckoChild.h"
#include "mozilla/SystemGroup.h"
#include "nsCookie.h"
#include "nsCookieService.h"
#include "nsContentUtils.h"
#include "nsNetCID.h"
@@ -31,26 +32,27 @@ namespace net {
// Pref string constants
static const char kPrefCookieBehavior[] = "network.cookie.cookieBehavior";
static const char kPrefThirdPartySession[] =
"network.cookie.thirdparty.sessionOnly";
static const char kPrefCookieIPCSync[] = "network.cookie.ipc.sync";
static const char kCookieLeaveSecurityAlone[] = "network.cookie.leave-secure-alone";
-static CookieServiceChild *gCookieService;
+static StaticRefPtr<CookieServiceChild> gCookieService;
-CookieServiceChild*
+already_AddRefed<CookieServiceChild>
CookieServiceChild::GetSingleton()
{
- if (!gCookieService)
+ if (!gCookieService) {
gCookieService = new CookieServiceChild();
+ ClearOnShutdown(&gCookieService);
+ }
- NS_ADDREF(gCookieService);
- return gCookieService;
+ return do_AddRef(gCookieService);
}
NS_IMPL_ISUPPORTS(CookieServiceChild,
nsICookieService,
nsIObserver,
nsISupportsWeakReference)
CookieServiceChild::CookieServiceChild()
--- a/netwerk/cookie/CookieServiceChild.h
+++ b/netwerk/cookie/CookieServiceChild.h
@@ -35,17 +35,17 @@ public:
NS_DECL_NSICOOKIESERVICE
NS_DECL_NSIOBSERVER
typedef nsTArray<RefPtr<nsCookie>> CookiesList;
typedef nsClassHashtable<nsCookieKey, CookiesList> CookiesMap;
CookieServiceChild();
- static CookieServiceChild* GetSingleton();
+ static already_AddRefed<CookieServiceChild> GetSingleton();
void
TrackCookieLoad(nsIChannel *aChannel);
protected:
virtual ~CookieServiceChild();
void SerializeURIs(nsIURI *aHostURI,
--- a/netwerk/cookie/CookieServiceParent.cpp
+++ b/netwerk/cookie/CookieServiceParent.cpp
@@ -65,18 +65,17 @@ namespace net {
CookieServiceParent::CookieServiceParent()
{
// Instantiate the cookieservice via the service manager, so it sticks around
// until shutdown.
nsCOMPtr<nsICookieService> cs = do_GetService(NS_COOKIESERVICE_CONTRACTID);
// Get the nsCookieService instance directly, so we can call internal methods.
- mCookieService =
- already_AddRefed<nsCookieService>(nsCookieService::GetSingleton());
+ mCookieService = nsCookieService::GetSingleton();
NS_ASSERTION(mCookieService, "couldn't get nsICookieService");
}
CookieServiceParent::~CookieServiceParent()
{
}
void
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -1,15 +1,16 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set sw=2 ts=8 et tw=80 : */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "mozilla/Attributes.h"
+#include "mozilla/ClearOnShutdown.h"
#include "mozilla/DebugOnly.h"
#include "mozilla/Likely.h"
#include "mozilla/Printf.h"
#include "mozilla/Unused.h"
#include "mozilla/net/CookieServiceChild.h"
#include "mozilla/net/NeckoCommon.h"
@@ -67,17 +68,17 @@ using namespace mozilla::net;
#define DEFAULT_APP_KEY(baseDomain) \
nsCookieKey(baseDomain, OriginAttributes())
/******************************************************************************
* nsCookieService impl:
* useful types & constants
******************************************************************************/
-static nsCookieService *gCookieService;
+static StaticRefPtr<nsCookieService> gCookieService;
// XXX_hack. See bug 178993.
// This is a hack to hide HttpOnly cookies from older browsers
#define HTTP_ONLY_PREFIX "#HttpOnly_"
#define COOKIES_FILE "cookies.sqlite"
#define COOKIES_SCHEMA_VERSION 8
@@ -633,50 +634,50 @@ DBState::SizeOfIncludingThis(MallocSizeO
return amount;
}
/******************************************************************************
* nsCookieService impl:
* singleton instance ctor/dtor methods
******************************************************************************/
-nsICookieService*
+already_AddRefed<nsICookieService>
nsCookieService::GetXPCOMSingleton()
{
if (IsNeckoChild())
return CookieServiceChild::GetSingleton();
return GetSingleton();
}
-nsCookieService*
+already_AddRefed<nsCookieService>
nsCookieService::GetSingleton()
{
NS_ASSERTION(!IsNeckoChild(), "not a parent process");
if (gCookieService) {
- NS_ADDREF(gCookieService);
- return gCookieService;
+ return do_AddRef(gCookieService);
}
// Create a new singleton nsCookieService.
// We AddRef only once since XPCOM has rules about the ordering of module
// teardowns - by the time our module destructor is called, it's too late to
// Release our members (e.g. nsIObserverService and nsIPrefBranch), since GC
// cycles have already been completed and would result in serious leaks.
// See bug 209571.
gCookieService = new nsCookieService();
if (gCookieService) {
- NS_ADDREF(gCookieService);
- if (NS_FAILED(gCookieService->Init())) {
- NS_RELEASE(gCookieService);
+ if (NS_SUCCEEDED(gCookieService->Init())) {
+ ClearOnShutdown(&gCookieService);
+ } else {
+ gCookieService = nullptr;
}
}
- return gCookieService;
+ return do_AddRef(gCookieService);
}
/* static */ void
nsCookieService::AppClearDataObserverInit()
{
nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
nsCOMPtr<nsIObserver> obs = new AppClearDataObserver();
observerService->AddObserver(obs, TOPIC_CLEAR_ORIGIN_DATA,
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -210,18 +210,18 @@ class nsCookieService final : public nsI
NS_DECL_ISUPPORTS
NS_DECL_NSIOBSERVER
NS_DECL_NSICOOKIESERVICE
NS_DECL_NSICOOKIEMANAGER
NS_DECL_NSICOOKIEMANAGER2
NS_DECL_NSIMEMORYREPORTER
nsCookieService();
- static nsICookieService* GetXPCOMSingleton();
- nsresult Init();
+ static already_AddRefed<nsICookieService> GetXPCOMSingleton();
+ nsresult Init();
/**
* Start watching the observer service for messages indicating that an app has
* been uninstalled. When an app is uninstalled, we get the cookie service
* (thus instantiating it, if necessary) and clear all the cookies for that
* app.
*/
static void AppClearDataObserverInit();
@@ -325,13 +325,13 @@ class nsCookieService final : public nsI
uint16_t mMaxCookiesPerHost;
int64_t mCookiePurgeAge;
// friends!
friend class DBListenerErrorHandler;
friend class ReadCookieDBListener;
friend class CloseCookieDBListener;
- static nsCookieService* GetSingleton();
+ static already_AddRefed<nsCookieService> GetSingleton();
friend class mozilla::net::CookieServiceParent;
};
#endif // nsCookieService_h__
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -198,17 +198,17 @@ HttpChannelChild::HttpChannelChild()
sSecurityPrefChecked = true;
}
#endif
// Ensure that the cookie service is initialized before the first
// IPC HTTP channel is created.
// We require that the parent cookie service actor exists while
// processing HTTP responses.
- CookieServiceChild::GetSingleton();
+ RefPtr<CookieServiceChild> cookieService = CookieServiceChild::GetSingleton();
}
HttpChannelChild::~HttpChannelChild()
{
LOG(("Destroying HttpChannelChild @%p\n", this));
ReleaseMainThreadOnlyReferences();
}
--- a/xpcom/base/StaticPtr.h
+++ b/xpcom/base/StaticPtr.h
@@ -269,9 +269,17 @@ RefPtr<T>::RefPtr(const mozilla::StaticR
template<class T> template<class U>
RefPtr<T>&
RefPtr<T>::operator=(const mozilla::StaticRefPtr<U>& aOther)
{
return operator=(aOther.get());
}
+template <class T>
+inline already_AddRefed<T>
+do_AddRef(const mozilla::StaticRefPtr<T>& aObj)
+{
+ RefPtr<T> ref(aObj);
+ return ref.forget();
+}
+
#endif