Bug 1554377 - Cleanup nsCookie implementation - network_cookie_thirdparty_nonsecureSessionOnly to StaticPrefs, r=Ehsan
authorAndrea Marchesini <amarchesini@mozilla.com>
Fri, 31 May 2019 09:32:33 +0000
changeset 476341 6c5a7ea701e20d22a182ff9959442f8f2217b70b
parent 476340 4a90f6a4c101d74ba4aa96d33fbf1fdb4bbc8f2d
child 476342 519ab3e488483b7a13c5787164e469cda3795ad0
push id36092
push userarchaeopteryx@coole-files.de
push dateFri, 31 May 2019 17:03:46 +0000
treeherdermozilla-central@8384972e1f6a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1554377
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1554377 - Cleanup nsCookie implementation - network_cookie_thirdparty_nonsecureSessionOnly to StaticPrefs, r=Ehsan Differential Revision: https://phabricator.services.mozilla.com/D32615
modules/libpref/init/StaticPrefList.h
modules/libpref/init/all.js
netwerk/cookie/CookieServiceChild.cpp
netwerk/cookie/CookieServiceChild.h
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -5720,16 +5720,23 @@ VARCACHE_PREF(
 
 VARCACHE_PREF(
   Live,
   "network.cookie.thirdparty.sessionOnly",
    network_cookie_thirdparty_sessionOnly,
   bool, false
 )
 
+VARCACHE_PREF(
+  Live,
+  "network.cookie.thirdparty.nonsecureSessionOnly",
+   network_cookie_thirdparty_nonsecureSessionOnly,
+  bool, false
+)
+
 // Enables the predictive service.
 VARCACHE_PREF(
   Live,
   "network.predictor.enabled",
   network_predictor_enabled,
   bool, true
 )
 
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2322,17 +2322,16 @@ pref("network.proxy.socks_port",        
 pref("network.proxy.socks_version",         5);
 pref("network.proxy.socks_remote_dns",      false);
 pref("network.proxy.proxy_over_tls",        true);
 pref("network.proxy.no_proxies_on",         "");
 // Set true to allow resolving proxy for localhost
 pref("network.proxy.allow_hijacking_localhost", false);
 pref("network.proxy.failover_timeout",      1800); // 30 minutes
 pref("network.online",                      true); //online/offline
-pref("network.cookie.thirdparty.nonsecureSessionOnly", false);
 
 // The interval in seconds to move the cookies in the child process.
 // Set to 0 to disable moving the cookies.
 pref("network.cookie.move.interval_sec",    10);
 
 pref("network.cookie.maxNumber", 3000);
 pref("network.cookie.maxPerHost", 180);
 // Cookies quota for each host. If cookies exceed the limit maxPerHost,
--- a/netwerk/cookie/CookieServiceChild.cpp
+++ b/netwerk/cookie/CookieServiceChild.cpp
@@ -32,18 +32,16 @@
 
 using namespace mozilla::ipc;
 using mozilla::OriginAttributes;
 
 namespace mozilla {
 namespace net {
 
 // Pref string constants
-static const char kPrefThirdPartyNonsecureSession[] =
-    "network.cookie.thirdparty.nonsecureSessionOnly";
 static const char kCookieMoveIntervalSecs[] =
     "network.cookie.move.interval_sec";
 
 static StaticRefPtr<CookieServiceChild> gCookieService;
 static uint32_t gMoveCookiesIntervalSeconds = 10;
 
 already_AddRefed<CookieServiceChild> CookieServiceChild::GetSingleton() {
   if (!gCookieService) {
@@ -52,18 +50,17 @@ already_AddRefed<CookieServiceChild> Coo
   }
 
   return do_AddRef(gCookieService);
 }
 
 NS_IMPL_ISUPPORTS(CookieServiceChild, nsICookieService, nsIObserver,
                   nsITimerCallback, nsISupportsWeakReference)
 
-CookieServiceChild::CookieServiceChild()
-    : mThirdPartyNonsecureSession(false), mIPCOpen(false) {
+CookieServiceChild::CookieServiceChild() : mIPCOpen(false) {
   NS_ASSERTION(IsNeckoChild(), "not a child process");
 
   mozilla::dom::ContentChild* cc =
       static_cast<mozilla::dom::ContentChild*>(gNeckoChild->Manager());
   if (cc->IsShuttingDown()) {
     return;
   }
 
@@ -82,17 +79,16 @@ CookieServiceChild::CookieServiceChild()
 
   mTLDService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID);
   NS_ASSERTION(mTLDService, "couldn't get TLDService");
 
   // Init our prefs and observer.
   nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
   NS_WARNING_ASSERTION(prefBranch, "no prefservice");
   if (prefBranch) {
-    prefBranch->AddObserver(kPrefThirdPartyNonsecureSession, this, true);
     prefBranch->AddObserver(kCookieMoveIntervalSecs, this, true);
     PrefChanged(prefBranch);
   }
 
   nsCOMPtr<nsIObserverService> observerService =
       mozilla::services::GetObserverService();
   if (observerService) {
     observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
@@ -245,21 +241,16 @@ mozilla::ipc::IPCResult CookieServiceChi
         aCookiesList[i].sameSite());
     RecordDocumentCookie(cookie, aAttrs);
   }
 
   return IPC_OK();
 }
 
 void CookieServiceChild::PrefChanged(nsIPrefBranch* aPrefBranch) {
-  bool boolval;
-  if (NS_SUCCEEDED(
-          aPrefBranch->GetBoolPref(kPrefThirdPartyNonsecureSession, &boolval)))
-    mThirdPartyNonsecureSession = boolval;
-
   int32_t val;
   if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookieMoveIntervalSecs, &val))) {
     gMoveCookiesIntervalSeconds = clamped<uint32_t>(val, 0, 3600);
     if (gMoveCookiesIntervalSeconds && !mCookieTimer) {
       NS_NewTimerWithCallback(getter_AddRefs(mCookieTimer), this,
                               gMoveCookiesIntervalSeconds * 1000,
                               nsITimer::TYPE_REPEATING_SLACK_LOW_PRIORITY);
     }
@@ -309,18 +300,18 @@ void CookieServiceChild::GetCookieString
   aHostURI->SchemeIs("https", &isSecure);
   int64_t currentTimeInUsec = PR_Now();
   int64_t currentTime = currentTimeInUsec / PR_USEC_PER_SEC;
 
   nsCOMPtr<nsICookieSettings> cookieSettings =
       nsCookieService::GetCookieSettings(aChannel);
 
   CookieStatus cookieStatus = nsCookieService::CheckPrefs(
-      cookieSettings, mThirdPartyNonsecureSession, aHostURI, aIsForeign,
-      aIsTrackingResource, aFirstPartyStorageAccessGranted, nullptr,
+      cookieSettings, aHostURI, aIsForeign, aIsTrackingResource,
+      aFirstPartyStorageAccessGranted, nullptr,
       CountCookiesFromHashTable(baseDomain, attrs), attrs, &aRejectedReason);
 
   if (cookieStatus != STATUS_ACCEPTED &&
       cookieStatus != STATUS_ACCEPT_SESSION) {
     return;
   }
 
   cookiesList->Sort(CompareCookiesForSending());
@@ -414,17 +405,17 @@ bool CookieServiceChild::RequireThirdPar
 
   uint32_t cookieBehavior = cookieSettings->GetCookieBehavior();
   return cookieBehavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
          cookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN ||
          cookieBehavior == nsICookieService::BEHAVIOR_REJECT_TRACKER ||
          cookieBehavior ==
              nsICookieService::BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN ||
          StaticPrefs::network_cookie_thirdparty_sessionOnly() ||
-         mThirdPartyNonsecureSession;
+         StaticPrefs::network_cookie_thirdparty_nonsecureSessionOnly();
 }
 
 void CookieServiceChild::RecordDocumentCookie(nsCookie* aCookie,
                                               const OriginAttributes& aAttrs) {
   nsAutoCString baseDomain;
   nsCookieService::GetBaseDomainFromHost(mTLDService, aCookie->Host(),
                                          baseDomain);
 
@@ -583,18 +574,18 @@ nsresult CookieServiceChild::SetCookieSt
   nsCString baseDomain;
   nsCookieService::GetBaseDomain(mTLDService, aHostURI, baseDomain,
                                  requireHostMatch);
 
   nsCOMPtr<nsICookieSettings> cookieSettings =
       nsCookieService::GetCookieSettings(aChannel);
 
   CookieStatus cookieStatus = nsCookieService::CheckPrefs(
-      cookieSettings, mThirdPartyNonsecureSession, aHostURI, isForeign,
-      isTrackingResource, firstPartyStorageAccessGranted, aCookieString,
+      cookieSettings, aHostURI, isForeign, isTrackingResource,
+      firstPartyStorageAccessGranted, aCookieString,
       CountCookiesFromHashTable(baseDomain, attrs), attrs, &rejectedReason);
 
   if (cookieStatus != STATUS_ACCEPTED &&
       cookieStatus != STATUS_ACCEPT_SESSION) {
     return NS_OK;
   }
 
   nsCookieKey key(baseDomain, attrs);
--- a/netwerk/cookie/CookieServiceChild.h
+++ b/netwerk/cookie/CookieServiceChild.h
@@ -99,16 +99,15 @@ class CookieServiceChild : public PCooki
                                         const OriginAttributes& aAttrs);
 
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   CookiesMap mCookiesMap;
   nsCOMPtr<nsITimer> mCookieTimer;
   nsCOMPtr<mozIThirdPartyUtil> mThirdPartyUtil;
   nsCOMPtr<nsIEffectiveTLDService> mTLDService;
-  bool mThirdPartyNonsecureSession;
   bool mIPCOpen;
 };
 
 }  // namespace net
 }  // namespace mozilla
 
 #endif  // mozilla_net_CookieServiceChild_h__
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -125,18 +125,16 @@ static const uint32_t kCookieQuotaPerHos
 static const uint32_t kMaxBytesPerCookie = 4096;
 static const uint32_t kMaxBytesPerPath = 1024;
 
 // pref string constants
 static const char kPrefMaxNumberOfCookies[] = "network.cookie.maxNumber";
 static const char kPrefMaxCookiesPerHost[] = "network.cookie.maxPerHost";
 static const char kPrefCookieQuotaPerHost[] = "network.cookie.quotaPerHost";
 static const char kPrefCookiePurgeAge[] = "network.cookie.purgeAge";
-static const char kPrefThirdPartyNonsecureSession[] =
-    "network.cookie.thirdparty.nonsecureSessionOnly";
 
 static void bindCookieParameters(mozIStorageBindingParamsArray* aParamsArray,
                                  const nsCookieKey& aKey,
                                  const nsCookie* aCookie);
 
 // stores the nsCookieEntry entryclass and an index into the cookie array
 // within that entryclass, for purposes of storing an iteration state that
 // points to a certain cookie.
@@ -579,17 +577,16 @@ void nsCookieService::AppClearDataObserv
  * public methods
  ******************************************************************************/
 
 NS_IMPL_ISUPPORTS(nsCookieService, nsICookieService, nsICookieManager,
                   nsIObserver, nsISupportsWeakReference, nsIMemoryReporter)
 
 nsCookieService::nsCookieService()
     : mDBState(nullptr),
-      mThirdPartyNonsecureSession(false),
       mMaxNumberOfCookies(kMaxNumberOfCookies),
       mMaxCookiesPerHost(kMaxCookiesPerHost),
       mCookieQuotaPerHost(kCookieQuotaPerHost),
       mCookiePurgeAge(kCookiePurgeAge),
       mThread(nullptr),
       mMonitor("CookieThread"),
       mInitializedDBStates(false),
       mInitializedDBConn(false) {}
@@ -606,17 +603,16 @@ nsresult nsCookieService::Init() {
   NS_ENSURE_SUCCESS(rv, rv);
 
   // init our pref and observer
   nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (prefBranch) {
     prefBranch->AddObserver(kPrefMaxNumberOfCookies, this, true);
     prefBranch->AddObserver(kPrefMaxCookiesPerHost, this, true);
     prefBranch->AddObserver(kPrefCookiePurgeAge, this, true);
-    prefBranch->AddObserver(kPrefThirdPartyNonsecureSession, this, true);
     PrefChanged(prefBranch);
   }
 
   mStorageService = do_GetService("@mozilla.org/storage/service;1", &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Init our default, and possibly private DBStates.
   InitDBStates();
@@ -2167,20 +2163,20 @@ void nsCookieService::SetCookieStringInt
   nsCOMPtr<nsICookieSettings> cookieSettings = GetCookieSettings(aChannel);
 
   // check default prefs
   uint32_t priorCookieCount = 0;
   uint32_t rejectedReason = aRejectedReason;
   nsAutoCString hostFromURI;
   aHostURI->GetHost(hostFromURI);
   CountCookiesFromHost(hostFromURI, &priorCookieCount);
-  CookieStatus cookieStatus = CheckPrefs(
-      cookieSettings, mThirdPartyNonsecureSession, aHostURI, aIsForeign,
-      aIsTrackingResource, aFirstPartyStorageAccessGranted, aCookieHeader.get(),
-      priorCookieCount, aOriginAttrs, &rejectedReason);
+  CookieStatus cookieStatus =
+      CheckPrefs(cookieSettings, aHostURI, aIsForeign, aIsTrackingResource,
+                 aFirstPartyStorageAccessGranted, aCookieHeader.get(),
+                 priorCookieCount, aOriginAttrs, &rejectedReason);
 
   MOZ_ASSERT_IF(rejectedReason, cookieStatus == STATUS_REJECTED);
 
   // fire a notification if third party or if cookie was rejected
   // (but not if there was an error)
   switch (cookieStatus) {
     case STATUS_REJECTED:
       NotifyRejected(aHostURI, aChannel, rejectedReason, OPERATION_WRITE);
@@ -2339,21 +2335,16 @@ void nsCookieService::PrefChanged(nsIPre
     mMaxCookiesPerHost = (uint16_t)LIMIT(val, mCookieQuotaPerHost + 1, 0xFFFF,
                                          kMaxCookiesPerHost);
   }
 
   if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookiePurgeAge, &val))) {
     mCookiePurgeAge =
         int64_t(LIMIT(val, 0, INT32_MAX, INT32_MAX)) * PR_USEC_PER_SEC;
   }
-
-  bool boolval;
-  if (NS_SUCCEEDED(
-          aPrefBranch->GetBoolPref(kPrefThirdPartyNonsecureSession, &boolval)))
-    mThirdPartyNonsecureSession = boolval;
 }
 
 /******************************************************************************
  * nsICookieManager impl:
  * nsICookieManager
  ******************************************************************************/
 
 NS_IMETHODIMP
@@ -2991,20 +2982,20 @@ void nsCookieService::GetCookiesForURI(
   }
 
   nsCOMPtr<nsICookieSettings> cookieSettings = GetCookieSettings(aChannel);
 
   // check default prefs
   uint32_t rejectedReason = aRejectedReason;
   uint32_t priorCookieCount = 0;
   CountCookiesFromHost(hostFromURI, &priorCookieCount);
-  CookieStatus cookieStatus = CheckPrefs(
-      cookieSettings, mThirdPartyNonsecureSession, aHostURI, aIsForeign,
-      aIsTrackingResource, aFirstPartyStorageAccessGranted, nullptr,
-      priorCookieCount, aOriginAttrs, &rejectedReason);
+  CookieStatus cookieStatus =
+      CheckPrefs(cookieSettings, aHostURI, aIsForeign, aIsTrackingResource,
+                 aFirstPartyStorageAccessGranted, nullptr, priorCookieCount,
+                 aOriginAttrs, &rejectedReason);
 
   MOZ_ASSERT_IF(rejectedReason, cookieStatus == STATUS_REJECTED);
 
   // for GetCookie(), we only fire acceptance/rejection notifications
   // (but not if there was an error)
   switch (cookieStatus) {
     case STATUS_REJECTED:
       NotifyRejected(aHostURI, aChannel, rejectedReason, OPERATION_READ);
@@ -3926,21 +3917,20 @@ nsresult nsCookieService::NormalizeHost(
 static inline bool IsSubdomainOf(const nsCString& a, const nsCString& b) {
   if (a == b) return true;
   if (a.Length() > b.Length())
     return a[a.Length() - b.Length() - 1] == '.' && StringEndsWith(a, b);
   return false;
 }
 
 CookieStatus nsCookieService::CheckPrefs(
-    nsICookieSettings* aCookieSettings, bool aThirdPartyNonsecureSession,
-    nsIURI* aHostURI, bool aIsForeign, bool aIsTrackingResource,
-    bool aFirstPartyStorageAccessGranted, const char* aCookieHeader,
-    const int aNumOfCookies, const OriginAttributes& aOriginAttrs,
-    uint32_t* aRejectedReason) {
+    nsICookieSettings* aCookieSettings, nsIURI* aHostURI, bool aIsForeign,
+    bool aIsTrackingResource, bool aFirstPartyStorageAccessGranted,
+    const char* aCookieHeader, const int aNumOfCookies,
+    const OriginAttributes& aOriginAttrs, uint32_t* aRejectedReason) {
   nsresult rv;
 
   MOZ_ASSERT(aRejectedReason);
 
   uint32_t aInputRejectedReason = *aRejectedReason;
 
   *aRejectedReason = 0;
 
@@ -4029,17 +4019,17 @@ CookieStatus nsCookieService::CheckPrefs
       *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
       return STATUS_REJECTED;
     }
 
     if (StaticPrefs::network_cookie_thirdparty_sessionOnly()) {
       return STATUS_ACCEPT_SESSION;
     }
 
-    if (aThirdPartyNonsecureSession) {
+    if (StaticPrefs::network_cookie_thirdparty_nonsecureSessionOnly()) {
       bool isHTTPS = false;
       aHostURI->SchemeIs("https", &isHTTPS);
       if (!isHTTPS) return STATUS_ACCEPT_SESSION;
     }
   }
 
   // if nothing has complained, accept cookie
   return STATUS_ACCEPTED;
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -208,21 +208,20 @@ class nsCookieService final : public nsI
   static bool CanSetCookie(nsIURI* aHostURI, const nsCookieKey& aKey,
                            mozilla::net::CookieStruct& aCookieData,
                            bool aRequireHostMatch, CookieStatus aStatus,
                            nsDependentCString& aCookieHeader,
                            int64_t aServerTime, bool aFromHttp,
                            nsIChannel* aChannel, bool& aSetCookie,
                            mozIThirdPartyUtil* aThirdPartyUtil);
   static CookieStatus CheckPrefs(
-      nsICookieSettings* aCookieSettings, bool aThirdPartyNonsecureSession,
-      nsIURI* aHostURI, bool aIsForeign, bool aIsTrackingResource,
-      bool aIsFirstPartyStorageAccessGranted, const char* aCookieHeader,
-      const int aNumOfCookies, const OriginAttributes& aOriginAttrs,
-      uint32_t* aRejectedReason);
+      nsICookieSettings* aCookieSettings, nsIURI* aHostURI, bool aIsForeign,
+      bool aIsTrackingResource, bool aIsFirstPartyStorageAccessGranted,
+      const char* aCookieHeader, const int aNumOfCookies,
+      const OriginAttributes& aOriginAttrs, uint32_t* aRejectedReason);
   static int64_t ParseServerTime(const nsCString& aServerTime);
 
   static already_AddRefed<nsICookieSettings> GetCookieSettings(
       nsIChannel* aChannel);
 
   void GetCookiesForURI(nsIURI* aHostURI, nsIChannel* aChannel, bool aIsForeign,
                         bool aIsTrackingResource,
                         bool aFirstPartyStorageAccessGranted,
@@ -363,17 +362,16 @@ class nsCookieService final : public nsI
   // private browsing, switching between them on a per-cookie-request basis.
   // this state encapsulates both the in-memory table and the on-disk DB.
   // note that the private states' dbConn should always be null - we never
   // want to be dealing with the on-disk DB when in private browsing.
   DBState* mDBState;
   RefPtr<DBState> mDefaultDBState;
   RefPtr<DBState> mPrivateDBState;
 
-  bool mThirdPartyNonsecureSession;
   uint16_t mMaxNumberOfCookies;
   uint16_t mMaxCookiesPerHost;
   uint16_t mCookieQuotaPerHost;
   int64_t mCookiePurgeAge;
 
   // thread
   nsCOMPtr<nsIThread> mThread;
   mozilla::Monitor mMonitor;