Bug 1365877 - resetDatabase in SafeBrowsing test case should also reset cache. r?francois draft
authorDimiL <dlee@mozilla.com>
Thu, 18 May 2017 17:21:00 +0800
changeset 582340 efd4923a1d719243f01d8e5b6a4bea149c060573
parent 580040 baf05f61bc14fdf45511bc1165ce76daa08c5c0f
child 629735 7fc9fbb4d63ddb83fba750c508c6e11c146df2f6
push id60045
push userbmo:dlee@mozilla.com
push dateMon, 22 May 2017 09:02:49 +0000
reviewersfrancois
bugs1365877, 1333328
milestone55.0a1
Bug 1365877 - resetDatabase in SafeBrowsing test case should also reset cache. r?francois resetDatabase() is used to clear out the Safe Browsing database and cache in tests. Since bug 1333328 however we can no longer rely on updates clearing the cache. There are two solutions to address this issue: 1. resetDatabase() calls another test-only function: reloadDatabase(). Since the cache is in memory, reloading the URL classifier will automatically clear the cache. 2. During an update, remove cache entries which are not in the database. I prefer taking the first one because if we implement the second approach, an update will take longer since we need to check if each prefix in the cache can also be found in the database. I think this is not necessary because prefixes not in the database will eventually be removed when they are expired. MozReview-Commit-ID: BjsDKDMr205
toolkit/components/url-classifier/tests/browser/classifierHelper.js
toolkit/components/url-classifier/tests/mochitest/classifierHelper.js
toolkit/components/url-classifier/tests/mochitest/test_bug1254766.html
toolkit/components/url-classifier/tests/mochitest/test_cachemiss.html
toolkit/components/url-classifier/tests/mochitest/test_classifier.html
toolkit/components/url-classifier/tests/mochitest/test_gethash.html
--- a/toolkit/components/url-classifier/tests/browser/classifierHelper.js
+++ b/toolkit/components/url-classifier/tests/browser/classifierHelper.js
@@ -138,17 +138,17 @@ classifierHelper.removeUrlFromDB = funct
     });
 
   return classifierHelper._update(testUpdate);
 };
 
 // This API is used to expire all add/sub chunks we have updated
 // by using addUrlToDB and removeUrlFromDB.
 // Returns a Promise.
-classifierHelper.resetDB = function() {
+classifierHelper.resetDatabase = function() {
   var testUpdate = "";
   for (var update of classifierHelper._updatesToCleanup) {
     if (testUpdate.includes(update.db))
       continue;
 
     testUpdate +=
       "n:1000\n" +
       "i:" + update.db + "\n" +
@@ -209,12 +209,12 @@ classifierHelper._cleanup = function() {
   for (var pref in PREFS) {
     Services.prefs.clearUserPref(pref);
   }
 
   if (!classifierHelper._updatesToCleanup) {
     return Promise.resolve();
   }
 
-  return classifierHelper.resetDB();
+  return classifierHelper.resetDatabase();
 };
 // Cleanup will be called at end of each testcase to remove all the urls added to database.
 registerCleanupFunction(classifierHelper._cleanup);
--- a/toolkit/components/url-classifier/tests/mochitest/classifierHelper.js
+++ b/toolkit/components/url-classifier/tests/mochitest/classifierHelper.js
@@ -103,32 +103,40 @@ classifierHelper.removeUrlFromDB = funct
       });
 
     classifierHelper._update(testUpdate, resolve, reject);
   });
 };
 
 // This API is used to expire all add/sub chunks we have updated
 // by using addUrlToDB and removeUrlFromDB.
-classifierHelper.resetDB = function() {
-  return new Promise(function(resolve, reject) {
-    var testUpdate = "";
-    for (var update of classifierHelper._updatesToCleanup) {
-      if (testUpdate.includes(update.db))
-        continue;
+classifierHelper.resetDatabase = function() {
+  function removeDatabase() {
+    return new Promise(function(resolve, reject) {
+      var testUpdate = "";
+      for (var update of classifierHelper._updatesToCleanup) {
+        if (testUpdate.includes(update.db))
+          continue;
 
-      testUpdate +=
-        "n:1000\n" +
-        "i:" + update.db + "\n" +
-        "ad:" + ADD_CHUNKNUM + "\n" +
-        "sd:" + SUB_CHUNKNUM + "\n"
-    }
+        testUpdate +=
+          "n:1000\n" +
+          "i:" + update.db + "\n" +
+          "ad:" + ADD_CHUNKNUM + "\n" +
+          "sd:" + SUB_CHUNKNUM + "\n"
+      }
 
-    classifierHelper._update(testUpdate, resolve, reject);
-  });
+      classifierHelper._update(testUpdate, resolve, reject);
+    });
+  }
+
+  // Remove and then reload will ensure both database and cache will
+  // be cleared.
+  return Promise.resolve()
+    .then(removeDatabase)
+    .then(classifierHelper.reloadDatabase);
 };
 
 classifierHelper.reloadDatabase = function() {
   return new Promise(function(resolve, reject) {
     gScript.addMessageListener("reloadSuccess", function handler() {
       gScript.removeMessageListener('reloadSuccess', handler);
       resolve();
     });
@@ -190,12 +198,12 @@ classifierHelper._cleanup = function() {
   for (var pref in PREFS) {
     SpecialPowers.clearUserPref(pref);
   }
 
   if (!classifierHelper._updatesToCleanup) {
     return Promise.resolve();
   }
 
-  return classifierHelper.resetDB();
+  return classifierHelper.resetDatabase();
 };
 
 classifierHelper._setup();
--- a/toolkit/components/url-classifier/tests/mochitest/test_bug1254766.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_bug1254766.html
@@ -75,17 +75,17 @@ function setup() {
     addCompletionToServer(MALWARE_LIST, MALWARE_HOST2, GETHASH_URL),
     addCompletionToServer(UNWANTED_LIST, UNWANTED_HOST1, GETHASH_URL),
     addCompletionToServer(UNWANTED_LIST, UNWANTED_HOST2, GETHASH_URL),
   ]);
 }
 
 // Reset function in helper try to simulate the behavior we restart firefox
 function reset() {
-  return classifierHelper.resetDB()
+  return classifierHelper.resetDatabase()
     .catch(err => {
       ok(false, "Couldn't update classifier. Error code: " + errorCode);
       // Abort test.
       SimpleTest.finish();
     });
 }
 
 function updateUnusedUrl() {
--- a/toolkit/components/url-classifier/tests/mochitest/test_cachemiss.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_cachemiss.html
@@ -68,17 +68,17 @@ function addPrefixToDB(list, url) {
       ok(false, "Couldn't update classifier. Error code: " + err);
       // Abort test.
       SimpleTest.finish();
     });
 }
 
 // manually reset DB to make sure next test won't be affected by cache.
 function reset() {
-  return classifierHelper.resetDB;
+  return classifierHelper.resetDatabase();
 }
 
 // This test has to come before testPositiveCache to ensure gethash server doesn't
 // contain completions.
 function testNegativeCache() {
   shouldLoad = true;
 
   function setup() {
--- a/toolkit/components/url-classifier/tests/mochitest/test_classifier.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_classifier.html
@@ -13,17 +13,17 @@
 <pre id="test">
 
 <script class="testbody" type="text/javascript">
 
 var firstLoad = true;
 
 // Add some URLs to the malware database.
 // Note that we intentionally don't touch test-phish-simple, and will
-// use the URL registered by addMozEntries().  Otherwise, classifierHelper.resetDB()
+// use the URL registered by addMozEntries().  Otherwise, classifierHelper.resetDatabase()
 // will reset this table, and waitForInit() can't find the known phishing
 // URL in test-phish-simple, breaking the tests following this one.
 var testData = [
   { url: "malware.example.com/",
     db: "test-malware-simple"
   },
   { url: "unwanted.example.com/",
     db: "test-unwanted-simple"
--- a/toolkit/components/url-classifier/tests/mochitest/test_gethash.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_gethash.html
@@ -76,17 +76,17 @@ function setup() {
     addPrefixToDB(UNWANTED_LIST, UNWANTED_HOST),
     addCompletionToServer(MALWARE_LIST, MALWARE_HOST, GETHASH_URL),
     addCompletionToServer(UNWANTED_LIST, UNWANTED_HOST, GETHASH_URL),
   ]);
 }
 
 // manually reset DB to make sure next test won't be affected by cache.
 function reset() {
-  return classifierHelper.resetDB;
+  return classifierHelper.resetDatabase();
 }
 
 function runTest() {
   Promise.resolve()
     // This test resources get blocked when gethash returns successfully
     .then(classifierHelper.waitForInit)
     .then(setup)
     .then(() => loadTestFrame("testFrame1"))