Bug 1395411 - Unregister tables when they're removed from urlclassifier.*Table. r=francois

MozReview-Commit-ID: Ex1ZxMcJLep

--HG--
extra : rebase_source : a200c02b536874a5a4255087b42d7074f5de3b43

toolkit/components/url-classifier/SafeBrowsing.jsm

@@ -130,6 +130,17 @@ this.SafeBrowsing = {
     }
   },
 
+  unregisterTables(obsoleteLists) {
+    let listManager = Cc["@mozilla.org/url-classifier/listmanager;1"].
+      getService(Ci.nsIUrlListManager);
+
+    for (let i = 0; i < obsoleteLists.length; ++i) {
+      for (let j = 0; j < obsoleteLists[i].length; ++j) {
+        listManager.unregisterTable(obsoleteLists[i][j]);
+      }
+    }
+  },
+
 
   initialized:          false,
   phishingEnabled:      false,
@@ -225,6 +236,21 @@ this.SafeBrowsing = {
         flashExceptTable, flashSubDocTable,
         flashSubDocExceptTable;
 
+    let obsoleteLists;
+    // Make a copy of the original lists before we re-read the prefs.
+    if (this.initialized) {
+      obsoleteLists = [this.phishingLists,
+                       this.malwareLists,
+                       this.downloadBlockLists,
+                       this.downloadAllowLists,
+                       this.passwordAllowLists,
+                       this.trackingProtectionLists,
+                       this.trackingProtectionWhitelists,
+                       this.blockedLists,
+                       this.flashLists,
+                       this.flashInfobarLists];
+    }
+
     [this.phishingLists,
      this.malwareLists,
      this.downloadBlockLists,
@@ -247,8 +273,29 @@ this.SafeBrowsing = {
                                              flashSubDocTable,
                                              flashSubDocExceptTable)
 
+    if (obsoleteLists) {
+      let newLists = [this.phishingLists,
+                      this.malwareLists,
+                      this.downloadBlockLists,
+                      this.downloadAllowLists,
+                      this.passwordAllowLists,
+                      this.trackingProtectionLists,
+                      this.trackingProtectionWhitelists,
+                      this.blockedLists,
+                      this.flashLists,
+                      this.flashInfobarLists];
+
+      for (let i = 0; i < obsoleteLists.length; ++i) {
+        obsoleteLists[i] = obsoleteLists[i]
+          .filter(list => newLists[i].indexOf(list) == -1);
+      }
+    }
+
     this.updateProviderURLs();
     this.registerTables();
+    if (obsoleteLists) {
+      this.unregisterTables(obsoleteLists);
+    }
 
     // XXX The listManager backend gets confused if this is called before the
     // lists are registered. So only call it here when a pref changes, and not

toolkit/components/url-classifier/nsIUrlListManager.idl

@@ -39,6 +39,11 @@ interface nsIUrlListManager : nsISupports
                           in ACString updateUrl,
                           in ACString gethashUrl);
 
+    /**
+     * Unregister table from the list
+     */
+    void unregisterTable(in ACString tableName);
+
     /**
      * Turn on update checking for a table. I.e., during the next server
      * check, download updates for this table.

toolkit/components/url-classifier/nsUrlClassifierListManager.js

@@ -108,6 +108,24 @@ PROT_ListManager.prototype.registerTable = function(tableName,
   return true;
 }
 
+/**
+ * Unregister a table table from list
+ */
+PROT_ListManager.prototype.unregisterTable = function(tableName) {
+  log("unregistering " + tableName);
+  var table = this.tablesData[tableName];
+  if (table) {
+    if (!this.updatesNeeded_(table.updateUrl) &&
+        this.updateCheckers_[table.updateUrl]) {
+      this.updateCheckers_[table.updateUrl].cancel();
+      this.updateCheckers_[table.updateUrl] = null;
+    }
+    delete this.needsUpdate_[table.updateUrl][tableName];
+  }
+  delete this.tablesData[tableName];
+
+}
+
 /**
  * Delete all of our data tables which seem to leak otherwise.
  * Remove observers

toolkit/components/url-classifier/tests/mochitest/chrome.ini

@@ -27,6 +27,7 @@ tags = trackingprotection
 [test_safebrowsing_bug1272239.html]
 [test_donottrack.html]
 [test_classifier_changetablepref.html]
+[test_classifier_changetablepref_bug1395411.html]
 [test_reporturl.html]
 [test_trackingprotection_bug1312515.html]
 [test_advisory_link.html]

toolkit/components/url-classifier/tests/mochitest/test_classifier_changetablepref_bug1395411.html

@@ -0,0 +1,74 @@
+<!DOCTYPE HTML>
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<head>
+  <title>Bug 1395411 - Changing the urlclassifier.*Table prefs doesn't remove them from the update checker.</title>
+  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="classifierHelper.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+</head>
+
+<body>
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+
+<script class="testbody" type="text/javascript">
+/* import-globals-from classifierHelper.js */
+var Cc = SpecialPowers.Cc;
+var Ci = SpecialPowers.Ci;
+
+const testTableV2 = "mochi1-phish-simple";
+const testTableV4 = "mochi1-phish-proto";
+const UPDATE_URL_V2 = "http://mochi.test:8888/tests/toolkit/components/url-classifier/dummyV2";
+const UPDATE_URL_V4 = "http://mochi.test:8888/tests/toolkit/components/url-classifier/dummyV4";
+
+let listmanager = Cc["@mozilla.org/url-classifier/listmanager;1"].
+                    getService(Ci.nsIUrlListManager);
+
+let pushPrefs = (...p) => SpecialPowers.pushPrefEnv({set: p});
+
+SpecialPowers.pushPrefEnv(
+  {"set": [["browser.safebrowsing.phishing.enabled", true],
+           ["browser.safebrowsing.provider.mozilla.lists", testTableV2],
+           ["browser.safebrowsing.provider.mozilla4.lists", testTableV4],
+           ["browser.safebrowsing.provider.mozilla4.updateURL", UPDATE_URL_V4],
+           ["browser.safebrowsing.provider.mozilla.updateURL", UPDATE_URL_V2]]},
+  runTest);
+
+function runTest() {
+  (async function() {
+    await classifierHelper.waitForInit();
+
+    await pushPrefs(["urlclassifier.phishTable", testTableV2 + "," + testTableV4]);
+    is(listmanager.getUpdateUrl(testTableV4), UPDATE_URL_V4, "Correct update url v4");
+    is(listmanager.getUpdateUrl(testTableV2), UPDATE_URL_V2, "Correct update url v2");
+
+    await pushPrefs(["urlclassifier.phishTable", testTableV2]);
+    is(listmanager.getUpdateUrl(testTableV4), "", "Correct empty update url v4");
+    is(listmanager.getUpdateUrl(testTableV2), UPDATE_URL_V2, "Correct update url v2");
+
+    await pushPrefs(["urlclassifier.phishTable", testTableV4]);
+    is(listmanager.getUpdateUrl(testTableV4), UPDATE_URL_V4, "Correct update url v4");
+    is(listmanager.getUpdateUrl(testTableV2), "", "Correct empty update url v2");
+
+    await pushPrefs(["urlclassifier.phishTable", ""]);
+    is(listmanager.getUpdateUrl(testTableV4), "", "Correct empty update url v4");
+    is(listmanager.getUpdateUrl(testTableV2), "", "Correct empty update url v2");
+
+    await classifierHelper._cleanup();
+
+    SimpleTest.finish();
+  })();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+<iframe id="testFrame" width="100%" height="100%" onload=""></iframe>
+</body>
+</html>
+