Skip to content

Commit

Permalink
Display API safety (#479)
Browse files Browse the repository at this point in the history 
* Remove unused

* Add TODOs

* Rename

* Initial setup

* More updates

* Simplify naming

* Set up window API

* Remove type

* Move type

* Update type
  • Loading branch information
@toasted-nutbread
toasted-nutbread authored Dec 29, 2023
1 parent 6a072df commit 8b68504
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 83 deletions.
2 changes: 2 additions & 0 deletions ext/js/app/popup-proxy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ export class PopupProxy extends EventDispatcher {

// Private

// TODO : Type safety
/**
* @template {import('core').SerializableObject} TParams
* @template [TReturn=unknown]
Expand All @@ -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]
Expand Down
9 changes: 5 additions & 4 deletions ext/js/app/popup-window.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class PopupWindow extends EventDispatcher {
* @returns {Promise<void>}
*/
async setOptionsContext(optionsContext) {
await this._invoke(false, 'Display.setOptionsContext', {id: this._id, optionsContext});
await this._invoke(false, 'displaySetOptionsContext', {id: this._id, optionsContext});
}

/**
Expand Down Expand Up @@ -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});
}

/**
Expand All @@ -192,15 +192,15 @@ export class PopupWindow extends EventDispatcher {
* @returns {Promise<void>}
*/
async setCustomCss(css) {
await this._invoke(false, 'Display.setCustomCss', {id: this._id, css});
await this._invoke(false, 'displaySetCustomCss', {id: this._id, css});
}

/**
* Stops the audio auto-play timer, if one has started.
* @returns {Promise<void>}
*/
async clearAutoPlayTimer() {
await this._invoke(false, 'Display.clearAutoPlayTimer', {id: this._id});
await this._invoke(false, 'displayAudioClearAutoPlayTimer', {id: this._id});
}

/**
Expand Down Expand Up @@ -266,6 +266,7 @@ export class PopupWindow extends EventDispatcher {

// Private

// TODO : Type safety
/**
* @template {import('core').SerializableObject} TParams
* @template [TReturn=unknown]
Expand Down
20 changes: 11 additions & 9 deletions ext/js/app/popup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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});
}
}

Expand Down Expand Up @@ -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});
}
}

Expand All @@ -308,15 +308,15 @@ 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});
}

/**
* Stops the audio auto-play timer, if one has started.
*/
async clearAutoPlayTimer() {
if (this._frameConnected) {
await this._invokeSafe('Display.clearAutoPlayTimer', {});
await this._invokeSafe('displayAudioClearAutoPlayTimer', {});
}
}

Expand All @@ -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});
}

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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});
}

/**
Expand All @@ -679,6 +679,7 @@ export class Popup extends EventDispatcher {
}
}

// TODO : Type safety
/**
* @template {import('core').SerializableObject} TParams
* @template [TReturn=unknown]
Expand All @@ -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]
Expand Down Expand Up @@ -728,7 +730,7 @@ export class Popup extends EventDispatcher {
* @returns {void}
*/
_onExtensionUnloaded() {
this._invokeWindow('Display.extensionUnloaded');
this._invokeWindow('displayExtensionUnloaded');
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ext/js/display/display-audio.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -260,7 +260,7 @@ export class DisplayAudio {
this.playAudio(this._display.selectedIndex, 0, source);
}

/** */
/** @type {import('display').DirectApiHandler<'displayAudioClearAutoPlayTimer'>} */
_onMessageClearAutoPlayTimer() {
this.clearAutoPlayTimer();
}
Expand Down
108 changes: 55 additions & 53 deletions ext/js/display/display.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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} */
Expand Down Expand Up @@ -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 */
}
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -635,22 +635,35 @@ export class Display extends EventDispatcher {
// Message handlers

/**
* @param {import('frame-client').Message<import('display').MessageDetails>} data
* @returns {import('core').MessageHandlerResult}
* @param {import('frame-client').Message<import('display').DirectApiMessageAny>} data
* @returns {Promise<import('display').DirectApiReturnAny>}
* @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<import('frame-client').Message<import('display').MessageDetails>>} details
* @param {MessageEvent<import('frame-client').Message<import('display').WindowApiMessageAny>>} details
*/
_onWindowMessage({data}) {
let data2;
Expand All @@ -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;
Expand All @@ -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();
Expand Down
Loading

0 comments on commit 8b68504

Please sign in to comment.