From 6c341a13d813fc63b76fbbffe7920eeaf116d3a8 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sat, 2 May 2020 12:50:44 -0400 Subject: [PATCH] Refactor frontend API classes (#482) * Mark functions as private * Remove constructor side effects * Use safer handler invocation * Mark functions as private * Mark fields as private * Update BackendApiForwarder public API --- ext/bg/js/backend-api-forwarder.js | 6 +-- ext/bg/js/backend.js | 3 +- ext/fg/js/frontend-api-receiver.js | 36 +++++++------- ext/fg/js/frontend-api-sender.js | 77 +++++++++++++++--------------- ext/fg/js/popup-proxy-host.js | 4 +- 5 files changed, 65 insertions(+), 61 deletions(-) diff --git a/ext/bg/js/backend-api-forwarder.js b/ext/bg/js/backend-api-forwarder.js index 93db77d7..4ac12730 100644 --- a/ext/bg/js/backend-api-forwarder.js +++ b/ext/bg/js/backend-api-forwarder.js @@ -17,11 +17,11 @@ class BackendApiForwarder { - constructor() { - chrome.runtime.onConnect.addListener(this.onConnect.bind(this)); + prepare() { + chrome.runtime.onConnect.addListener(this._onConnect.bind(this)); } - onConnect(port) { + _onConnect(port) { if (port.name !== 'backend-api-forwarder') { return; } let tabId; diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js index 8a8f00eb..2fce4be9 100644 --- a/ext/bg/js/backend.js +++ b/ext/bg/js/backend.js @@ -70,7 +70,8 @@ class Backend { this.popupWindow = null; - this.apiForwarder = new BackendApiForwarder(); + const apiForwarder = new BackendApiForwarder(); + apiForwarder.prepare(); this.messageToken = yomichan.generateId(16); diff --git a/ext/fg/js/frontend-api-receiver.js b/ext/fg/js/frontend-api-receiver.js index 4abd4e81..c5bb58af 100644 --- a/ext/fg/js/frontend-api-receiver.js +++ b/ext/fg/js/frontend-api-receiver.js @@ -20,38 +20,42 @@ class FrontendApiReceiver { constructor(source='', handlers=new Map()) { this._source = source; this._handlers = handlers; - - chrome.runtime.onConnect.addListener(this.onConnect.bind(this)); } - onConnect(port) { + prepare() { + chrome.runtime.onConnect.addListener(this._onConnect.bind(this)); + } + + _onConnect(port) { if (port.name !== 'frontend-api-receiver') { return; } - port.onMessage.addListener(this.onMessage.bind(this, port)); + port.onMessage.addListener(this._onMessage.bind(this, port)); } - onMessage(port, {id, action, params, target, senderId}) { + _onMessage(port, {id, action, params, target, senderId}) { if (target !== this._source) { return; } const handler = this._handlers.get(action); if (typeof handler !== 'function') { return; } - this.sendAck(port, id, senderId); - - handler(params).then( - (result) => { - this.sendResult(port, id, senderId, {result}); - }, - (error) => { - this.sendResult(port, id, senderId, {error: errorToJson(error)}); - }); + this._sendAck(port, id, senderId); + this._invokeHandler(handler, params, port, id, senderId); } - sendAck(port, id, senderId) { + async _invokeHandler(handler, params, port, id, senderId) { + try { + const result = await handler(params); + this._sendResult(port, id, senderId, {result}); + } catch (error) { + this._sendResult(port, id, senderId, {error: errorToJson(error)}); + } + } + + _sendAck(port, id, senderId) { port.postMessage({type: 'ack', id, senderId}); } - sendResult(port, id, senderId, data) { + _sendResult(port, id, senderId, data) { port.postMessage({type: 'result', id, senderId, data}); } } diff --git a/ext/fg/js/frontend-api-sender.js b/ext/fg/js/frontend-api-sender.js index 0ad3f085..d5084c29 100644 --- a/ext/fg/js/frontend-api-sender.js +++ b/ext/fg/js/frontend-api-sender.js @@ -18,68 +18,67 @@ class FrontendApiSender { constructor() { - this.senderId = yomichan.generateId(16); - this.ackTimeout = 3000; // 3 seconds - this.responseTimeout = 10000; // 10 seconds - this.callbacks = new Map(); - this.disconnected = false; - this.nextId = 0; - - this.port = null; + this._senderId = yomichan.generateId(16); + this._ackTimeout = 3000; // 3 seconds + this._responseTimeout = 10000; // 10 seconds + this._callbacks = new Map(); + this._disconnected = false; + this._nextId = 0; + this._port = null; } invoke(action, params, target) { - if (this.disconnected) { + if (this._disconnected) { // attempt to reconnect the next time - this.disconnected = false; + this._disconnected = false; return Promise.reject(new Error('Disconnected')); } - if (this.port === null) { - this.createPort(); + if (this._port === null) { + this._createPort(); } - const id = `${this.nextId}`; - ++this.nextId; + const id = `${this._nextId}`; + ++this._nextId; return new Promise((resolve, reject) => { const info = {id, resolve, reject, ack: false, timer: null}; - this.callbacks.set(id, info); - info.timer = setTimeout(() => this.onError(id, 'Timeout (ack)'), this.ackTimeout); + this._callbacks.set(id, info); + info.timer = setTimeout(() => this._onError(id, 'Timeout (ack)'), this._ackTimeout); - this.port.postMessage({id, action, params, target, senderId: this.senderId}); + this._port.postMessage({id, action, params, target, senderId: this._senderId}); }); } - createPort() { - this.port = chrome.runtime.connect(null, {name: 'backend-api-forwarder'}); - this.port.onDisconnect.addListener(this.onDisconnect.bind(this)); - this.port.onMessage.addListener(this.onMessage.bind(this)); + _createPort() { + this._port = chrome.runtime.connect(null, {name: 'backend-api-forwarder'}); + this._port.onDisconnect.addListener(this._onDisconnect.bind(this)); + this._port.onMessage.addListener(this._onMessage.bind(this)); } - onMessage({type, id, data, senderId}) { - if (senderId !== this.senderId) { return; } + _onMessage({type, id, data, senderId}) { + if (senderId !== this._senderId) { return; } switch (type) { case 'ack': - this.onAck(id); + this._onAck(id); break; case 'result': - this.onResult(id, data); + this._onResult(id, data); break; } } - onDisconnect() { - this.disconnected = true; - this.port = null; + _onDisconnect() { + this._disconnected = true; + this._port = null; - for (const id of this.callbacks.keys()) { - this.onError(id, 'Disconnected'); + for (const id of this._callbacks.keys()) { + this._onError(id, 'Disconnected'); } } - onAck(id) { - const info = this.callbacks.get(id); + _onAck(id) { + const info = this._callbacks.get(id); if (typeof info === 'undefined') { yomichan.logWarning(new Error(`ID ${id} not found for ack`)); return; @@ -92,11 +91,11 @@ class FrontendApiSender { info.ack = true; clearTimeout(info.timer); - info.timer = setTimeout(() => this.onError(id, 'Timeout (response)'), this.responseTimeout); + info.timer = setTimeout(() => this._onError(id, 'Timeout (response)'), this._responseTimeout); } - onResult(id, data) { - const info = this.callbacks.get(id); + _onResult(id, data) { + const info = this._callbacks.get(id); if (typeof info === 'undefined') { yomichan.logWarning(new Error(`ID ${id} not found`)); return; @@ -107,7 +106,7 @@ class FrontendApiSender { return; } - this.callbacks.delete(id); + this._callbacks.delete(id); clearTimeout(info.timer); info.timer = null; @@ -118,10 +117,10 @@ class FrontendApiSender { } } - onError(id, reason) { - const info = this.callbacks.get(id); + _onError(id, reason) { + const info = this._callbacks.get(id); if (typeof info === 'undefined') { return; } - this.callbacks.delete(id); + this._callbacks.delete(id); info.timer = null; info.reject(new Error(reason)); } diff --git a/ext/fg/js/popup-proxy-host.js b/ext/fg/js/popup-proxy-host.js index d87553e6..39150bc7 100644 --- a/ext/fg/js/popup-proxy-host.js +++ b/ext/fg/js/popup-proxy-host.js @@ -24,7 +24,6 @@ class PopupProxyHost { constructor() { this._popups = new Map(); - this._apiReceiver = null; this._frameId = null; } @@ -35,7 +34,7 @@ class PopupProxyHost { if (typeof frameId !== 'number') { return; } this._frameId = frameId; - this._apiReceiver = new FrontendApiReceiver(`popup-proxy-host#${this._frameId}`, new Map([ + const apiReceiver = new FrontendApiReceiver(`popup-proxy-host#${this._frameId}`, new Map([ ['getOrCreatePopup', this._onApiGetOrCreatePopup.bind(this)], ['setOptionsContext', this._onApiSetOptionsContext.bind(this)], ['hide', this._onApiHide.bind(this)], @@ -48,6 +47,7 @@ class PopupProxyHost { ['setContentScale', this._onApiSetContentScale.bind(this)], ['getHostUrl', this._onApiGetHostUrl.bind(this)] ])); + apiReceiver.prepare(); } getOrCreatePopup(id=null, parentId=null, depth=null) {