Issue 6599 - Detect data corruption of storage.local

lib/prefs.js

Index: lib/prefs.js
===================================================================
--- a/lib/prefs.js
+++ b/lib/prefs.js
@@ -214,6 +214,31 @@
   */
 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.
    *
@@ -261,7 +286,7 @@
 
 function savePref(pref)
 {
-  browser.storage.local.set({[prefToKey(pref)]: overrides[pref]});
+  return browser.storage.local.set({[prefToKey(pref)]: overrides[pref]});
 }
 
 let customSave = new Map();
@@ -272,27 +297,28 @@
   // 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;
-
-    let callback = () =>
+    if (!promise)
     {
-      lastUpdate = performance.now();
-      updateScheduled = false;
-      savePref(pref);
-    };
+      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 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;
   });
 }
 
@@ -302,21 +328,7 @@
     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
   });
@@ -332,10 +344,6 @@
     {
       for (let key in items)
         overrides[keyToPref(key)] = items[key];
-    },
-    (error) =>
-    {
-      console.error(error);
     }
   );