Bug 591447 - Cookie rowids may collide if PR_Now() winds backward. Part 3: test improvements. r=sdwilsh, a=betaN+

extensions/cookie/test/unit/test_eviction.js

@@ -2,16 +2,28 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
-function run_test()
+let test_generator = do_run_test();
+
+function run_test() {
+  do_test_pending();
+  test_generator.next();
+}
+
+function finish_test() {
+  do_execute_soon(function() {
+    test_generator.close();
+    do_test_finished();
+  });
+}
+
+function do_run_test()
 {
-  var cs = Cc["@mozilla.org/cookieService;1"].getService(Ci.nsICookieService);
-  var cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
-  var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
-  var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+  // Set up a profile.
+  let profile = do_get_profile();
 
   // twiddle prefs to convenient values for this test
-  prefs.setIntPref("network.cookie.purgeAge", 1);
-  prefs.setIntPref("network.cookie.maxNumber", 1000);
+  Services.prefs.setIntPref("network.cookie.purgeAge", 1);
+  Services.prefs.setIntPref("network.cookie.maxNumber", 1000);
 
   // eviction is performed based on two limits: when the total number of cookies
   // exceeds maxNumber + 10% (1100), and when cookies are older than purgeAge
@@ -21,50 +33,78 @@ function run_test()
   // we test the following cases of eviction:
   // 1) excess and age are satisfied, but only some of the excess are old enough
   // to be purged.
-  do_check_eq(testEviction(cm, 1101, 2, 50, 1051), 1051);
+  force_eviction(1101, 50);
+
+  // Fake a profile change, to ensure eviction affects the database correctly.
+  do_close_profile(test_generator);
+  yield;
+  do_load_profile();
+
+  do_check_true(check_remaining_cookies(1101, 50, 1051));
 
   // 2) excess and age are satisfied, and all of the excess are old enough
   // to be purged.
-  do_check_eq(testEviction(cm, 1101, 2, 100, 1001), 1001);
+  force_eviction(1101, 100);
+  do_close_profile(test_generator);
+  yield;
+  do_load_profile();
+  do_check_true(check_remaining_cookies(1101, 100, 1001));
 
   // 3) excess and age are satisfied, and more than the excess are old enough
   // to be purged.
-  do_check_eq(testEviction(cm, 1101, 2, 500, 1001), 1001);
+  force_eviction(1101, 500);
+  do_close_profile(test_generator);
+  yield;
+  do_load_profile();
+  do_check_true(check_remaining_cookies(1101, 500, 1001));
 
   // 4) excess but not age are satisfied.
-  do_check_eq(testEviction(cm, 2000, 0, 0, 2000), 2000);
+  force_eviction(2000, 0);
+  do_close_profile(test_generator);
+  yield;
+  do_load_profile();
+  do_check_true(check_remaining_cookies(2000, 0, 2000));
 
   // 5) age but not excess are satisfied.
-  do_check_eq(testEviction(cm, 1100, 2, 200, 1100), 1100);
+  force_eviction(1100, 200);
+  do_close_profile(test_generator);
+  yield;
+  do_load_profile();
+  do_check_true(check_remaining_cookies(1100, 200, 1100));
 
-  cm.removeAll();
-
-  // reset prefs to defaults
-  prefs.setIntPref("network.cookie.purgeAge", 30 * 24 * 60 * 60);
-  prefs.setIntPref("network.cookie.maxNumber", 2000);
+  finish_test();
 }
 
-// test that cookies are evicted by order of lastAccessed time, if both the limit
-// on total cookies (maxNumber + 10%) and the purge age are exceeded
+// Set 'aNumberTotal' cookies, ensuring that the first 'aNumberOld' cookies
+// will be measurably older than the rest.
 function
-testEviction(aCM, aNumberTotal, aSleepDuration, aNumberOld, aNumberToExpect)
+force_eviction(aNumberTotal, aNumberOld)
 {
-  aCM.removeAll();
+  Services.cookiemgr.removeAll();
   var expiry = (Date.now() + 1e6) * 1000;
 
   var i;
   for (i = 0; i < aNumberTotal; ++i) {
     var host = "eviction." + i + ".tests";
-    aCM.add(host, "", "test", "eviction", false, false, false, expiry);
+    Services.cookiemgr.add(host, "", "test", "eviction", false, false, false,
+      expiry);
 
-    if ((i == aNumberOld - 1) && aSleepDuration) {
-      // sleep a while, to make sure the first batch of cookies is older than
-      // the second (timer resolution varies on different platforms).
-      sleep(aSleepDuration * 1000);
+    if (i == aNumberOld - 1) {
+      // Sleep a while, to make sure the first batch of cookies is older than
+      // the second (timer resolution varies on different platforms). This
+      // delay must be measurably greater than the eviction age threshold.
+      sleep(1100);
     }
   }
+}
 
-  var enumerator = aCM.enumerator;
+// Test that 'aNumberToExpect' cookies remain after purging is complete, and
+// that the cookies that remain consist of the set expected given the number of
+// of older and newer cookies -- eviction should occur by order of lastAccessed
+// time, if both the limit on total cookies (maxNumber + 10%) and the purge age
+// are exceeded.
+function check_remaining_cookies(aNumberTotal, aNumberOld, aNumberToExpect) {
+  var enumerator = Services.cookiemgr.enumerator;
 
   i = 0;
   while (enumerator.hasMoreElements()) {
@@ -78,7 +118,7 @@ testEviction(aCM, aNumberTotal, aSleepDuration, aNumberOld, aNumberToExpect)
     }
   }
 
-  return i;
+  return i == aNumberToExpect;
 }
 
 // delay for a number of milliseconds

netwerk/cookie/nsCookieService.cpp

@@ -3029,7 +3029,7 @@ nsCookieService::PurgeCookies(PRInt64 aCurrentTimeInUsec)
 #ifdef PR_LOGGING
   PRUint32 initialCookieCount = mDBState->cookieCount;
   COOKIE_LOGSTRING(PR_LOG_DEBUG,
-    ("PurgeCookies(): beginning purge with %ld cookies and %lld age",
+    ("PurgeCookies(): beginning purge with %ld cookies and %lld oldest age",
      mDBState->cookieCount, aCurrentTimeInUsec - mDBState->cookieOldestTime));
 #endif
 
@@ -3077,7 +3077,7 @@ nsCookieService::PurgeCookies(PRInt64 aCurrentTimeInUsec)
   for (nsPurgeData::ArrayType::index_type i = purgeList.Length(); i--; ) {
     nsCookie *cookie = purgeList[i].Cookie();
     removedList->AppendElement(cookie, PR_FALSE);
-    COOKIE_LOGEVICTED(cookie, "Cookie expired or too old");
+    COOKIE_LOGEVICTED(cookie, "Cookie too old");
 
     RemoveCookieFromList(purgeList[i], paramsArray);
   }
@@ -3104,7 +3104,7 @@ nsCookieService::PurgeCookies(PRInt64 aCurrentTimeInUsec)
   COOKIE_LOGSTRING(PR_LOG_DEBUG,
     ("PurgeCookies(): %ld expired; %ld purged; %ld remain; %lld oldest age",
      initialCookieCount - postExpiryCookieCount,
-     mDBState->cookieCount - postExpiryCookieCount,
+     postExpiryCookieCount - mDBState->cookieCount,
      mDBState->cookieCount,
      aCurrentTimeInUsec - mDBState->cookieOldestTime));
 }