Cross frame API safety (#491)

* Require error type

* Add TODOs

* Fix init

* Updates

* More type safety

* Fix incorrect API map

* Update type safety

* Updates

* Add API

* Update types

* Update types

* Updates

* Remove unused

* Restore types

* Update frame ancestry handler

* Simplify names

* Fix

* Remove old message handlers

3 files changed, 73 insertions, 83 deletions

diff --git a/ext/js/comm/cross-frame-api.js b/ext/js/comm/cross-frame-api.js
index 14e410a8..3569c037 100644
--- a/ext/js/comm/cross-frame-api.js
+++ b/ext/js/comm/cross-frame-api.js
@@ -16,7 +16,8 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-import {EventDispatcher, EventListenerCollection, invokeMessageHandler, log} from '../core.js';
+import {EventDispatcher, EventListenerCollection, log} from '../core.js';
+import {extendApiMap, invokeApiMapHandler} from '../core/api-map.js';
 import {ExtensionError} from '../core/extension-error.js';
 import {parseJson} from '../core/json.js';
 import {yomitan} from '../yomitan.js';
@@ -29,9 +30,9 @@ export class CrossFrameAPIPort extends EventDispatcher {
      * @param {number} otherTabId
      * @param {number} otherFrameId
      * @param {chrome.runtime.Port} port
-     * @param {import('core').MessageHandlerMap} messageHandlers
+     * @param {import('cross-frame-api').ApiMap} apiMap
      */
-    constructor(otherTabId, otherFrameId, port, messageHandlers) {
+    constructor(otherTabId, otherFrameId, port, apiMap) {
         super();
         /** @type {number} */
         this._otherTabId = otherTabId;
@@ -39,8 +40,8 @@ export class CrossFrameAPIPort extends EventDispatcher {
         this._otherFrameId = otherFrameId;
         /** @type {?chrome.runtime.Port} */
         this._port = port;
-        /** @type {import('core').MessageHandlerMap} */
-        this._messageHandlers = messageHandlers;
+        /** @type {import('cross-frame-api').ApiMap} */
+        this._apiMap = apiMap;
         /** @type {Map<number, import('cross-frame-api').Invocation>} */
         this._activeInvocations = new Map();
         /** @type {number} */
@@ -69,13 +70,12 @@ export class CrossFrameAPIPort extends EventDispatcher {
     }
 
     /**
-     * @template [TParams=import('core').SerializableObject]
-     * @template [TReturn=unknown]
-     * @param {string} action
-     * @param {TParams} params
+     * @template {import('cross-frame-api').ApiNames} TName
+     * @param {TName} action
+     * @param {import('cross-frame-api').ApiParams<TName>} params
      * @param {number} ackTimeout
      * @param {number} responseTimeout
-     * @returns {Promise<TReturn>}
+     * @returns {Promise<import('cross-frame-api').ApiReturn<TName>>}
      */
     invoke(action, params, ackTimeout, responseTimeout) {
         return new Promise((resolve, reject) => {
@@ -186,7 +186,7 @@ export class CrossFrameAPIPort extends EventDispatcher {
 
     /**
      * @param {number} id
-     * @param {import('core').Response<unknown>} data
+     * @param {import('core').Response<import('cross-frame-api').ApiReturnAny>} data
      */
     _onResult(id, data) {
         const invocation = this._activeInvocations.get(id);
@@ -217,15 +217,13 @@ export class CrossFrameAPIPort extends EventDispatcher {
 
     /**
      * @param {number} id
-     * @param {unknown} error
+     * @param {unknown} errorOrMessage
      */
-    _onError(id, error) {
+    _onError(id, errorOrMessage) {
         const invocation = this._activeInvocations.get(id);
         if (typeof invocation === 'undefined') { return; }
 
-        if (!(error instanceof Error)) {
-            error = new Error(`${error} (${invocation.action})`);
-        }
+        const error = errorOrMessage instanceof Error ? errorOrMessage : new Error(`${errorOrMessage} (${invocation.action})`);
 
         this._activeInvocations.delete(id);
         if (invocation.timer !== null) {
@@ -239,23 +237,18 @@ export class CrossFrameAPIPort extends EventDispatcher {
 
     /**
      * @param {number} id
-     * @param {import('cross-frame-api').InvocationData} details
-     * @returns {boolean}
+     * @param {import('cross-frame-api').ApiMessageAny} details
      */
     _onInvoke(id, {action, params}) {
-        const messageHandler = this._messageHandlers.get(action);
         this._sendAck(id);
-        if (typeof messageHandler === 'undefined') {
-            this._sendError(id, new Error(`Unknown action: ${action}`));
-            return false;
-        }
-
-        /**
-         * @param {import('core').Response<unknown>} data
-         * @returns {void}
-         */
-        const callback = (data) => this._sendResult(id, data);
-        return invokeMessageHandler(messageHandler, params, callback);
+        invokeApiMapHandler(
+            this._apiMap,
+            action,
+            params,
+            [],
+            (data) => this._sendResult(id, data),
+            () => this._sendError(id, new Error(`Unknown action: ${action}`))
+        );
     }
 
     /**
@@ -279,7 +272,7 @@ export class CrossFrameAPIPort extends EventDispatcher {
 
     /**
      * @param {number} id
-     * @param {import('core').Response<unknown>} data
+     * @param {import('core').Response<import('cross-frame-api').ApiReturnAny>} data
      */
     _sendResult(id, data) {
         this._sendResponse({type: 'result', id, data});
@@ -302,8 +295,8 @@ export class CrossFrameAPI {
         this._responseTimeout = 10000; // 10 seconds
         /** @type {Map<number, Map<number, CrossFrameAPIPort>>} */
         this._commPorts = new Map();
-        /** @type {import('core').MessageHandlerMap} */
-        this._messageHandlers = new Map();
+        /** @type {import('cross-frame-api').ApiMap} */
+        this._apiMap = new Map();
         /** @type {(port: CrossFrameAPIPort) => void} */
         this._onDisconnectBind = this._onDisconnect.bind(this);
         /** @type {?number} */
@@ -319,25 +312,23 @@ export class CrossFrameAPI {
     }
 
     /**
-     * @template [TParams=import('core').SerializableObject]
-     * @template [TReturn=unknown]
+     * @template {import('cross-frame-api').ApiNames} TName
      * @param {number} targetFrameId
-     * @param {string} action
-     * @param {TParams} params
-     * @returns {Promise<TReturn>}
+     * @param {TName} action
+     * @param {import('cross-frame-api').ApiParams<TName>} params
+     * @returns {Promise<import('cross-frame-api').ApiReturn<TName>>}
      */
     invoke(targetFrameId, action, params) {
         return this.invokeTab(null, targetFrameId, action, params);
     }
 
     /**
-     * @template [TParams=import('core').SerializableObject]
-     * @template [TReturn=unknown]
+     * @template {import('cross-frame-api').ApiNames} TName
      * @param {?number} targetTabId
      * @param {number} targetFrameId
-     * @param {string} action
-     * @param {TParams} params
-     * @returns {Promise<TReturn>}
+     * @param {TName} action
+     * @param {import('cross-frame-api').ApiParams<TName>} params
+     * @returns {Promise<import('cross-frame-api').ApiReturn<TName>>}
      */
     async invokeTab(targetTabId, targetFrameId, action, params) {
         if (typeof targetTabId !== 'number') {
@@ -351,24 +342,10 @@ export class CrossFrameAPI {
     }
 
     /**
-     * @param {import('core').MessageHandlerMapInit} messageHandlers
-     * @throws {Error}
-     */
-    registerHandlers(messageHandlers) {
-        for (const [key, value] of messageHandlers) {
-            if (this._messageHandlers.has(key)) {
-                throw new Error(`Handler ${key} is already registered`);
-            }
-            this._messageHandlers.set(key, value);
-        }
-    }
-
-    /**
-     * @param {string} key
-     * @returns {boolean}
+     * @param {import('cross-frame-api').ApiMapInit} handlers
      */
-    unregisterHandler(key) {
-        return this._messageHandlers.delete(key);
+    registerHandlers(handlers) {
+        extendApiMap(this._apiMap, handlers);
     }
 
     // Private
@@ -451,7 +428,7 @@ export class CrossFrameAPI {
      * @returns {CrossFrameAPIPort}
      */
     _setupCommPort(otherTabId, otherFrameId, port) {
-        const commPort = new CrossFrameAPIPort(otherTabId, otherFrameId, port, this._messageHandlers);
+        const commPort = new CrossFrameAPIPort(otherTabId, otherFrameId, port, this._apiMap);
         let tabPorts = this._commPorts.get(otherTabId);
         if (typeof tabPorts === 'undefined') {
             tabPorts = new Map();
diff --git a/ext/js/comm/frame-ancestry-handler.js b/ext/js/comm/frame-ancestry-handler.js
index 261ea943..e715cedc 100644
--- a/ext/js/comm/frame-ancestry-handler.js
+++ b/ext/js/comm/frame-ancestry-handler.js
@@ -37,12 +37,12 @@ export class FrameAncestryHandler {
         this._isPrepared = false;
         /** @type {string} */
         this._requestMessageId = 'FrameAncestryHandler.requestFrameInfo';
-        /** @type {string} */
-        this._responseMessageIdBase = `${this._requestMessageId}.response.`;
         /** @type {?Promise<number[]>} */
         this._getFrameAncestryInfoPromise = null;
         /** @type {Map<number, {window: Window, frameElement: ?(undefined|Element)}>} */
         this._childFrameMap = new Map();
+        /** @type {Map<string, import('frame-ancestry-handler').ResponseHandler>} */
+        this._responseHandlers = new Map();
     }
 
     /**
@@ -59,6 +59,9 @@ export class FrameAncestryHandler {
     prepare() {
         if (this._isPrepared) { return; }
         window.addEventListener('message', this._onWindowMessage.bind(this), false);
+        yomitan.crossFrame.registerHandlers([
+            ['frameAncestryHandlerRequestFrameInfoResponse', this._onFrameAncestryHandlerRequestFrameInfoResponse.bind(this)]
+        ]);
         this._isPrepared = true;
     }
 
@@ -119,7 +122,6 @@ export class FrameAncestryHandler {
 
             const uniqueId = generateId(16);
             let nonce = generateId(16);
-            const responseMessageId = `${this._responseMessageIdBase}${uniqueId}`;
             /** @type {number[]} */
             const results = [];
             /** @type {?import('core').Timeout} */
@@ -130,12 +132,9 @@ export class FrameAncestryHandler {
                     clearTimeout(timer);
                     timer = null;
                 }
-                yomitan.crossFrame.unregisterHandler(responseMessageId);
+                this._removeResponseHandler(uniqueId);
             };
-            /**
-             * @param {import('frame-ancestry-handler').RequestFrameInfoResponseParams} params
-             * @returns {?import('frame-ancestry-handler').RequestFrameInfoResponseReturn}
-             */
+            /** @type {import('frame-ancestry-handler').ResponseHandler} */
             const onMessage = (params) => {
                 if (params.nonce !== nonce) { return null; }
 
@@ -164,9 +163,7 @@ export class FrameAncestryHandler {
             };
 
             // Start
-            yomitan.crossFrame.registerHandlers([
-                [responseMessageId, onMessage]
-            ]);
+            this._addResponseHandler(uniqueId, onMessage);
             resetTimeout();
             const frameId = this._frameId;
             this._requestFrameInfo(targetWindow, frameId, frameId, uniqueId, nonce);
@@ -212,13 +209,9 @@ export class FrameAncestryHandler {
             const frameId = this._frameId;
             const {parent} = window;
             const more = (window !== parent);
-            /** @type {import('frame-ancestry-handler').RequestFrameInfoResponseParams} */
-            const responseParams = {frameId, nonce, more};
-            const responseMessageId = `${this._responseMessageIdBase}${uniqueId}`;
 
             try {
-                /** @type {?import('frame-ancestry-handler').RequestFrameInfoResponseReturn} */
-                const response = await yomitan.crossFrame.invoke(originFrameId, responseMessageId, responseParams);
+                const response = await yomitan.crossFrame.invoke(originFrameId, 'frameAncestryHandlerRequestFrameInfoResponse', {uniqueId, frameId, nonce, more});
                 if (response === null) { return; }
                 const nonce2 = response.nonce;
                 if (typeof nonce2 !== 'string') { return; }
@@ -317,4 +310,27 @@ export class FrameAncestryHandler {
         // Not found
         return null;
     }
+
+    /**
+     * @param {string} id
+     * @param {import('frame-ancestry-handler').ResponseHandler} handler
+     * @throws {Error}
+     */
+    _addResponseHandler(id, handler) {
+        if (this._responseHandlers.has(id)) { throw new Error('Identifier already used'); }
+        this._responseHandlers.set(id, handler);
+    }
+
+    /**
+     * @param {string} id
+     */
+    _removeResponseHandler(id) {
+        this._responseHandlers.delete(id);
+    }
+
+    /** @type {import('cross-frame-api').ApiHandler<'frameAncestryHandlerRequestFrameInfoResponse'>} */
+    _onFrameAncestryHandlerRequestFrameInfoResponse(params) {
+        const handler = this._responseHandlers.get(params.uniqueId);
+        return typeof handler !== 'undefined' ? handler(params) : null;
+    }
 }
diff --git a/ext/js/comm/frame-offset-forwarder.js b/ext/js/comm/frame-offset-forwarder.js
index afa6a5e6..570f3e88 100644
--- a/ext/js/comm/frame-offset-forwarder.js
+++ b/ext/js/comm/frame-offset-forwarder.js
@@ -36,7 +36,7 @@ export class FrameOffsetForwarder {
     prepare() {
         this._frameAncestryHandler.prepare();
         yomitan.crossFrame.registerHandlers([
-            ['FrameOffsetForwarder.getChildFrameRect', this._onMessageGetChildFrameRect.bind(this)]
+            ['frameOffsetForwarderGetChildFrameRect', this._onMessageGetChildFrameRect.bind(this)]
         ]);
     }
 
@@ -55,7 +55,7 @@ export class FrameOffsetForwarder {
             /** @type {Promise<?import('frame-offset-forwarder').ChildFrameRect>[]} */
             const promises = [];
             for (const frameId of ancestorFrameIds) {
-                promises.push(yomitan.crossFrame.invoke(frameId, 'FrameOffsetForwarder.getChildFrameRect', {frameId: childFrameId}));
+                promises.push(yomitan.crossFrame.invoke(frameId, 'frameOffsetForwarderGetChildFrameRect', {frameId: childFrameId}));
                 childFrameId = frameId;
             }
 
@@ -76,10 +76,7 @@ export class FrameOffsetForwarder {
 
     // Private
 
-    /**
-     * @param {{frameId: number}} event
-     * @returns {?import('frame-offset-forwarder').ChildFrameRect}
-     */
+    /** @type {import('cross-frame-api').ApiHandler<'frameOffsetForwarderGetChildFrameRect'>} */
     _onMessageGetChildFrameRect({frameId}) {
         const frameElement = this._frameAncestryHandler.getChildFrameElement(frameId);
         if (frameElement === null) { return null; }