Bug 1416987 - Force an update when safebrowsing tables are changed in preference. r?francois draft
authorDimiL <dlee@mozilla.com>
Thu, 16 Nov 2017 10:19:03 +0800
changeset 698740 1d01310d77aa79b22efb28c2f9a85f3dbf7ca5cb
parent 697453 e1d7427787f7a26983c92ea1a1ac99eb863edd6c
child 740429 b9a7696caf92161b3388a72d45cf0b0e0fe8de41
push id89345
push userbmo:dlee@mozilla.com
push dateThu, 16 Nov 2017 02:28:25 +0000
reviewersfrancois
bugs1416987
milestone59.0a1
Bug 1416987 - Force an update when safebrowsing tables are changed in preference. r?francois This patch adds a |ForceUpdates| API in listmanager so when we update safebrwsing tables, we can use this API to force an update to ensure new tables are downloaded immediately. If the update fail for any reason (Server is down for example), then the new tables will have to wait until next update time. MozReview-Commit-ID: Kw2buGLWsZJ
browser/components/preferences/blocklists.js
browser/components/preferences/in-content/privacy.js
toolkit/components/url-classifier/nsIUrlListManager.idl
toolkit/components/url-classifier/nsUrlClassifierListManager.js
toolkit/components/url-classifier/tests/mochitest/test_threathit_report.html
toolkit/content/aboutUrlClassifier.js
--- a/browser/components/preferences/blocklists.js
+++ b/browser/components/preferences/blocklists.js
@@ -3,17 +3,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 Components.utils.import("resource://gre/modules/Services.jsm");
 const BASE_LIST_ID = "base";
 const CONTENT_LIST_ID = "content";
 const TRACK_SUFFIX = "-track-digest256";
 const TRACKING_TABLE_PREF = "urlclassifier.trackingTable";
 const LISTS_PREF_BRANCH = "browser.safebrowsing.provider.mozilla.lists.";
-const UPDATE_TIME_PREF = "browser.safebrowsing.provider.mozilla.nextupdatetime";
 
 var gBlocklistManager = {
   _type: "",
   _blockLists: [],
   _brandShortName: null,
   _bundle: null,
   _tree: null,
 
@@ -114,17 +113,23 @@ var gBlocklistManager = {
     if (activeList !== selected.id) {
       let trackingTable = Services.prefs.getCharPref(TRACKING_TABLE_PREF);
       if (selected.id != CONTENT_LIST_ID) {
         trackingTable = trackingTable.replace("," + CONTENT_LIST_ID + TRACK_SUFFIX, "");
       } else {
         trackingTable += "," + CONTENT_LIST_ID + TRACK_SUFFIX;
       }
       Services.prefs.setCharPref(TRACKING_TABLE_PREF, trackingTable);
-      Services.prefs.setCharPref(UPDATE_TIME_PREF, 42);
+
+      // Force an update after changing the tracking protection table.
+      let listmanager = Components.classes["@mozilla.org/url-classifier/listmanager;1"]
+                        .getService(Components.interfaces.nsIUrlListManager);
+      if (listmanager) {
+        listmanager.forceUpdates(trackingTable);
+      }
     }
 
     window.close();
   },
 
   _loadBlockLists() {
     this._blockLists = [];
 
--- a/browser/components/preferences/in-content/privacy.js
+++ b/browser/components/preferences/in-content/privacy.js
@@ -1110,16 +1110,23 @@ var gPrivacyPane = {
 
         malware.push("test-unwanted-simple");
       }
 
       // sort alphabetically to keep the pref consistent
       malware.sort();
 
       malwareTable.value = malware.join(",");
+
+      // Force an update after changing the malware table.
+      let listmanager = Components.classes["@mozilla.org/url-classifier/listmanager;1"]
+                        .getService(Components.interfaces.nsIUrlListManager);
+      if (listmanager) {
+        listmanager.forceUpdates(malwareTable.value);
+      }
     });
 
     // set initial values
 
     enableSafeBrowsing.checked = safeBrowsingPhishingPref.value && safeBrowsingMalwarePref.value;
     if (!enableSafeBrowsing.checked) {
       if (blockDownloads) {
         blockDownloads.setAttribute("disabled", "true");
--- a/toolkit/components/url-classifier/nsIUrlListManager.idl
+++ b/toolkit/components/url-classifier/nsIUrlListManager.idl
@@ -56,20 +56,22 @@ interface nsIUrlListManager : nsISupport
     void disableUpdate(in ACString tableName);
 
     /**
      * Toggle update checking, if necessary.
      */
     void maybeToggleUpdateChecking();
 
     /**
-     * This is currently used by about:url-classifier to force an update
-     * for the update url. Update may still fail because of backoff algorithm.
+     * Force updates for the given tables, updates are still restricted to
+     * backoff algorithm.
+     * @param tables  A string lists all the tables that we want to trigger updates.
+     *                table names are separated with ','.
      */
-    boolean checkForUpdates(in ACString updateUrl);
+    void forceUpdates(in ACString tableNames);
 
     /**
      * This is currently used by about:url-classifier to get back-off time
      * (in millisecond since epoch) for the given provider. Return 0 if we
      * are not in back-off mode.
      */
     uint64_t getBackOffTime(in ACString provider);
 };
--- a/toolkit/components/url-classifier/nsUrlClassifierListManager.js
+++ b/toolkit/components/url-classifier/nsUrlClassifierListManager.js
@@ -325,16 +325,42 @@ PROT_ListManager.prototype.maybeToggleUp
     }
   } else {
     log("Stopping managing lists (if currently active)");
     this.stopUpdateCheckers(); // Cancel pending updates
   }
 };
 
 /**
+ * Force updates for the given tables.
+ */
+PROT_ListManager.prototype.forceUpdates = function(tables) {
+  log("forceUpdates with " + tables);
+  if (!tables) {
+    return;
+  }
+
+  var updateUrls = new Set();
+  tables.split(",").forEach((table) => {
+    if (this.tablesData[table]) {
+      updateUrls.add(this.tablesData[table].updateUrl);
+    }
+  });
+
+  updateUrls.forEach((url) => {
+    // Cancel current update timer for the url because we are forcing an update.
+    this.updateCheckers_[url].cancel();
+    this.updateCheckers_[url] = null;
+
+    // Trigger an update for the given url.
+    this.checkForUpdates(url);
+  });
+}
+
+/**
  * Updates our internal tables from the update server
  *
  * @param updateUrl: request updates for tables associated with that url, or
  * for all tables if the url is empty.
  */
 PROT_ListManager.prototype.checkForUpdates = function(updateUrl) {
   log("checkForUpdates with " + updateUrl);
   // See if we've triggered the request backoff logic.
--- a/toolkit/components/url-classifier/tests/mochitest/test_threathit_report.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_threathit_report.html
@@ -132,17 +132,17 @@ function checkResults(aTestdata, aExpect
   let url = "http://" + SJS + "?" + params.toString();
 
   xhr.open("GET", url, true);
   xhr.setRequestHeader("Content-Type", "text/plain");
   xhr.send();
 }
 
 function waitForUpdate(data) {
-  listmanager.checkForUpdates(data.updateUrl);
+  listmanager.forceUpdates(data.list);
   return new Promise(resolve => {
     Services.obs.addObserver(function observer(aSubject, aTopic) {
       Services.obs.removeObserver(observer, aTopic);
       resolve();
     }, "safebrowsing-update-finished");
   });
 }
 
--- a/toolkit/content/aboutUrlClassifier.js
+++ b/toolkit/content/aboutUrlClassifier.js
@@ -169,21 +169,19 @@ var Provider = {
   },
 
   // Call update for the provider.
   update(provider) {
     let listmanager = Cc["@mozilla.org/url-classifier/listmanager;1"]
                       .getService(Ci.nsIUrlListManager);
 
     let pref = "browser.safebrowsing.provider." + provider + ".lists";
-    let tables = Services.prefs.getCharPref(pref, "").split(",");
-    let table = tables.find(t => listmanager.getUpdateUrl(t) != "");
+    let tables = Services.prefs.getCharPref(pref, "");
 
-    let updateUrl = listmanager.getUpdateUrl(table);
-    if (!listmanager.checkForUpdates(updateUrl)) {
+    if (!listmanager.forceUpdates(tables)) {
       // This may because of back-off algorithm.
       let elem = document.getElementById(provider + "-col-lastupdateresult");
       elem.childNodes[0].nodeValue = bundle.GetStringFromName("CannotUpdate");
     }
   },
 
 };