save prefs in bg to avoid data loss

* add `now` to simplify usage of prefs.subscribe
* tweak/simplify bits by separating bg/content concerns

background/background.js

@@ -59,7 +59,8 @@ window.API_METHODS = Object.assign(window.API_METHODS || {}, {
   parseCss({code}) {
     return backgroundWorker.parseMozFormat({code});
   },
-  getPrefs: prefs.getAll,
+  getPrefs: () => prefs.values, // will be deepCopy'd by invokeAPI handler
+  setPref: (key, value) => prefs.set(key, value),
 
   openEditor,
 
@@ -218,6 +219,14 @@ function createContextMenus(ids) {
 }
 
 if (chrome.contextMenus) {
+  // "Delete" item in context menu for browsers that don't have it
+  if (CHROME &&
+      // looking at the end of UA string
+      /(Vivaldi|Safari)\/[\d.]+$/.test(navigator.userAgent) &&
+      // skip forks with Flash as those are likely to have the menu e.g. CentBrowser
+      !Array.from(navigator.plugins).some(p => p.name === 'Shockwave Flash')) {
+    prefs.defaults['editor.contextDelete'] = true;
+  }
   // circumvent the bug with disabling check marks in Chrome 62-64
   const toggleCheckmark = CHROME >= 62 && CHROME <= 64 ?
     (id => chrome.contextMenus.remove(id, () => createContextMenus([id]) + ignoreChromeError())) :
@@ -233,7 +242,7 @@ if (chrome.contextMenus) {
 
   const keys = Object.keys(contextMenus);
   prefs.subscribe(keys.filter(id => typeof prefs.defaults[id] === 'boolean'), toggleCheckmark);
-  prefs.subscribe(keys.filter(id => contextMenus[id].presentIf), togglePresence);
+  prefs.subscribe(keys.filter(id => contextMenus[id].presentIf && id in prefs.defaults), togglePresence);
   createContextMenus(keys);
 }
 

js/prefs.js

@@ -1,9 +1,11 @@
 /* global promisifyChrome msg API */
+/* global deepCopy deepEqual debounce */ // not used in content scripts
 'use strict';
 
-// Needs msg.js loaded first
+// eslint-disable-next-line no-unused-expressions
-
+window.INJECTED !== 1 && (() => {
-self.prefs = self.INJECTED === 1 ? self.prefs : (() => {
+  const STORAGE_KEY = 'settings';
+  const clone = msg.isBg ? deepCopy : (val => JSON.parse(JSON.stringify(val)));
   const defaults = {
     'openEditInWindow': false,      // new editor opens in a own browser window
     'openEditInWindow.popup': false, // new editor opens in a simplified browser window without omnibox
@@ -72,7 +74,8 @@ self.prefs = self.INJECTED === 1 ? self.prefs : (() => {
                                       // '' (empty string) = disabled
     'editor.autoCloseBrackets': true,    // auto-add a closing pair when typing an opening one of ()[]{}''""
     'editor.autocompleteOnTyping': false, // show autocomplete dropdown on typing a word token
-    'editor.contextDelete': contextDeleteMissing(), // "Delete" item in context menu
+    // "Delete" item in context menu for browsers that don't have it
+    'editor.contextDelete': null,
     'editor.selectByTokens': true,
 
     'editor.appliesToLineWidget': true, // show applies-to line widget on the editor
@@ -105,189 +108,126 @@ self.prefs = self.INJECTED === 1 ? self.prefs : (() => {
 
     'updateInterval': 24,           // user-style automatic update interval, hours (0 = disable)
   };
-  const values = deepCopy(defaults);
+  const values = clone(defaults);
-
   const onChange = {
     any: new Set(),
-    specific: new Map(),
+    specific: {},
   };
-
+  if (msg.isBg) {
     promisifyChrome({
       'storage.sync': ['get', 'set'],
     });
-
+  }
   const initializing = (
     msg.isBg
-      ? browser.storage.sync.get('settings').then(res => res.settings)
+      ? browser.storage.sync.get(STORAGE_KEY).then(res => res[STORAGE_KEY])
       : API.getPrefs()
-  ).then(res => res && setAll(res, true));
+  ).then(setAll);
 
-  chrome.storage.onChanged.addListener((changes, area) => {
+  chrome.storage.onChanged.addListener(async (changes, area) => {
-    if (area !== 'sync' || !changes.settings || !changes.settings.newValue) {
+    const data = area === 'sync' && changes[STORAGE_KEY];
-      return;
+    if (data) {
+      await initializing;
+      setAll(data.newValue);
     }
-    initializing.then(() => setAll(changes.settings.newValue, true));
   });
 
-  let timer;
+  // This direct assignment allows IDEs to provide correct autocomplete for methods
-
+  const prefs = window.prefs = {
-  // coalesce multiple pref changes in broadcast
-  // let changes = {};
-
-  return {
     initializing,
     defaults,
-    get(key, defaultValue) {
+    values,
-      if (key in values) {
+    get(key) {
-        return values[key];
+      return isKnown(key) && values[key];
-      }
-      if (defaultValue !== undefined) {
-        return defaultValue;
-      }
-      if (key in defaults) {
-        return defaults[key];
-      }
-      console.warn("No default preference for '%s'", key);
     },
-    getAll() {
+    set(key, value, isSynced) {
-      return deepCopy(values);
+      if (!isKnown(key)) return;
+      const oldValue = values[key];
+      const type = typeof defaults[key];
+      if (type !== typeof value) {
+        if (type === 'string') value = String(value);
+        if (type === 'number') value = Number(value) || 0;
+        if (type === 'boolean') value = Boolean(value);
+      }
+      if (value !== oldValue && !deepEqual(value, oldValue)) {
+        values[key] = value;
+        emitChange(key, value, isSynced);
+      }
     },
-    set,
+    reset(key) {
-    reset: key => set(key, deepCopy(defaults[key])),
+      prefs.set(key, clone(defaults[key]));
-    subscribe(keys, listener) {
+    },
-      // keys:     string[] ids
+    /**
-      //           or a falsy value to subscribe to everything
+     * @param {?string|string[]} keys - pref ids or a falsy value to subscribe to everything
-      // listener: function (key, value)
+     * @param {function(key:string, value:any)} fn
+     * @param {Object} [opts]
+     * @param {boolean} [opts.now] - when truthy, the listener is called immediately:
+     *   1) if `keys` is an array of keys, each `key` will be fired separately with a real `value`
+     *   2) if `keys` is falsy, no key/value will be provided
+     */
+    subscribe(keys, fn, {now} = {}) {
+      if (keys) {
+        for (const key of Array.isArray(keys) ? keys : [keys]) {
+          if (!isKnown(key)) continue;
+          const listeners = onChange.specific[key] ||
+            (onChange.specific[key] = new Set());
+          listeners.add(fn);
+          if (now) fn(key, values[key]);
+        }
+      } else {
+        onChange.any.add(fn);
+        if (now) fn();
+      }
+    },
+    unsubscribe(keys, fn) {
       if (keys) {
         for (const key of keys) {
-          const existing = onChange.specific.get(key);
+          const listeners = onChange.specific[key];
-          if (!existing) {
+          if (listeners) {
-            onChange.specific.set(key, listener);
+            listeners.delete(fn);
-          } else if (existing instanceof Set) {
+            if (!listeners.size) {
-            existing.add(listener);
+              delete onChange.specific[key];
-          } else {
+            }
-            onChange.specific.set(key, new Set([existing, listener]));
           }
         }
       } else {
-        onChange.any.add(listener);
+        onChange.all.remove(fn);
-      }
-    },
-    unsubscribe(keys, listener) {
-      if (keys) {
-        for (const key of keys) {
-          const existing = onChange.specific.get(key);
-          if (existing instanceof Set) {
-            existing.delete(listener);
-            if (!existing.size) {
-              onChange.specific.delete(key);
-            }
-          } else if (existing) {
-            onChange.specific.delete(key);
-          }
-        }
-      } else {
-        onChange.all.remove(listener);
       }
     },
   };
 
-  function setAll(settings, synced) {
+  function isKnown(key) {
-    for (const [key, value] of Object.entries(settings)) {
+    const res = defaults.hasOwnProperty(key);
-      set(key, value, synced);
+    if (!res) console.warn('Unknown preference "%s"', key);
+    return res;
+  }
+
+  function setAll(settings) {
+    for (const [key, value] of Object.entries(settings || {})) {
+      prefs.set(key, value, true);
     }
   }
 
-  function set(key, value, synced = false) {
+  function emitChange(key, value, isSynced) {
-    const oldValue = values[key];
+    for (const fn of onChange.specific[key] || []) {
-    switch (typeof defaults[key]) {
+      fn(key, value);
-      case typeof value:
-        break;
-      case 'string':
-        value = String(value);
-        break;
-      case 'number':
-        value |= 0;
-        break;
-      case 'boolean':
-        value = value === true || value === 'true';
-        break;
     }
-    if (equal(value, oldValue)) {
+    for (const fn of onChange.any) {
-      return;
+      fn(key, value);
     }
-    values[key] = value;
+    if (!isSynced) {
-    emitChange(key, value);
+      /* browser.storage is slow and can randomly lose values if the tab was closed immediately
-    if (!synced && !timer) {
+       so we're sending the value to the background script which will save it to the storage;
-      timer = syncPrefsLater();
+       the extra bonus is that invokeAPI is immediate in extension tabs */
+      if (msg.isBg) {
+        debounce(updateStorage);
+      } else {
+        API.setPref(key, value);
       }
-    return timer;
-  }
-
-  function emitChange(key, value) {
-    const specific = onChange.specific.get(key);
-    if (typeof specific === 'function') {
-      specific(key, value);
-    } else if (specific instanceof Set) {
-      for (const listener of specific.values()) {
-        listener(key, value);
-      }
-    }
-    for (const listener of onChange.any.values()) {
-      listener(key, value);
     }
   }
 
-  function syncPrefsLater() {
+  function updateStorage() {
-    return new Promise((resolve, reject) => {
+    return browser.storage.sync.set({[STORAGE_KEY]: values});
-      setTimeout(() => {
-        timer = null;
-        browser.storage.sync.set({settings: values})
-          .then(resolve, reject);
-      });
-    });
-  }
-
-  function equal(a, b) {
-    if (!a || !b || typeof a !== 'object' || typeof b !== 'object') {
-      return a === b;
-    }
-    if (Object.keys(a).length !== Object.keys(b).length) {
-      return false;
-    }
-    for (const k in a) {
-      if (typeof a[k] === 'object') {
-        if (!equal(a[k], b[k])) {
-          return false;
-        }
-      } else if (a[k] !== b[k]) {
-        return false;
-      }
-    }
-    return true;
-  }
-
-  function contextDeleteMissing() {
-    return /Chrome\/\d+/.test(navigator.userAgent) && (
-      // detect browsers without Delete by looking at the end of UA string
-      /Vivaldi\/[\d.]+$/.test(navigator.userAgent) ||
-      // Chrome and co.
-      /Safari\/[\d.]+$/.test(navigator.userAgent) &&
-      // skip forks with Flash as those are likely to have the menu e.g. CentBrowser
-      !Array.from(navigator.plugins).some(p => p.name === 'Shockwave Flash')
-    );
-  }
-
-  function deepCopy(obj) {
-    if (!obj || typeof obj !== 'object') {
-      return obj;
-    }
-    if (Array.isArray(obj)) {
-      return obj.map(deepCopy);
-    }
-    return Object.keys(obj).reduce((output, key) => {
-      output[key] = deepCopy(obj[key]);
-      return output;
-    }, {});
   }
 })();