From 8b68504b3b95d0647b4305a43d7b326b801e1936 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Fri, 29 Dec 2023 00:02:14 -0500 Subject: Display API safety (#479) * Remove unused * Add TODOs * Rename * Initial setup * More updates * Simplify naming * Set up window API * Remove type * Move type * Update type --- ext/js/app/popup-proxy.js | 2 + ext/js/app/popup-window.js | 9 +-- ext/js/app/popup.js | 20 ++++--- ext/js/display/display-audio.js | 4 +- ext/js/display/display.js | 108 ++++++++++++++++++------------------ types/ext/display.d.ts | 118 +++++++++++++++++++++++++++++++++++----- 6 files changed, 178 insertions(+), 83 deletions(-) diff --git a/ext/js/app/popup-proxy.js b/ext/js/app/popup-proxy.js index d141d35b..2821d774 100644 --- a/ext/js/app/popup-proxy.js +++ b/ext/js/app/popup-proxy.js @@ -297,6 +297,7 @@ export class PopupProxy extends EventDispatcher { // Private + // TODO : Type safety /** * @template {import('core').SerializableObject} TParams * @template [TReturn=unknown] @@ -308,6 +309,7 @@ export class PopupProxy extends EventDispatcher { return yomitan.crossFrame.invoke(this._frameId, action, params); } + // TODO : Type safety /** * @template {import('core').SerializableObject} TParams * @template [TReturn=unknown] diff --git a/ext/js/app/popup-window.js b/ext/js/app/popup-window.js index 0b083d80..801afb3f 100644 --- a/ext/js/app/popup-window.js +++ b/ext/js/app/popup-window.js @@ -126,7 +126,7 @@ export class PopupWindow extends EventDispatcher { * @returns {Promise} */ async setOptionsContext(optionsContext) { - await this._invoke(false, 'Display.setOptionsContext', {id: this._id, optionsContext}); + await this._invoke(false, 'displaySetOptionsContext', {id: this._id, optionsContext}); } /** @@ -183,7 +183,7 @@ export class PopupWindow extends EventDispatcher { */ async showContent(_details, displayDetails) { if (displayDetails === null) { return; } - await this._invoke(true, 'Display.setContent', {id: this._id, details: displayDetails}); + await this._invoke(true, 'displaySetContent', {id: this._id, details: displayDetails}); } /** @@ -192,7 +192,7 @@ export class PopupWindow extends EventDispatcher { * @returns {Promise} */ async setCustomCss(css) { - await this._invoke(false, 'Display.setCustomCss', {id: this._id, css}); + await this._invoke(false, 'displaySetCustomCss', {id: this._id, css}); } /** @@ -200,7 +200,7 @@ export class PopupWindow extends EventDispatcher { * @returns {Promise} */ async clearAutoPlayTimer() { - await this._invoke(false, 'Display.clearAutoPlayTimer', {id: this._id}); + await this._invoke(false, 'displayAudioClearAutoPlayTimer', {id: this._id}); } /** @@ -266,6 +266,7 @@ export class PopupWindow extends EventDispatcher { // Private + // TODO : Type safety /** * @template {import('core').SerializableObject} TParams * @template [TReturn=unknown] diff --git a/ext/js/app/popup.js b/ext/js/app/popup.js index 4f2368d4..a8cdf1a6 100644 --- a/ext/js/app/popup.js +++ b/ext/js/app/popup.js @@ -215,7 +215,7 @@ export class Popup extends EventDispatcher { async setOptionsContext(optionsContext) { await this._setOptionsContext(optionsContext); if (this._frameConnected) { - await this._invokeSafe('Display.setOptionsContext', {optionsContext}); + await this._invokeSafe('displaySetOptionsContext', {optionsContext}); } } @@ -299,7 +299,7 @@ export class Popup extends EventDispatcher { await this._show(sourceRects, writingMode); if (displayDetails !== null) { - this._invokeSafe('Display.setContent', {details: displayDetails}); + this._invokeSafe('displaySetContent', {details: displayDetails}); } } @@ -308,7 +308,7 @@ export class Popup extends EventDispatcher { * @param {string} css The CSS rules. */ async setCustomCss(css) { - await this._invokeSafe('Display.setCustomCss', {css}); + await this._invokeSafe('displaySetCustomCss', {css}); } /** @@ -316,7 +316,7 @@ export class Popup extends EventDispatcher { */ async clearAutoPlayTimer() { if (this._frameConnected) { - await this._invokeSafe('Display.clearAutoPlayTimer', {}); + await this._invokeSafe('displayAudioClearAutoPlayTimer', {}); } } @@ -327,7 +327,7 @@ export class Popup extends EventDispatcher { async setContentScale(scale) { this._contentScale = scale; this._frame.style.fontSize = `${scale}px`; - await this._invokeSafe('Display.setContentScale', {scale}); + await this._invokeSafe('displaySetContentScale', {scale}); } /** @@ -480,7 +480,7 @@ export class Popup extends EventDispatcher { this._frameConnected = true; // Configure - /** @type {import('display').ConfigureMessageDetails} */ + /** @type {import('display').DirectApiParams<'displayConfigure'>} */ const configureParams = { depth: this._depth, parentPopupId: this._id, @@ -489,7 +489,7 @@ export class Popup extends EventDispatcher { scale: this._contentScale, optionsContext: this._optionsContext }; - await this._invokeSafe('Display.configure', configureParams); + await this._invokeSafe('displayConfigure', configureParams); } /** @@ -657,7 +657,7 @@ export class Popup extends EventDispatcher { if (this._visibleValue === value) { return; } this._visibleValue = value; this._frame.style.setProperty('visibility', value ? 'visible' : 'hidden', 'important'); - this._invokeSafe('Display.visibilityChanged', {value}); + this._invokeSafe('displayVisibilityChanged', {value}); } /** @@ -679,6 +679,7 @@ export class Popup extends EventDispatcher { } } + // TODO : Type safety /** * @template {import('core').SerializableObject} TParams * @template [TReturn=unknown] @@ -696,6 +697,7 @@ export class Popup extends EventDispatcher { return await yomitan.crossFrame.invoke(this._frameClient.frameId, 'popupMessage', message); } + // TODO : Type safety /** * @template {import('core').SerializableObject} TParams * @template [TReturn=unknown] @@ -728,7 +730,7 @@ export class Popup extends EventDispatcher { * @returns {void} */ _onExtensionUnloaded() { - this._invokeWindow('Display.extensionUnloaded'); + this._invokeWindow('displayExtensionUnloaded'); } /** diff --git a/ext/js/display/display-audio.js b/ext/js/display/display-audio.js index cba9c43c..2df13306 100644 --- a/ext/js/display/display-audio.js +++ b/ext/js/display/display-audio.js @@ -89,7 +89,7 @@ export class DisplayAudio { ['playAudioFromSource', this._onHotkeyActionPlayAudioFromSource.bind(this)] ]); this._display.registerDirectMessageHandlers([ - ['Display.clearAutoPlayTimer', this._onMessageClearAutoPlayTimer.bind(this)] + ['displayAudioClearAutoPlayTimer', this._onMessageClearAutoPlayTimer.bind(this)] ]); /* eslint-enable no-multi-spaces */ this._display.on('optionsUpdated', this._onOptionsUpdated.bind(this)); @@ -260,7 +260,7 @@ export class DisplayAudio { this.playAudio(this._display.selectedIndex, 0, source); } - /** */ + /** @type {import('display').DirectApiHandler<'displayAudioClearAutoPlayTimer'>} */ _onMessageClearAutoPlayTimer() { this.clearAutoPlayTimer(); } diff --git a/ext/js/display/display.js b/ext/js/display/display.js index 08f640d0..e3c92ee2 100644 --- a/ext/js/display/display.js +++ b/ext/js/display/display.js @@ -18,7 +18,9 @@ import {ThemeController} from '../app/theme-controller.js'; import {FrameEndpoint} from '../comm/frame-endpoint.js'; -import {DynamicProperty, EventDispatcher, EventListenerCollection, clone, deepEqual, invokeMessageHandler, log, promiseTimeout} from '../core.js'; +import {DynamicProperty, EventDispatcher, EventListenerCollection, clone, deepEqual, log, promiseTimeout} from '../core.js'; +import {invokeApiMapHandler} from '../core/api-map.js'; +import {ExtensionError} from '../core/extension-error.js'; import {PopupMenu} from '../dom/popup-menu.js'; import {querySelectorNotNull} from '../dom/query-selector.js'; import {ScrollElement} from '../dom/scroll-element.js'; @@ -87,12 +89,10 @@ export class Display extends EventDispatcher { contentManager: this._contentManager, hotkeyHelpController: this._hotkeyHelpController }); - /** @type {import('core').MessageHandlerMap} */ - this._messageHandlers = new Map(); - /** @type {import('core').MessageHandlerMap} */ - this._directMessageHandlers = new Map(); - /** @type {import('core').MessageHandlerMap} */ - this._windowMessageHandlers = new Map(); + /** @type {import('display').DirectApiMap} */ + this._directApiMap = new Map(); + /** @type {import('display').WindowApiMap} */ + this._windowApiMap = new Map(); /** @type {DisplayHistory} */ this._history = new DisplayHistory({clearable: true, useBrowserHistory: false}); /** @type {boolean} */ @@ -207,15 +207,15 @@ export class Display extends EventDispatcher { ['previousEntryDifferentDictionary', () => { this._focusEntryWithDifferentDictionary(-1, true); }] ]); this.registerDirectMessageHandlers([ - ['Display.setOptionsContext', this._onMessageSetOptionsContext.bind(this)], - ['Display.setContent', this._onMessageSetContent.bind(this)], - ['Display.setCustomCss', this._onMessageSetCustomCss.bind(this)], - ['Display.setContentScale', this._onMessageSetContentScale.bind(this)], - ['Display.configure', this._onMessageConfigure.bind(this)], - ['Display.visibilityChanged', this._onMessageVisibilityChanged.bind(this)] + ['displaySetOptionsContext', this._onMessageSetOptionsContext.bind(this)], + ['displaySetContent', this._onMessageSetContent.bind(this)], + ['displaySetCustomCss', this._onMessageSetCustomCss.bind(this)], + ['displaySetContentScale', this._onMessageSetContentScale.bind(this)], + ['displayConfigure', this._onMessageConfigure.bind(this)], + ['displayVisibilityChanged', this._onMessageVisibilityChanged.bind(this)] ]); this.registerWindowMessageHandlers([ - ['Display.extensionUnloaded', this._onMessageExtensionUnloaded.bind(this)] + ['displayExtensionUnloaded', this._onMessageExtensionUnloaded.bind(this)] ]); /* eslint-enable no-multi-spaces */ } @@ -504,20 +504,20 @@ export class Display extends EventDispatcher { } /** - * @param {import('core').MessageHandlerMapInit} handlers + * @param {import('display').DirectApiMapInit} handlers */ registerDirectMessageHandlers(handlers) { for (const [name, handlerInfo] of handlers) { - this._directMessageHandlers.set(name, handlerInfo); + this._directApiMap.set(name, handlerInfo); } } /** - * @param {import('core').MessageHandlerMapInit} handlers + * @param {import('display').WindowApiMapInit} handlers */ registerWindowMessageHandlers(handlers) { for (const [name, handlerInfo] of handlers) { - this._windowMessageHandlers.set(name, handlerInfo); + this._windowApiMap.set(name, handlerInfo); } } @@ -635,22 +635,35 @@ export class Display extends EventDispatcher { // Message handlers /** - * @param {import('frame-client').Message} data - * @returns {import('core').MessageHandlerResult} + * @param {import('frame-client').Message} data + * @returns {Promise} * @throws {Error} */ _onDirectMessage(data) { - const {action, params} = this._authenticateMessageData(data); - const handler = this._directMessageHandlers.get(action); - if (typeof handler === 'undefined') { - throw new Error(`Invalid action: ${action}`); - } - - return handler(params); + return new Promise((resolve, reject) => { + const {action, params} = this._authenticateMessageData(data); + invokeApiMapHandler( + this._directApiMap, + action, + params, + [], + (result) => { + const {error} = result; + if (typeof error !== 'undefined') { + reject(ExtensionError.deserialize(error)); + } else { + resolve(result.result); + } + }, + () => { + reject(new Error(`Invalid action: ${action}`)); + } + ); + }); } /** - * @param {MessageEvent>} details + * @param {MessageEvent>} details */ _onWindowMessage({data}) { let data2; @@ -660,46 +673,37 @@ export class Display extends EventDispatcher { return; } - const {action, params} = data2; - const messageHandler = this._windowMessageHandlers.get(action); - if (typeof messageHandler === 'undefined') { return; } - - const callback = () => {}; // NOP - invokeMessageHandler(messageHandler, params, callback); + try { + const {action, params} = data2; + const callback = () => {}; // NOP + invokeApiMapHandler(this._directApiMap, action, params, [], callback); + } catch (e) { + // NOP + } } - /** - * @param {{optionsContext: import('settings').OptionsContext}} details - */ + /** @type {import('display').DirectApiHandler<'displaySetOptionsContext'>} */ async _onMessageSetOptionsContext({optionsContext}) { await this.setOptionsContext(optionsContext); this.searchLast(true); } - /** - * @param {{details: import('display').ContentDetails}} details - */ + /** @type {import('display').DirectApiHandler<'displaySetContent'>} */ _onMessageSetContent({details}) { this.setContent(details); } - /** - * @param {{css: string}} details - */ + /** @type {import('display').DirectApiHandler<'displaySetCustomCss'>} */ _onMessageSetCustomCss({css}) { this.setCustomCss(css); } - /** - * @param {{scale: number}} details - */ + /** @type {import('display').DirectApiHandler<'displaySetContentScale'>} */ _onMessageSetContentScale({scale}) { this._setContentScale(scale); } - /** - * @param {import('display').ConfigureMessageDetails} details - */ + /** @type {import('display').DirectApiHandler<'displayConfigure'>} */ async _onMessageConfigure({depth, parentPopupId, parentFrameId, childrenSupported, scale, optionsContext}) { this._depth = depth; this._parentPopupId = parentPopupId; @@ -709,15 +713,13 @@ export class Display extends EventDispatcher { await this.setOptionsContext(optionsContext); } - /** - * @param {{value: boolean}} details - */ + /** @type {import('display').DirectApiHandler<'displayVisibilityChanged'>} */ _onMessageVisibilityChanged({value}) { this._frameVisible = value; this.trigger('frameVisibilityChange', {value}); } - /** */ + /** @type {import('display').WindowApiHandler<'displayExtensionUnloaded'>} */ _onMessageExtensionUnloaded() { if (yomitan.isExtensionUnloaded) { return; } yomitan.triggerExtensionUnloaded(); diff --git a/types/ext/display.d.ts b/types/ext/display.d.ts index bafb1fa1..f7c45b02 100644 --- a/types/ext/display.d.ts +++ b/types/ext/display.d.ts @@ -18,12 +18,19 @@ import type {DisplayContentManager} from '../../ext/js/display/display-content-manager'; import type {HotkeyHelpController} from '../../ext/js/input/hotkey-help-controller'; import type {JapaneseUtil} from '../../ext/js/language/sandbox/japanese-util'; -import type * as Core from './core'; import type * as Dictionary from './dictionary'; import type * as Extension from './extension'; import type * as Settings from './settings'; import type * as TextScannerTypes from './text-scanner'; import type {EventNames, EventArgument as BaseEventArgument} from './core'; +import type { + ApiMap as BaseApiMap, + ApiParams as BaseApiParams, + ApiNames as BaseApiNames, + ApiMapInit as BaseApiMapInit, + ApiParamsAny as BaseApiParamsAny, + ApiHandler as BaseApiHandler, +} from './api-map'; export type HistoryMode = 'clear' | 'overwrite' | 'new'; @@ -159,22 +166,103 @@ export type Events = { export type EventArgument> = BaseEventArgument; -export type ConfigureMessageDetails = { - depth: number; - parentPopupId: string; - parentFrameId: number; - childrenSupported: boolean; - scale: number; - optionsContext: Settings.OptionsContext; -}; - -export type MessageDetails = { - action: string; - params: Core.SerializableObject; -}; - export type DisplayGeneratorConstructorDetails = { japaneseUtil: JapaneseUtil; contentManager: DisplayContentManager; hotkeyHelpController?: HotkeyHelpController | null; }; + +// Direct API + +export type DirectApiSurface = { + displayAudioClearAutoPlayTimer: { + params: void; + return: void; + }; + displaySetOptionsContext: { + params: { + optionsContext: Settings.OptionsContext; + }; + return: void; + }; + displaySetContent: { + params: { + details: ContentDetails; + }; + return: void; + }; + displaySetCustomCss: { + params: { + css: string; + }; + return: void; + }; + displaySetContentScale: { + params: { + scale: number; + }; + return: void; + }; + displayConfigure: { + params: { + depth: number; + parentPopupId: string; + parentFrameId: number; + childrenSupported: boolean; + scale: number; + optionsContext: Settings.OptionsContext; + }; + return: void; + }; + displayVisibilityChanged: { + params: { + value: boolean; + }; + return: void; + }; +}; + +type DirectApiNames = BaseApiNames; + +export type DirectApiMapInit = BaseApiMapInit; + +export type DirectApiMap = BaseApiMap; + +export type DirectApiHandler = BaseApiHandler; + +type DirectApiParams = BaseApiParams; + +export type DirectApiMessageAny = {[name in DirectApiNames]: DirectApiMessage}[DirectApiNames]; + +export type DirectApiReturnAny = BaseApiParamsAny; + +type DirectApiMessage = { + action: TName; + params: DirectApiParams; +}; + +// Window API + +export type WindowApiSurface = { + 'displayExtensionUnloaded': { + params: void; + return: void; + }; +}; + +type WindowApiNames = BaseApiNames; + +export type WindowApiMapInit = BaseApiMapInit; + +export type WindowApiMap = BaseApiMap; + +export type WindowApiHandler = BaseApiHandler; + +type WindowApiParams = BaseApiParams; + +export type WindowApiMessageAny = {[name in WindowApiNames]: WindowApiMessage}[WindowApiNames]; + +type WindowApiMessage = { + action: TName; + params: WindowApiParams; +}; -- cgit v1.2.3