Merge branch 'master' into feat/platform-observers

Author: eritbh

.eslintrc.json

@@ -32,6 +32,9 @@
         "no-implied-eval": "error",
         "no-return-await": "error",
         "no-sequences": "error",
+        "no-unused-vars": ["error", {
+            "destructuredArrayIgnorePattern": "^_"
+        }],
         "no-var": "error",
         // these comment markers fail CI (notes to self in code that need to be
         // resolved before a PR can be merged)

extension/data/background/handlers/settings.js

@@ -0,0 +1,64 @@
+// Handling for writing Toolbox settings into extension storage.
+//
+// The entire settings object is stored as a single JSON value, so in order to
+// update a single setting, we need to read the entire object, update the
+// setting in our copy of the object, and then write back the entire object.
+// However, because reading and writing extension storage are asynchronous
+// operations, doing this naively would result in conflicts if two things wanted
+// to change settings at the same time - two consumers could both read the state
+// before the other has time to commit an update, leading to one update being
+// "forgotten" by the other. We get around this by introducing a mutex for
+// writing to setting storage. Content scripts request writes by messaging the
+// background page, so writes from all sources are governed by a single mutex.
+
+import {Mutex} from 'async-mutex';
+import browser from 'webextension-polyfill';
+
+// these are safe to import because they're implemented purely in terms of
+// `browser.storage`, with no reliance on state held in the content script
+import {messageHandlers} from '../messageHandling';
+
+/** Mutex governing writes to the `tbsettings` key of `browser.storage.local` */
+const settingsWriteMutex = new Mutex();
+
+/** Reads settings from storage */
+const getSettings = async () => (await browser.storage.local.get('tbsettings')).tbsettings ?? {};
+
+/** Writes a full settings object into storage */
+const writeSettings = newSettings => browser.storage.local.set({tbsettings: newSettings});
+
+// Updates the value(s) of one or more settings.
+//
+// NOTE: Messages are serialized to JSON, so it's not possible for a key to be
+//       present in a message object but have its value be `undefined` (which is
+//       how we typically represent the operation of removing a setting's key
+//       from storage). I don't want to use `null` for that instead, because
+//       there's lots of places in the storage code that freak out if they see
+//       `null`s as values and I don't want to cause issues with whatever's
+//       going on there. So we do it the slightly janky way: two message fields,
+//       one object mapping keys to new values, and one array of deleted keys.
+messageHandlers.set('tb-update-settings', async ({
+    updatedSettings,
+    deletedSettings,
+}) => {
+    await settingsWriteMutex.runExclusive(async () => {
+        const settings = await getSettings();
+        for (const [key, value] of Object.entries(updatedSettings ?? {})) {
+            if (value == null) {
+                continue;
+            }
+            settings[key] = value;
+        }
+        for (const key of deletedSettings ?? []) {
+            delete settings[key];
+        }
+        await writeSettings(settings);
+    });
+});
+
+// Overwrites the entire settings object.
+messageHandlers.set('tb-overwrite-all-settings', async ({newSettings}) => {
+    await settingsWriteMutex.runExclusive(async () => {
+        await writeSettings(newSettings);
+    });
+});

extension/data/background/index.ts

@@ -3,5 +3,6 @@ import './handlers/globalmessage.js';
 import './handlers/modqueue.js';
 import './handlers/notifications.js';
 import './handlers/reload.js';
+import './handlers/settings.js';
 import './handlers/url_changed.js';
 import './handlers/webrequest.js';

extension/data/components/controls/Icon.tsx

@@ -1,4 +1,4 @@
-import {icons} from '../../tbconstants';
+import {icons} from '../../util/icons';
 import {classes} from '../../util/ui_interop';
 import css from './Icon.module.css';
 

extension/data/hooks.ts

@@ -1,5 +1,6 @@
 import {useEffect, useState} from 'react';
-import {getSettingAsync} from './tbstorage';
+import {useSelector} from 'react-redux';
+import {RootState} from './store';
 
 /** Hook to get the return value of a promise. */
 export const useFetched = <T>(promise: Promise<T>) => {
@@ -23,5 +24,15 @@ export const useFetched = <T>(promise: Promise<T>) => {
 
 /** Hook to get a Toolbox setting. */
 export const useSetting = (moduleName: string, settingName: string, defaultValue: any) => {
-    return useFetched(getSettingAsync(moduleName, settingName, defaultValue));
+    const savedValue = useSelector((state: RootState) => state.settings.values[`Toolbox.${moduleName}.${settingName}`]);
+
+    // Return the given default value if the setting doesn't have a value (i.e.
+    // is `undefined`) *or* if the setting's value is `null` (mirroring the old
+    // implementation of `getSetting` from the old `tbstorage.js`, which says
+    // that `null` is never a valid value for any setting)
+    if (savedValue == null) {
+        return defaultValue;
+    }
+
+    return savedValue;
 };

extension/data/init.ts

@@ -32,14 +32,15 @@ import * as TBApi from './tbapi';
 import * as TBCore from './tbcore.js';
 import {delay} from './tbhelpers.js';
 import TBListener from './tblistener.js';
-import TBLog from './tblog';
 import TBModule from './tbmodule.jsx';
-import * as TBStorage from './tbstorage.js';
 
 import AppRoot from './AppRoot';
 import {initializeObserver} from './frontends';
+import {getCache, setCache} from './util/cache';
 import {documentInteractive} from './util/dom';
+import createLogger from './util/logging';
 import {isUserLoggedInQuick} from './util/platform';
+import {getSettingAsync, setSettingAsync, updateSettings} from './util/settings';
 import {reactRenderer} from './util/ui_interop';
 
 import Achievements from './modules/achievements.js';
@@ -162,12 +163,12 @@ async function checkLoadConditions (tries = 3) {
 
     // Write a setting and read back its value, if this fails something is wrong
     let echoValue = Math.random();
-    let echoResult: number;
     try {
-        echoResult = await TBStorage.setSettingAsync('Utils', 'echoTest', echoValue);
+        await setSettingAsync('Utils', 'echoTest', echoValue);
     } catch (error) {
         throw new Error('Failed to write to settings', {cause: error});
     }
+    const echoResult = await getSettingAsync('Utils', 'echoTest');
     if (echoResult !== echoValue) {
         throw new Error(`Settings read/write inconsistent: expected ${echoValue}, received ${echoResult}`);
     }
@@ -185,11 +186,11 @@ async function doSettingsUpdates () {
     const currentUser = await TBApi.getCurrentUser();
     let lastVersion = await TBCore.getLastVersion();
 
-    const cacheName = await TBStorage.getCache('Utils', 'cacheName', '');
+    const cacheName = await getCache('Utils', 'cacheName', '');
 
     // Update cache if we're logged in as someone else
     if (cacheName !== currentUser) {
-        await TBStorage.setCache(SETTINGS_NAME, 'cacheName', currentUser);
+        await setCache(SETTINGS_NAME, 'cacheName', currentUser);
 
         // Force refresh of timed cache
         browser.runtime.sendMessage({
@@ -200,46 +201,46 @@ async function doSettingsUpdates () {
     // Extra checks on old faults
     if (typeof lastVersion !== 'number') {
         lastVersion = parseInt(lastVersion);
-        await TBStorage.setSettingAsync(SETTINGS_NAME, 'lastVersion', lastVersion);
+        await setSettingAsync(SETTINGS_NAME, 'lastVersion', lastVersion);
     }
 
-    let shortLength = await TBStorage.getSettingAsync(SETTINGS_NAME, 'shortLength', 15);
-    let longLength = await TBStorage.getSettingAsync(SETTINGS_NAME, 'longLength', 45);
+    let shortLength = await getSettingAsync(SETTINGS_NAME, 'shortLength', 15);
+    let longLength = await getSettingAsync(SETTINGS_NAME, 'longLength', 45);
 
     if (typeof shortLength !== 'number') {
         shortLength = parseInt(shortLength);
-        await TBStorage.setSettingAsync(SETTINGS_NAME, 'shortLength', shortLength);
+        await setSettingAsync(SETTINGS_NAME, 'shortLength', shortLength);
     }
 
     if (typeof longLength !== 'number') {
         longLength = parseInt(longLength);
-        await TBStorage.setSettingAsync(SETTINGS_NAME, 'longLength', longLength);
+        await setSettingAsync(SETTINGS_NAME, 'longLength', longLength);
     }
 
     // First run changes for all releases.
     if (TBCore.shortVersion > lastVersion) {
         // These need to happen for every version change
-        await TBStorage.setSettingAsync(SETTINGS_NAME, 'lastVersion', TBCore.shortVersion); // set last version to this version.
+        await setSettingAsync(SETTINGS_NAME, 'lastVersion', TBCore.shortVersion); // set last version to this version.
         TBCore.getToolboxDevs(); // always repopulate tb devs for each version change
 
         // This should be a per-release section of stuff we want to change in each update.  Like setting/converting data/etc.  It should always be removed before the next release.
 
         // Start: version changes.
         // reportsThreshold should be 0 by default
         if (lastVersion < 50101) {
-            await TBStorage.setSettingAsync('QueueTools', 'reportsThreshold', 0);
+            await setSettingAsync('QueueTools', 'reportsThreshold', 0);
         }
 
         // Clean up removed settings - it doesn't really matter what version
         // we're coming from, we just want to make sure these removed settings
         // aren't cluttering up storage
-        await Promise.all([
+        const keysToDelete = [
             // Some new modmail settings were removed in 5.7.0
-            TBStorage.setSettingAsync('NewModMail', 'searchhelp', undefined),
-            TBStorage.setSettingAsync('NewModMail', 'checkForNewMessages', undefined),
+            'Toolbox.NewModMail.searchhelp',
+            'Toolbox.NewModMail.checkForNewMessages',
 
             // Beta mode setting removed in favor of dedicated beta builds #917
-            TBStorage.setSettingAsync(SETTINGS_NAME, 'betaMode', undefined),
+            'Toolbox.Utils.betaMode',
 
             // (old) modmail pro removed in v7, RIP old modmail
             ...[
@@ -265,26 +266,27 @@ async function doSettingsUpdates () {
                 'entryProcessRate',
                 'chunkProcessSize',
                 'twoPhaseProcessing',
-            ].map(setting => TBStorage.setSettingAsync('ModMail', setting, undefined)),
+            ].map(setting => `Toolbox.ModMail.${setting}`),
 
             // new reddit is dead, long live shreddit i guess. the setting to
             // skip the new reddit lightbox when viewing comments no longer
             // applies to anything, remove it
-            TBStorage.setSettingAsync('Comments', 'commentsAsFullPage', undefined),
-        ]);
+            'Toolbox.Comments.commentsAsFullPage',
+        ];
+        await updateSettings(Object.fromEntries(keysToDelete.map(key => [key, undefined])));
 
         // End: version changes.
 
         // This is a super extra check to make sure the wiki page for settings export really is private.
-        const settingSubEnabled = await TBStorage.getSettingAsync('Utils', 'settingSub', '');
+        const settingSubEnabled = await getSettingAsync('Utils', 'settingSub', '');
         if (settingSubEnabled) {
             TBCore.setWikiPrivate('tbsettings', settingSubEnabled, false);
         }
 
         // These two should be left for every new release. If there is a new beta feature people want, it should be opt-in, not left to old settings.
-        // TBStorage.setSetting('Notifier', 'lastSeenModmail', now); // don't spam 100 new mod mails on first install.
-        // TBStorage.setSetting('Notifier', 'modmailCount', 0);
-        await TBStorage.setSettingAsync(SETTINGS_NAME, 'debugMode', false);
+        // await setSettingAsync('Notifier', 'lastSeenModmail', now); // don't spam 100 new mod mails on first install.
+        // await setSettingAsync('Notifier', 'modmailCount', 0);
+        await setSettingAsync(SETTINGS_NAME, 'debugMode', false);
     }
 }
 
@@ -295,13 +297,13 @@ async function doSettingsUpdates () {
     }
 
     // Create a logger
-    const logger = TBLog('Init');
+    const log = createLogger('Init');
 
     // Ensure that other conditions are met, and return early if not
     try {
         await checkLoadConditions();
     } catch (error) {
-        logger.error('Load condition not met:', (error as Error).message);
+        log.error('Load condition not met:', error);
         return;
     }
 
@@ -391,7 +393,7 @@ async function doSettingsUpdates () {
             OldReddit,
         ]
     ) {
-        logger.debug('Registering module', m);
+        log.debug('Registering module', m);
         TBModule.register_module(m);
     }
 

extension/data/modules/achievements.js

@@ -3,7 +3,10 @@ import $ from 'jquery';
 import * as TBCore from '../tbcore.js';
 import * as TBHelpers from '../tbhelpers.js';
 import {Module} from '../tbmodule.jsx';
-import {getSettingAsync} from '../tbstorage.js';
+import createLogger from '../util/logging.ts';
+import {getSettingAsync} from '../util/settings.ts';
+
+const log = createLogger('Achievements');
 
 const self = new Module({
     name: 'Achievements',
@@ -65,12 +68,12 @@ function Manager () {
             const title = titles[i];
             const maxValue = maxValues[i];
 
-            self.log('Registering Achievement');
+            log.debug('Registering Achievement');
             if (debugMode) {
-                self.log(`  name=${title}`);
+                log.debug(`  name=${title}`);
             } // spoilers
-            self.log(`  maxValue=${maxValue}`);
-            self.log(`  saveIndex=${saveIndex}`);
+            log.debug(`  maxValue=${maxValue}`);
+            log.debug(`  saveIndex=${saveIndex}`);
 
             achievementsBlock.push({
                 title,
@@ -89,19 +92,19 @@ function Manager () {
         if (value === undefined) {
             value = 1;
         }
-        self.log(`Unlocking achievement block: index=${saveIndex}, value=${value}`);
+        log.debug(`Unlocking achievement block: index=${saveIndex}, value=${value}`);
 
         const old = saves[saveIndex];
-        self.log(`  Old value: ${saves[saveIndex]}`);
+        log.debug(`  Old value: ${saves[saveIndex]}`);
         saves[saveIndex] += value;
-        self.log(`  New value: ${saves[saveIndex]}`);
+        log.debug(`  New value: ${saves[saveIndex]}`);
 
         const achievementsBlock = achievements[saveIndex];
         let achievement;
         for (let index = 0; index < achievementsBlock.length; index++) {
-            self.log(`  Checking achievement ${index}`);
+            log.debug(`  Checking achievement ${index}`);
             achievement = achievementsBlock[index];
-            self.log(`    Comparing to max value: ${achievement.maxValue}`);
+            log.debug(`    Comparing to max value: ${achievement.maxValue}`);
             if (saves[saveIndex] >= achievement.maxValue && old < achievement.maxValue) {
                 let title = achievement.title;
 
@@ -111,10 +114,10 @@ function Manager () {
                 try {
                     title = $(achievement.title).text() || achievement.title;
                 } catch (e) {
-                    self.log(`error: ${e}`);
+                    log.debug(`error: ${e}`);
                 }
 
-                self.log(`${title} Unlocked!`);
+                log.debug(`${title} Unlocked!`);
                 TBCore.notification(
                     'Mod achievement unlocked!',
                     title,
@@ -206,7 +209,7 @@ function init ({lastSeen}) {
     const $body = $('body');
 
     // Achievement definitions
-    self.log('Registering achievements');
+    log.debug('Registering achievements');
 
     // Random awesome
     self.manager.register(
@@ -216,7 +219,7 @@ function init ({lastSeen}) {
             const awesome = 7;
             const chanceOfBeingAwesome = TBHelpers.getRandomNumber(10000);
 
-            self.log(`You rolled a: ${chanceOfBeingAwesome}`);
+            log.debug(`You rolled a: ${chanceOfBeingAwesome}`);
             if (awesome === chanceOfBeingAwesome) {
                 self.manager.unlock(saveIndex);
             }
@@ -233,10 +236,10 @@ function init ({lastSeen}) {
             const now = TBHelpers.getTime();
             const timeSince = now - lastSeen;
             const daysSince = TBHelpers.millisecondsToDays(timeSince);
-            self.log(`daysSince: ${daysSince}`);
+            log.debug(`daysSince: ${daysSince}`);
 
             if (daysSince >= 7) {
-                // self.log("you've got an award!");
+                // log.debug("you've got an award!");
                 self.manager.unlock(saveIndex);
             }
 
@@ -278,7 +281,7 @@ function init ({lastSeen}) {
             }
             // TODO: wait for 'yes' click.
             // $body.on('click', '.yes', function(){
-            //  self.log('yes clicked');
+            //  log.debug('yes clicked');
             // });
         });
     });