Bug 1319403 - Modified nsCookieService::FindStaleCookie() and test cases, r=jdm
authorAmy Chung <amchung@mozilla.com>
Mon, 27 Feb 2017 18:31:33 +0800
changeset 394026 b780bfdac971fcba73575f454a45d042cf2ed14e
parent 394025 dd7207e80b803095d868985743134498d512f776
child 394027 e6332dc0a1837295e549629bf5f2ab1a50cba00b
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs1319403
milestone54.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 1319403 - Modified nsCookieService::FindStaleCookie() and test cases, r=jdm
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/test/unit/test_eviction.js
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -4619,27 +4619,19 @@ nsCookieService::FindStaleCookie(nsCooki
   if (aSource) {
     GetBaseDomain(aSource, baseDomain, requireHostMatch);
     aSource->GetAsciiHost(sourceHost);
     sourcePath = GetPathFromURI(aSource);
   }
 
   const nsCookieEntry::ArrayType &cookies = aEntry->GetCookies();
 
-  int64_t oldestNonMatchingSessionCookieTime = 0;
-  nsListIter oldestNonMatchingSessionCookie;
-  oldestNonMatchingSessionCookie.entry = nullptr;
-
-  int64_t oldestSessionCookieTime = 0;
-  nsListIter oldestSessionCookie;
-  oldestSessionCookie.entry = nullptr;
-
-  int64_t oldestNonMatchingNonSessionCookieTime = 0;
-  nsListIter oldestNonMatchingNonSessionCookie;
-  oldestNonMatchingNonSessionCookie.entry = nullptr;
+  int64_t oldestNonMatchingCookieTime = 0;
+  nsListIter oldestNonMatchingCookie;
+  oldestNonMatchingCookie.entry = nullptr;
 
   int64_t oldestCookieTime = 0;
   nsListIter oldestCookie;
   oldestCookie.entry = nullptr;
 
   int64_t actualOldestCookieTime = cookies.Length() ? cookies[0]->LastAccessed() : 0;
   for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) {
     nsCookie *cookie = cookies[i];
@@ -4662,69 +4654,44 @@ nsCookieService::FindStaleCookie(nsCooki
     if (aIsSecure.isSome() && !aIsSecure.value()) {
       // We want to look for the oldest non-secure cookie first time through,
       // then find the oldest secure cookie the second time we are called.
       if (cookie->IsSecure()) {
         continue;
       }
     }
 
-    // Update our various records of oldest cookies fitting several restrictions:
-    // * session cookies
-    // * non-session cookies
-    // * cookies with paths and domains that don't match the cookie triggering this purge
-
     // This cookie is a candidate for eviction if we have no information about
     // the source request, or if it is not a path or domain match against the
     // source request.
     bool isPrimaryEvictionCandidate = true;
     if (aSource) {
       isPrimaryEvictionCandidate = !PathMatches(cookie, sourcePath) || !DomainMatches(cookie, sourceHost);
     }
 
-    if (cookie->IsSession()) {
-      if (!oldestSessionCookie.entry || oldestSessionCookieTime > lastAccessed) {
-        oldestSessionCookieTime = lastAccessed;
-        oldestSessionCookie.entry = aEntry;
-        oldestSessionCookie.index = i;
-      }
-
-      if (isPrimaryEvictionCandidate &&
-          (!oldestNonMatchingSessionCookie.entry ||
-           oldestNonMatchingSessionCookieTime > lastAccessed)) {
-        oldestNonMatchingSessionCookieTime = lastAccessed;
-        oldestNonMatchingSessionCookie.entry = aEntry;
-        oldestNonMatchingSessionCookie.index = i;
-      }
-    } else if (isPrimaryEvictionCandidate &&
-               (!oldestNonMatchingNonSessionCookie.entry ||
-                oldestNonMatchingNonSessionCookieTime > lastAccessed)) {
-      oldestNonMatchingNonSessionCookieTime = lastAccessed;
-      oldestNonMatchingNonSessionCookie.entry = aEntry;
-      oldestNonMatchingNonSessionCookie.index = i;
+    if (isPrimaryEvictionCandidate &&
+        (!oldestNonMatchingCookie.entry ||
+         oldestNonMatchingCookieTime > lastAccessed)) {
+      oldestNonMatchingCookieTime = lastAccessed;
+      oldestNonMatchingCookie.entry = aEntry;
+      oldestNonMatchingCookie.index = i;
     }
 
     // Check if we've found the oldest cookie so far.
     if (!oldestCookie.entry || oldestCookieTime > lastAccessed) {
       oldestCookieTime = lastAccessed;
       oldestCookie.entry = aEntry;
       oldestCookie.index = i;
     }
   }
 
-  // Prefer to evict the oldest session cookies with a non-matching path/domain,
-  // followed by the oldest session cookie with a matching path/domain,
-  // followed by the oldest non-session cookie with a non-matching path/domain,
-  // resorting to the oldest non-session cookie with a matching path/domain.
-  if (oldestNonMatchingSessionCookie.entry) {
-    aIter = oldestNonMatchingSessionCookie;
-  } else if (oldestSessionCookie.entry) {
-    aIter = oldestSessionCookie;
-  } else if (oldestNonMatchingNonSessionCookie.entry) {
-    aIter = oldestNonMatchingNonSessionCookie;
+  // Prefer to evict the oldest cookie with a non-matching path/domain,
+  // followed by the oldest matching cookie.
+  if (oldestNonMatchingCookie.entry) {
+    aIter = oldestNonMatchingCookie;
   } else {
     aIter = oldestCookie;
   }
 
   return actualOldestCookieTime;
 }
 
 void
--- a/netwerk/cookie/test/unit/test_eviction.js
+++ b/netwerk/cookie/test/unit/test_eviction.js
@@ -147,85 +147,82 @@ function* test_basic_eviction(base_host,
     // Ensure that cookies set for the / path appear more recent.
     cs.getCookieString(OTHER_SUBDOMAIN_URI, null)
     verifyCookies(['non_session_non_path_non_domain',
                    'session_foo_path',
                    'session_bar_path',
                    'non_session_non_path_subdomain',
                    'session_non_path_pub_domain'], BASE_URI);
 
-    // Evict oldest session cookie that does not match example.org/foo (session_bar_path)
+    // Evict oldest cookie that does not match example.org/foo (session_bar_path)
     yield setCookie("session_foo_path_2", null, "/foo", null, FOO_PATH);
     verifyCookies(['non_session_non_path_non_domain',
                    'session_foo_path',
                    'non_session_non_path_subdomain',
                    'session_non_path_pub_domain',
                    'session_foo_path_2'], BASE_URI);
 
-    // Evict oldest session cookie that does not match example.org/bar (session_foo_path)
+    // Evict oldest cookie that does not match example.org/bar (session_foo_path)
     yield setCookie("session_bar_path_2", null, "/bar", null, BAR_PATH);
     verifyCookies(['non_session_non_path_non_domain',
                    'non_session_non_path_subdomain',
                    'session_non_path_pub_domain',
                    'session_foo_path_2',
                    'session_bar_path_2'], BASE_URI);
 
-    // Evict oldest session cookie that does not match example.org/ (session_non_path_pub_domain)
+    // Evict oldest cookie that does not match example.org/ (session_non_path_pub_domain)
     yield setCookie("non_session_non_path_non_domain_2", null, null, 100000, BASE_URI);
     verifyCookies(['non_session_non_path_non_domain',
                    'non_session_non_path_subdomain',
                    'session_foo_path_2',
                    'session_bar_path_2',
                    'non_session_non_path_non_domain_2'], BASE_URI);
 
-    // Evict oldest session cookie that does not match example.org/ (session_foo_path_2)
+    // Evict oldest cookie that does not match example.org/ (session_foo_path_2)
     yield setCookie("session_non_path_non_domain_3", null, null, null, BASE_URI);
     verifyCookies(['non_session_non_path_non_domain',
                    'non_session_non_path_subdomain',
                    'session_bar_path_2',
                    'non_session_non_path_non_domain_2',
                    'session_non_path_non_domain_3'], BASE_URI);
 
-    // Evict oldest session cookie; all such cookies match example.org/bar (session_bar_path_2)
-    // note: this new cookie doesn't have an explicit path, but empty paths inherit the
-    // request's path
+    // Evict oldest cookie; all such cookies match example.org/bar (non_session_non_path_non_domain)
     yield setCookie("non_session_bar_path_non_domain", null, null, 100000, BAR_PATH);
-    verifyCookies(['non_session_non_path_non_domain',
-                   'non_session_non_path_subdomain',
+    verifyCookies(['non_session_non_path_subdomain',
+                   'session_bar_path_2',
                    'non_session_non_path_non_domain_2',
                    'session_non_path_non_domain_3',
                    'non_session_bar_path_non_domain'], BASE_URI);
 
-    // Evict oldest session cookie, even though it matches pub.example.org (session_non_path_non_domain_3)
+    // Evict oldest cookie that deose not match example.org/ (session_bar_path_2)
     yield setCookie("non_session_non_path_pub_domain", null, null, 100000, OTHER_SUBDOMAIN_URI);
-    verifyCookies(['non_session_non_path_non_domain',
-                   'non_session_non_path_subdomain',
+    verifyCookies(['non_session_non_path_subdomain',
                    'non_session_non_path_non_domain_2',
+                   'session_non_path_non_domain_3',
                    'non_session_bar_path_non_domain',
                    'non_session_non_path_pub_domain'], BASE_URI);
 
-    // All session cookies have been evicted.
-    // Evict oldest non-session non-domain-matching cookie (non_session_non_path_pub_domain)
+    // Evict oldest cookie that does not match example.org/bar (non_session_non_path_pub_domain)
     yield setCookie("non_session_bar_path_non_domain_2", null, '/bar', 100000, BAR_PATH);
-    verifyCookies(['non_session_non_path_non_domain',
-                   'non_session_non_path_subdomain',
+    verifyCookies(['non_session_non_path_subdomain',
                    'non_session_non_path_non_domain_2',
+                   'session_non_path_non_domain_3',
                    'non_session_bar_path_non_domain',
                    'non_session_bar_path_non_domain_2'], BASE_URI);
 
-    // Evict oldest non-session non-path-matching cookie (non_session_bar_path_non_domain)
+    // Evict oldest cookie that does not match example.org/ (non_session_bar_path_non_domain)
     yield setCookie("non_session_non_path_non_domain_4", null, null, 100000, BASE_URI);
-    verifyCookies(['non_session_non_path_non_domain',
-                   'non_session_non_path_subdomain',
+    verifyCookies(['non_session_non_path_subdomain',
                    'non_session_non_path_non_domain_2',
+                   'session_non_path_non_domain_3',
                    'non_session_bar_path_non_domain_2',
                    'non_session_non_path_non_domain_4'], BASE_URI);
 
-    // At this point all remaining cookies are non-session cookies, have a path of /,
-    // and either don't have a domain or have one that matches subdomains.
+    // At this point all remaining cookies have a path of / and either don't have a domain
+    // or have one that matches subdomains.
     // They will therefore be evicted from oldest to newest if all new cookies added share
     // similar characteristics.
 }
 
 // Verify that the given cookie names exist, and are ordered from least to most recently accessed
 function verifyCookies(names, uri) {
     do_check_eq(cm.countCookiesFromHost(uri.host), names.length);
     let cookies = cm.getCookiesFromHost(uri.host, {});