Bug 423960 ? regression: disabling history remembers visits (for mak77@supereva.it, r=dietrich)

toolkit/components/places/src/nsNavHistory.cpp

@@ -1786,7 +1786,8 @@ nsNavHistory::LoadPrefs(PRBool aInitializing)
   mPrefBranch->GetIntPref(PREF_BROWSER_HISTORY_EXPIRE_DAYS_MAX, &mExpireDaysMax);
   mPrefBranch->GetIntPref(PREF_BROWSER_HISTORY_EXPIRE_DAYS_MIN, &mExpireDaysMin);
   // Cap max days to min days to prevent expiring pages younger than min
-  if (mExpireDaysMax < mExpireDaysMin)
+  // NOTE: if history is disabled in preferences, then mExpireDaysMax == 0
+  if (mExpireDaysMax && mExpireDaysMax < mExpireDaysMin)
     mExpireDaysMax = mExpireDaysMin;
   if (NS_FAILED(mPrefBranch->GetIntPref(PREF_BROWSER_HISTORY_EXPIRE_SITES,
                                         &mExpireSites)))

toolkit/components/places/tests/unit/test_expiration.js

@@ -688,5 +688,101 @@ function checkExpireNeitherOver() {
     do_throw(ex);
   }
   dump("done incremental expiration test 4\n");
+  startExpireHistoryDisabled();
+}
+
+/*
+test 5: HISTORY DISABLED (HISTORY_EXPIRE_DAYS = 0), EXPIRE EVERYTHING
+
+1. special case when history is disabled, expire all visits
+
+steps:
+  - clear history
+  - reset observer
+  - add a visit to an url, w/ current date
+  - set browser.history_expire_days to 0
+  - kick off incremental expiration
+
+confirmation:
+  - check onPageExpired, confirm visit was expired
+  - query for the visit, confirm it's not there
+
+*/
+function startExpireHistoryDisabled() {
+  dump("startExpireHistoryDisabled()\n");
+  // setup
+  histsvc.removeAllPages();
+  observer.expiredURI = null;
+
+  // add data
+  histsvc.addVisit(testURI, Date.now() * 1000, null, histsvc.TRANSITION_TYPED, false, 0);
+  annosvc.setPageAnnotation(testURI, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_WITH_HISTORY);
+
+  // set date maximum to 0
+  prefs.setIntPref("browser.history_expire_days", 0);
+
+  // setup confirmation
+  do_timeout(3600, "checkExpireHistoryDisabled();"); // incremental expiration timer is 3500
+}
+
+function checkExpireHistoryDisabled() {
+  dump("checkExpireHistoryDisabled()\n");
+  try {
+    do_check_eq(observer.expiredURI, testURI.spec);
+    do_check_eq(annosvc.getPageAnnotationNames(testURI, {}).length, 0);
+  } catch(ex) {
+    do_throw(ex);
+  }
+  dump("done incremental expiration test 5\n");
+  startExpireBadPrefs();
+}
+
+/*
+test 6: BAD EXPIRATION PREFS (MAX < MIN)
+
+1. if max < min we force max = min to avoid deleting wrong visits
+
+steps:
+  - clear history
+  - reset observer
+  - add a visit to an url, 10 days ago
+  - set browser.history_expire_days to 1
+  - set browser.history_expire_days_min to 20
+  - kick off incremental expiration
+
+confirmation:
+  - check onPageExpired, confirm nothing was expired
+  - query for the visit, confirm it's there
+
+*/
+function startExpireBadPrefs() {
+  dump("startExpireBadPrefs()\n");
+  // setup
+  histsvc.removeAllPages();
+  observer.expiredURI = null;
+
+  // add data
+  var age = (Date.now() - (86400 * 10 * 1000)) * 1000;
+  histsvc.addVisit(testURI, age, null, histsvc.TRANSITION_TYPED, false, 0);
+  annosvc.setPageAnnotation(testURI, testAnnoName, testAnnoVal, 0, annosvc.EXPIRE_WITH_HISTORY);
+
+  // set date minimum to 20
+  prefs.setIntPref("browser.history_expire_days_min", 20);
+  // set date maximum to 1
+  prefs.setIntPref("browser.history_expire_days", 1);
+
+  // setup confirmation
+  do_timeout(3600, "checkExpireBadPrefs();"); // incremental expiration timer is 3500
+}
+
+function checkExpireBadPrefs() {
+  dump("checkExpireBadPrefs()\n");
+  try {
+    do_check_eq(observer.expiredURI, null);
+    do_check_eq(annosvc.getPageAnnotationNames(testURI, {}).length, 1);
+  } catch(ex) {
+    do_throw(ex);
+  }
+  dump("done incremental expiration test 6\n");
   do_test_finished();
 }