Add safePerformance to guard against bad performance calls (#1677)

* Disallow the use of performance * Add safePerformance * Replace all performance with safePerformance * Disconnect safePerformance from Performance due to constructor issues * Fix missing import * Add safe-performance to eslintrc * Ignore errors for using performance in dev directory * Add missing imports
yomidevs · Dec 17, 2024 · b39bd51 · b39bd51
1 parent f23c870
commit b39bd51
Show file tree

Hide file tree

Showing 13 changed files with 148 additions and 58 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -79,6 +79,10 @@
             {
                 "message": "Avoid using Response.json(), prefer readResponseJson.",
                 "selector": "MemberExpression[property.name=json]"
+            },
+            {
+                "message": "Avoid using performance, prefer safePerformance.",
+                "selector": "MemberExpression[object.name=performance]"
             }
         ],
         "no-self-compare": "error",
@@ -582,6 +586,7 @@
                 "ext/js/core/extension-error.js",
                 "ext/js/core/json.js",
                 "ext/js/core/log.js",
+                "ext/js/core/safe-performance.js",
                 "ext/js/core/to-error.js",
                 "ext/js/core/utilities.js",
                 "ext/js/data/database.js",
@@ -619,6 +624,7 @@
                 "ext/js/core/log-utilities.js",
                 "ext/js/core/log.js",
                 "ext/js/core/object-utilities.js",
+                "ext/js/core/safe-performance.js",
                 "ext/js/core/to-error.js",
                 "ext/js/core/utilities.js",
                 "ext/js/data/anki-util.js",

diff --git a/dev/bin/schema-validate.js b/dev/bin/schema-validate.js
@@ -43,15 +43,18 @@ function main() {
     const schema = parseJson(schemaSource);
 
     for (const dataFileName of args.slice(1)) {
+        // eslint-disable-next-line no-restricted-syntax
         const start = performance.now();
         try {
             console.log(`Validating ${dataFileName}...`);
             const dataSource = fs.readFileSync(dataFileName, {encoding: 'utf8'});
             const data = parseJson(dataSource);
             createJsonSchema(mode, schema).validate(data);
+            // eslint-disable-next-line no-restricted-syntax
             const end = performance.now();
             console.log(`No issues detected (${((end - start) / 1000).toFixed(2)}s)`);
         } catch (e) {
+            // eslint-disable-next-line no-restricted-syntax
             const end = performance.now();
             console.log(`Encountered an error (${((end - start) / 1000).toFixed(2)}s)`);
             console.warn(e);

diff --git a/dev/dictionary-validate.js b/dev/dictionary-validate.js
@@ -131,14 +131,17 @@ export async function testDictionaryFiles(mode, dictionaryFileNames) {
     const schemas = getSchemas();
 
     for (const dictionaryFileName of dictionaryFileNames) {
+        // eslint-disable-next-line no-restricted-syntax
         const start = performance.now();
         try {
             console.log(`Validating ${dictionaryFileName}...`);
             const source = fs.readFileSync(dictionaryFileName);
             await validateDictionary(mode, source.buffer, schemas);
+            // eslint-disable-next-line no-restricted-syntax
             const end = performance.now();
             console.log(`No issues detected (${((end - start) / 1000).toFixed(2)}s)`);
         } catch (e) {
+            // eslint-disable-next-line no-restricted-syntax
             const end = performance.now();
             console.log(`Encountered an error (${((end - start) / 1000).toFixed(2)}s)`);
             console.warn(e);

diff --git a/ext/js/app/frontend.js b/ext/js/app/frontend.js
@@ -20,6 +20,7 @@ import {createApiMap, invokeApiMapHandler} from '../core/api-map.js';
 import {EventListenerCollection} from '../core/event-listener-collection.js';
 import {log} from '../core/log.js';
 import {promiseAnimationFrame} from '../core/promise-animation-frame.js';
+import {safePerformance} from '../core/safe-performance.js';
 import {setProfile} from '../data/profiles-util.js';
 import {addFullscreenChangeEventListener, getFullscreenElement} from '../dom/document-util.js';
 import {TextSourceElement} from '../dom/text-source-element.js';
@@ -952,13 +953,13 @@ export class Frontend {
      * @returns {Promise<boolean>}
      */
     async _scanSelectedText(allowEmptyRange, disallowExpandSelection, showEmpty = false) {
-        performance.mark('frontend:scanSelectedText:start');
+        safePerformance.mark('frontend:scanSelectedText:start');
         const range = this._getFirstSelectionRange(allowEmptyRange);
         if (range === null) { return false; }
         const source = disallowExpandSelection ? TextSourceRange.createLazy(range) : TextSourceRange.create(range);
         await this._textScanner.search(source, {focus: true, restoreSelection: true}, showEmpty);
-        performance.mark('frontend:scanSelectedText:end');
-        performance.measure('frontend:scanSelectedText', 'frontend:scanSelectedText:start', 'frontend:scanSelectedText:end');
+        safePerformance.mark('frontend:scanSelectedText:end');
+        safePerformance.measure('frontend:scanSelectedText', 'frontend:scanSelectedText:start', 'frontend:scanSelectedText:end');
         return true;
     }
 

diff --git a/ext/js/app/popup.js b/ext/js/app/popup.js
@@ -21,6 +21,7 @@ import {DynamicProperty} from '../core/dynamic-property.js';
 import {EventDispatcher} from '../core/event-dispatcher.js';
 import {EventListenerCollection} from '../core/event-listener-collection.js';
 import {ExtensionError} from '../core/extension-error.js';
+import {safePerformance} from '../core/safe-performance.js';
 import {deepEqual} from '../core/utilities.js';
 import {addFullscreenChangeEventListener, computeZoomScale, convertRectZoomCoordinates, getFullscreenElement} from '../dom/document-util.js';
 import {loadStyle} from '../dom/style-util.js';
@@ -302,7 +303,7 @@ export class Popup extends EventDispatcher {
         await this._show(sourceRects, writingMode);
 
         if (displayDetails !== null) {
-            performance.mark('invokeDisplaySetContent:start');
+            safePerformance.mark('invokeDisplaySetContent:start');
             void this._invokeSafe('displaySetContent', {details: displayDetails});
         }
     }

diff --git a/ext/js/comm/cross-frame-api.js b/ext/js/comm/cross-frame-api.js
@@ -22,6 +22,7 @@ import {EventListenerCollection} from '../core/event-listener-collection.js';
 import {ExtensionError} from '../core/extension-error.js';
 import {parseJson} from '../core/json.js';
 import {log} from '../core/log.js';
+import {safePerformance} from '../core/safe-performance.js';
 
 /**
  * @augments EventDispatcher<import('cross-frame-api').CrossFrameAPIPortEvents>
@@ -106,7 +107,7 @@ export class CrossFrameAPIPort extends EventDispatcher {
                     return;
                 }
             }
-            performance.mark(`cross-frame-api:invoke:${action}`);
+            safePerformance.mark(`cross-frame-api:invoke:${action}`);
             try {
                 this._port.postMessage(/** @type {import('cross-frame-api').InvokeMessage} */ ({type: 'invoke', id, data: {action, params}}));
             } catch (e) {

diff --git a/ext/js/core/promise-animation-frame.js b/ext/js/core/promise-animation-frame.js
@@ -16,6 +16,8 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
+import {safePerformance} from './safe-performance.js';
+
 /**
  * Creates a promise that will resolve after the next animation frame, using `requestAnimationFrame`.
  * @param {number} [timeout] A maximum duration (in milliseconds) to wait until the promise resolves. If null or omitted, no timeout is used.
@@ -51,7 +53,7 @@ export function promiseAnimationFrame(timeout) {
                 cancelAnimationFrame(frameRequest);
                 frameRequest = null;
             }
-            resolve({time: performance.now(), timeout: true});
+            resolve({time: safePerformance.now(), timeout: true});
         };
 
         frameRequest = requestAnimationFrame(onFrame);

diff --git a/ext/js/core/safe-performance.js b/ext/js/core/safe-performance.js
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2024  Yomitan Authors
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+import {log} from './log.js';
+
+/**
+ * This class safely handles performance methods.
+ */
+class SafePerformance {
+    constructor() {}
+
+    /**
+     * @param {string} markName
+     * @param {PerformanceMarkOptions} [markOptions]
+     * @returns {PerformanceMark | undefined}
+     */
+    mark(markName, markOptions) {
+        try {
+            // eslint-disable-next-line no-restricted-syntax
+            return performance.mark(markName, markOptions);
+        } catch (e) {
+            log.error(e);
+        }
+    }
+
+    /**
+     *
+     * @param {string} measureName
+     * @param {string | PerformanceMeasureOptions} [startOrMeasureOptions]
+     * @param {string} [endMark]
+     * @returns {PerformanceMeasure | undefined}
+     */
+    measure(measureName, startOrMeasureOptions, endMark) {
+        try {
+            // eslint-disable-next-line no-restricted-syntax
+            return performance.measure(measureName, startOrMeasureOptions, endMark);
+        } catch (e) {
+            log.error(e);
+        }
+    }
+
+    /**
+     * @returns {DOMHighResTimeStamp}
+     */
+    now() {
+        // eslint-disable-next-line no-restricted-syntax
+        return performance.now();
+    }
+}
+
+/**
+ * This object is the default performance measurer used by the runtime.
+ */
+export const safePerformance = new SafePerformance();
diff --git a/ext/js/dictionary/dictionary-database.js b/ext/js/dictionary/dictionary-database.js
@@ -17,6 +17,7 @@
  */
 
 import {log} from '../core/log.js';
+import {safePerformance} from '../core/safe-performance.js';
 import {stringReverse} from '../core/utilities.js';
 import {Database} from '../data/database.js';
 
@@ -455,16 +456,16 @@ export class DictionaryDatabase {
      * @returns {Promise<TResult[]>}
      */
     _findMultiBulk(objectStoreName, indexNames, items, createQuery, predicate, createResult) {
-        performance.mark('findMultiBulk:start');
+        safePerformance.mark('findMultiBulk:start');
         return new Promise((resolve, reject) => {
             const itemCount = items.length;
             const indexCount = indexNames.length;
             /** @type {TResult[]} */
             const results = [];
             if (itemCount === 0 || indexCount === 0) {
                 resolve(results);
-                performance.mark('findMultiBulk:end');
-                performance.measure('findMultiBulk', 'findMultiBulk:start', 'findMultiBulk:end');
+                safePerformance.mark('findMultiBulk:end');
+                safePerformance.measure('findMultiBulk', 'findMultiBulk:start', 'findMultiBulk:end');
                 return;
             }
 

diff --git a/ext/js/display/display-generator.js b/ext/js/display/display-generator.js
@@ -17,6 +17,7 @@
  */
 
 import {ExtensionError} from '../core/extension-error.js';
+import {safePerformance} from '../core/safe-performance.js';
 import {getDisambiguations, getGroupedPronunciations, getTermFrequency, groupKanjiFrequencies, groupTermFrequencies, groupTermTags, isNonNounVerbOrAdjective} from '../dictionary/dictionary-data-util.js';
 import {HtmlTemplateCollection} from '../dom/html-template-collection.js';
 import {distributeFurigana, getKanaMorae, getPitchCategory, isCodePointKanji} from '../language/ja/japanese.js';
@@ -111,23 +112,23 @@ export class DisplayGenerator {
         node.dataset.groupedFrequencyCount = `${groupedFrequencies.length}`;
         node.dataset.primaryMatchTypes = [...primaryMatchTypes].join(' ');
 
-        performance.mark('displayGenerator:createTermEntry:createTermHeadword:start');
+        safePerformance.mark('displayGenerator:createTermEntry:createTermHeadword:start');
         for (let i = 0, ii = headwords.length; i < ii; ++i) {
             const node2 = this._createTermHeadword(headwords[i], i, pronunciations);
             node2.dataset.index = `${i}`;
             headwordsContainer.appendChild(node2);
         }
         headwordsContainer.dataset.count = `${headwords.length}`;
-        performance.mark('displayGenerator:createTermEntry:createTermHeadword:end');
-        performance.measure('displayGenerator:createTermEntry:createTermHeadword', 'displayGenerator:createTermEntry:createTermHeadword:start', 'displayGenerator:createTermEntry:createTermHeadword:end');
+        safePerformance.mark('displayGenerator:createTermEntry:createTermHeadword:end');
+        safePerformance.measure('displayGenerator:createTermEntry:createTermHeadword', 'displayGenerator:createTermEntry:createTermHeadword:start', 'displayGenerator:createTermEntry:createTermHeadword:end');
 
-        performance.mark('displayGenerator:createTermEntry:promises:start');
+        safePerformance.mark('displayGenerator:createTermEntry:promises:start');
         this._appendMultiple(inflectionRuleChainsContainer, this._createInflectionRuleChain.bind(this), inflectionRuleChainCandidates);
         this._appendMultiple(frequencyGroupListContainer, this._createFrequencyGroup.bind(this), groupedFrequencies, false);
         this._appendMultiple(groupedPronunciationsContainer, this._createGroupedPronunciation.bind(this), groupedPronunciations);
         this._appendMultiple(headwordTagsContainer, this._createTermTag.bind(this), termTags, headwords.length);
-        performance.mark('displayGenerator:createTermEntry:promises:end');
-        performance.measure('displayGenerator:createTermEntry:promises', 'displayGenerator:createTermEntry:promises:start', 'displayGenerator:createTermEntry:promises:end');
+        safePerformance.mark('displayGenerator:createTermEntry:promises:end');
+        safePerformance.measure('displayGenerator:createTermEntry:promises', 'displayGenerator:createTermEntry:promises:start', 'displayGenerator:createTermEntry:promises:end');
 
         for (const term of uniqueTerms) {
             headwordTagsContainer.appendChild(this._createSearchTag(term));