refactor: rework/simplify GMxxxValueChanged handling

src/background/index.js

@@ -54,7 +54,7 @@ Object.assign(commands, {
     };
     if (isApplied) {
       const scripts = await (cache.get(`preinject:${url}`) || getScriptsByURL(url));
-      addValueOpener(tabId, Object.keys(scripts.values));
+      addValueOpener(tabId, src.frameId, Object.keys(scripts.values));
       Object.assign(data, scripts);
     }
     return data;

src/background/utils/values.js

@@ -1,11 +1,11 @@
-import { sendTabCmd } from '#/common';
+import { isEmpty, sendTabCmd } from '#/common';
+import { objectSet } from '#/common/object';
 import { getValueStoresByIds, dumpValueStores, dumpValueStore } from './db';
 import { commands } from './message';
 
-const openers = {}; // scriptId: { openerId: 1, ... }
-const tabScripts = {}; // openerId: { scriptId: 1, ... }
-let cache;
-let timer;
+const openers = {}; // { scriptId: { tabId: [frameId, ... ], ... } }
+let cache; // { scriptId: { key: [{ value, src }, ... ], ... } }
+let updateScheduled;
 
 Object.assign(commands, {
   /** @return {Promise<Object>} */
@@ -17,89 +17,101 @@ Object.assign(commands, {
   async SetValueStore({ where, valueStore }) {
     // Value store will be replaced soon.
     const store = await dumpValueStore(where, valueStore);
-    broadcastUpdates(store);
+    if (store) broadcastUpdates(store);
   },
   /** @return {Promise<void>} */
-  UpdateValue({ id, update }) {
+  UpdateValue({ id, update: { key, value } }, src) {
     // Value will be updated to store later.
     updateLater();
-    const { key, value } = update;
-    if (!cache) cache = {};
-    let updates = cache[id];
-    if (!updates) {
-      updates = {};
-      cache[id] = updates;
-    }
-    updates[key] = value || null;
+    cache = objectSet(cache, [id, [key]], {
+      value: value || null,
+      src: `${src.tab.id}:${src.frameId}`,
+    });
   },
 });
 
 browser.tabs.onRemoved.addListener(resetValueOpener);
+browser.tabs.onReplaced.addListener((addedId, removedId) => resetValueOpener(removedId));
 
-export function resetValueOpener(openerId) {
-  const scriptMap = tabScripts[openerId];
-  if (scriptMap) {
-    Object.keys(scriptMap).forEach((scriptId) => {
-      const map = openers[scriptId];
-      if (map) delete map[openerId];
-    });
-    delete tabScripts[openerId];
-  }
+export function resetValueOpener(tabId) {
+  Object.entries(openers).forEach(([id, openerTabs]) => {
+    if (tabId in openerTabs) {
+      delete openerTabs[tabId];
+      if (isEmpty(openerTabs)) delete openers[id];
+    }
+  });
 }
 
-export function addValueOpener(openerId, scriptIds) {
-  let scriptMap = tabScripts[openerId];
-  if (!scriptMap) {
-    scriptMap = {};
-    tabScripts[openerId] = scriptMap;
-  }
-  scriptIds.forEach((scriptId) => {
-    scriptMap[scriptId] = 1;
-    let openerMap = openers[scriptId];
-    if (!openerMap) {
-      openerMap = {};
-      openers[scriptId] = openerMap;
-    }
-    openerMap[openerId] = 1;
+export function addValueOpener(tabId, frameId, scriptIds) {
+  scriptIds.forEach((id) => {
+    objectSet(openers, [id, [tabId]], frameId);
   });
 }
 
-function updateLater() {
-  if (!timer) {
-    timer = Promise.resolve().then(doUpdate);
-    // timer = setTimeout(doUpdate);
+async function updateLater() {
+  if (!updateScheduled) {
+    updateScheduled = true;
+    await 0;
+    doUpdate();
+    updateScheduled = false;
+    if (cache) updateLater();
   }
 }
 
 async function doUpdate() {
+  const ids = Object.keys(cache);
   const currentCache = cache;
   cache = null;
-  const ids = Object.keys(currentCache);
   try {
     const valueStores = await getValueStoresByIds(ids);
     ids.forEach((id) => {
-      const valueStore = valueStores[id] || {};
-      valueStores[id] = valueStore;
+      const valueStore = valueStores[id] || (valueStores[id] = {});
       const updates = currentCache[id] || {};
       Object.keys(updates).forEach((key) => {
-        const value = updates[key];
-        if (!value) delete valueStore[key];
-        else valueStore[key] = value;
+        const history = updates[key];
+        if (!history) delete valueStore[key];
+        else valueStore[key] = history[history.length - 1].value;
       });
     });
-    await broadcastUpdates(await dumpValueStores(valueStores));
+    await dumpValueStores(valueStores);
+    await broadcastUpdates(valueStores, currentCache);
   } catch (err) {
     console.error('Values error:', err);
   }
-  timer = null;
-  if (cache) updateLater();
 }
 
-function broadcastUpdates(updates) {
-  if (updates) {
-    const updatedOpeners = Object.keys(updates)
-    .reduce((map, scriptId) => Object.assign(map, openers[scriptId]), {});
-    Object.keys(updatedOpeners)
-    .forEach(openerId => sendTabCmd(+openerId, 'UpdatedValues', updates));
+function broadcastUpdates(updates, oldCache = {}) {
+  // group updates by frame
+  const toSend = {};
+  Object.entries(updates).forEach(([id, data]) => {
+    Object.entries(openers[id]).forEach(([tabId, frames]) => {
+      frames.forEach(frameId => {
+        objectSet(toSend, [tabId, frameId, id],
+          avoidInitiator(data, oldCache[id], tabId, frameId));
+      });
+    });
+  });
+  // send the grouped updates
+  Object.entries(toSend).forEach(([tabId, frames]) => {
+    Object.entries(frames).forEach(([frameId, frameData]) => {
+      if (!isEmpty(frameData)) {
+        sendTabCmd(+tabId, 'UpdatedValues', frameData, { frameId: +frameId });
+      }
+    });
+  });
+}
+
+function avoidInitiator(data, history, tabId, frameId) {
+  let clone;
+  if (history) {
+    const src = `${tabId}:${frameId}`;
+    Object.entries(data).forEach(([key, value]) => {
+      if (history[key]?.some(h => h.value === value && h.src === src)) {
+        if (!clone) clone = { ...data };
+        delete clone[key];
+      }
+    });
+    if (clone) data = clone;
   }
+  return !isEmpty(data) ? data : undefined; // undef will remove the key in objectSet
 }

src/common/browser.js

@@ -96,6 +96,7 @@ if (!global.browser?.runtime?.sendMessage) {
       onCreated: true,
       onUpdated: true,
       onRemoved: true,
+      onReplaced: true,
       create: wrapAsync,
       get: wrapAsync,
       query: wrapAsync,

src/common/object.js

@@ -19,16 +19,15 @@ export function objectSet(obj, rawKey, val) {
   if (!keys.length) return val;
   const root = obj || {};
   let sub = root;
-  const lastKey = keys.pop();
+  let lastKey = keys.pop();
   keys.forEach((key) => {
-    let child = sub[key];
-    if (!child) {
-      child = {};
-      sub[key] = child;
-    }
-    sub = child;
+    sub = sub[key] || (sub[key] = {});
   });
-  if (typeof val === 'undefined') {
+  if (Array.isArray(lastKey)) {
+    lastKey = lastKey[0];
+    sub = sub[lastKey] || (sub[lastKey] = []);
+    sub.push(val);
+  } else if (typeof val === 'undefined') {
     delete sub[lastKey];
   } else {
     sub[lastKey] = val;

src/injected/web/gm-api.js

@@ -50,9 +50,9 @@ export function createGmApiProps() {
     /**
      * @callback GMValueChangeListener
      * @param {String} key
-     * @param {any} oldValue - undefined = value was created
-     * @param {any} newValue - undefined = value was removed
-     * @param {boolean} remote - true = value was modified in another tab
+     * @param {?} oldValue - `undefined` means value was created
+     * @param {?} newValue - `undefined` means value was removed
+     * @param {boolean} remote - `true` means value was modified in another tab
      */
     /**
      * @param {String} key - name of the value to monitor

src/injected/web/gm-values.js

@@ -1,19 +1,13 @@
 import bridge from './bridge';
 import store from './store';
 import {
-  jsonLoad, filter, forEach, push, slice,
-  objectKeys, objectValues, objectEntries, log, setTimeout,
+  jsonLoad, forEach, slice, objectKeys, objectValues, objectEntries, log,
 } from '../utils/helpers';
 
 const { Number } = global;
-const perfNow = performance.now.bind(performance);
 
 // Nested objects: scriptId -> keyName -> listenerId -> GMValueChangeListener
 export const changeHooks = {};
-let localChanges = [];
-let cleanupTimer;
-// wait until UpdatedValues message arrives (usually it happens in just a few milliseconds)
-const CLEANUP_TIMEOUT = 10e3;
 
 const dataDecoders = {
   o: jsonLoad,
@@ -47,7 +41,13 @@ export function dumpValue(change = {}) {
     id,
     update: { key, value: raw },
   });
-  if (raw !== oldRaw) changedLocally(change);
+  if (raw !== oldRaw) {
+    const hooks = changeHooks[id]?.[key];
+    if (hooks) {
+      change.hooks = hooks;
+      notifyChange(change);
+    }
+  }
 }
 
 export function decodeValue(raw) {
@@ -63,52 +63,19 @@ export function decodeValue(raw) {
 }
 
 // { id, key, val, raw, oldRaw }
-function changedLocally(change) {
-  const hooks = changeHooks[change.id]?.[change.key];
-  if (hooks) {
-    change.hooks = hooks;
-    notifyChange(change);
-  }
-}
-
 function changedRemotely(id, oldData, updates) {
   const data = updates[id];
-  const keyHooks = changeHooks[id];
-  // the remote id is a string, but all local data structures use a number
-  id = +id;
-  /**
-   * Since the script instance that produced the change already got its change reports
-   * (remote=false), we need to avoid re-reporting with the wrong remote=true parameter,
-   * so we coalesce the more detailed local changes to match the remote changes.
-   * For a contrived example let's look at this local history:
-   *     undefined 'a' - created
-   *     'a' 'b'       - changed
-   *     'b' undefined - deleted
-   * The remote history is debounced so it might look like:
-   *     undefined 'a'
-   *     'a' undefined
-   */
-  objectEntries(keyHooks)::forEach(([key, hooks]) => {
-    const oldRaw = oldData[key];
-    const raw = data[key];
-    if (raw === oldRaw) return;
-    let found;
-    let lookFor = oldRaw;
-    localChanges = localChanges::filter((ch) => {
-      if (found || ch.id !== id || ch.key !== key) return true;
-      if (ch.oldRaw !== lookFor) return;
-      lookFor = ch.raw;
-      found = lookFor === raw;
+  id = +id; // the remote id is a string, but all local data structures use a number
+  objectEntries(changeHooks[id])::forEach(([key, hooks]) => {
+    notifyChange({
+      id,
+      key,
+      hooks,
+      oldRaw: oldData[key],
+      raw: data[key],
+      remote: true,
     });
-    if (!found) {
-      notifyChange({
-        hooks, id, key, raw, oldRaw, remote: true,
-      });
-    }
   });
-  if (localChanges.length && !cleanupTimer) {
-    cleanupTimer = setTimeout(cleanupChanges, CLEANUP_TIMEOUT);
-  }
 }
 
 // { hooks, key, val, raw, oldRaw, remote }
@@ -119,18 +86,6 @@ function notifyChange(change) {
   const oldVal = oldRaw && decodeValue(oldRaw);
   const newVal = val == null && raw ? decodeValue(raw) : val;
   objectValues(hooks)::forEach(fn => tryCall(fn, key, oldVal, newVal, remote));
-  if (!remote) {
-    change.expiry = perfNow() + CLEANUP_TIMEOUT;
-    localChanges::push(change);
-  }
-}
-
-function cleanupChanges() {
-  const now = perfNow();
-  localChanges = localChanges::filter(ch => ch.expiry > now);
-  cleanupTimer = localChanges.length
-    ? setTimeout(cleanupChanges, CLEANUP_TIMEOUT)
-    : 0;
 }
 
 function tryCall(fn, ...args) {