Issue 6599 - Detect data corruption of storage.local

lib/prefs.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 196 matching lines @@
  * @type {number}
  */
 defaults.last_updates_page_displayed = 0;
 
 /**
   * @namespace
   * @static
   */
 let Prefs = exports.Prefs = {
   /**
+   * Sets the given preference.
+   *
+   * @param {string} preference
+   * @param {any}    value
+   * @return {Promise} A promise that resolves when the underlying
+                       browser.storage.local.set/remove() operation completes
+   */
+  set(preference, value)
+  {
+    let defaultValue = defaults[preference];
+
+    if (typeof value != typeof defaultValue)
+      throw new Error("Attempt to change preference type");
+
+    if (value == defaultValue)
+    {
+      delete overrides[preference];
+      return browser.storage.local.remove(prefToKey(preference));
+    }
+
+    overrides[preference] = value;
+    return (customSave.get(preference) || savePref)(preference);
+  },
+
+  /**
    * Adds a callback that is called when the
    * value of a specified preference changed.
    *
    * @param {string}   preference
    * @param {function} callback
    */
   on(preference, callback)
   {
     eventEmitter.on(preference, callback);
   },
@@ skipping 27 matching lines @@
   return key.substr(keyPrefix.length);
 }
 
 function prefToKey(pref)
 {
   return keyPrefix + pref;
 }
 
 function savePref(pref)
 {
-  browser.storage.local.set({[prefToKey(pref)]: overrides[pref]});
+  return browser.storage.local.set({[prefToKey(pref)]: overrides[pref]});
 }
 
 let customSave = new Map();
 if (require("../buildtools/info").platform == "gecko")
 {
   // Saving one storage value causes all others to be saved as well on Gecko.
   // Make sure that updating ad counter doesn't cause the filters data to be
   // saved frequently as a side-effect.
   const MIN_UPDATE_INTERVAL = 60 * 1000;
   let lastUpdate = -MIN_UPDATE_INTERVAL;
-  let updateScheduled = false;
+  let promise = null;
   customSave.set("blocked_total", pref =>
   {
-    if (updateScheduled)
-      return;
+    if (!promise)
+    {
+      promise = new Promise((resolve, reject) =>
+      {
+        let callback = () =>
+        {
+          lastUpdate = performance.now();
+          promise = null;

[Sebastian Noack, 2018/04/27 16:58:58]

Since callback() were potentially called synchronously below, this will
potentially reset promise to null before it gets returned. I figured the
simplest solution is to just always call setTimeout().

[Thomas Greiner, 2018/04/27 17:03:32]

Acknowledged.

+          savePref(pref).then(resolve, reject);
+        };
 
-    let callback = () =>
-    {
-      lastUpdate = performance.now();
-      updateScheduled = false;
-      savePref(pref);
-    };
-
-    let timeElapsed = performance.now() - lastUpdate;
-    if (timeElapsed < MIN_UPDATE_INTERVAL)
-    {
-      setTimeout(callback, MIN_UPDATE_INTERVAL - timeElapsed);
-      updateScheduled = true;
+        let timeElapsed = performance.now() - lastUpdate;
+        if (timeElapsed < MIN_UPDATE_INTERVAL)
+          setTimeout(callback, MIN_UPDATE_INTERVAL - timeElapsed);
+        else
+          callback();
+      });
     }
-    else
-      callback();
+    return promise;
   });
 }
 
 function addPreference(pref)
 {
   Object.defineProperty(Prefs, pref, {
     get() { return (pref in overrides ? overrides : defaults)[pref]; },
     set(value)
     {
-      let defaultValue = defaults[pref];
-
-      if (typeof value != typeof defaultValue)
-        throw new Error("Attempt to change preference type");
-
-      if (value == defaultValue)
-      {
-        delete overrides[pref];
-        browser.storage.local.remove(prefToKey(pref));
-      }
-      else
-      {
-        overrides[pref] = value;
-        (customSave.get(pref) || savePref)(pref);
-      }
+      Prefs.set(pref, value);
     },
     enumerable: true
   });
 }
 
 function init()
 {
   let prefs = Object.keys(defaults);
   prefs.forEach(addPreference);
 
   let localLoaded = browser.storage.local.get(prefs.map(prefToKey)).then(
     items =>
     {
       for (let key in items)
         overrides[keyToPref(key)] = items[key];
-    },
-    (error) =>
-    {
-      console.error(error);
     }
   );
 
   let managedLoaded;
   if ("managed" in browser.storage)
   {
     managedLoaded = browser.storage.managed.get(null).then(
       items =>
       {
         for (let key in items)
@@ skipping 28 matching lines @@
           eventEmitter.emit(pref);
         }
       }
     });
   }
 
   Prefs.untilLoaded = Promise.all([localLoaded, managedLoaded]).then(onLoaded);
 }
 
 init();