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

@@ -17,11 +17,11 @@
 
 
 class BackendApiForwarder {
-    constructor() {
+    prepare() {
-        chrome.runtime.onConnect.addListener(this.onConnect.bind(this));
+        chrome.runtime.onConnect.addListener(this._onConnect.bind(this));
     }
 
-    onConnect(port) {
+    _onConnect(port) {
         if (port.name !== 'backend-api-forwarder') { return; }
 
         let tabId;

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);
 

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);
+        this._sendAck(port, id, senderId);
-
+        this._invokeHandler(handler, params, port, id, senderId);
-        handler(params).then(
-            (result) => {
-                this.sendResult(port, id, senderId, {result});
-            },
-            (error) => {
-                this.sendResult(port, id, senderId, {error: errorToJson(error)});
-            });
     }
 
-    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});
     }
 }

ext/fg/js/frontend-api-sender.js

@@ -18,68 +18,67 @@
 
 class FrontendApiSender {
     constructor() {
-        this.senderId = yomichan.generateId(16);
+        this._senderId = yomichan.generateId(16);
-        this.ackTimeout = 3000; // 3 seconds
+        this._ackTimeout = 3000; // 3 seconds
-        this.responseTimeout = 10000; // 10 seconds
+        this._responseTimeout = 10000; // 10 seconds
-        this.callbacks = new Map();
+        this._callbacks = new Map();
-        this.disconnected = false;
+        this._disconnected = false;
-        this.nextId = 0;
+        this._nextId = 0;
-
+        this._port = null;
-        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) {
+        if (this._port === null) {
-            this.createPort();
+            this._createPort();
         }
 
-        const id = `${this.nextId}`;
+        const id = `${this._nextId}`;
-        ++this.nextId;
+        ++this._nextId;
 
         return new Promise((resolve, reject) => {
             const info = {id, resolve, reject, ack: false, timer: null};
-            this.callbacks.set(id, info);
+            this._callbacks.set(id, info);
-            info.timer = setTimeout(() => this.onError(id, 'Timeout (ack)'), this.ackTimeout);
+            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() {
+    _createPort() {
-        this.port = chrome.runtime.connect(null, {name: 'backend-api-forwarder'});
+        this._port = chrome.runtime.connect(null, {name: 'backend-api-forwarder'});
-        this.port.onDisconnect.addListener(this.onDisconnect.bind(this));
+        this._port.onDisconnect.addListener(this._onDisconnect.bind(this));
-        this.port.onMessage.addListener(this.onMessage.bind(this));
+        this._port.onMessage.addListener(this._onMessage.bind(this));
     }
 
-    onMessage({type, id, data, senderId}) {
+    _onMessage({type, id, data, senderId}) {
-        if (senderId !== this.senderId) { return; }
+        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() {
+    _onDisconnect() {
-        this.disconnected = true;
+        this._disconnected = true;
-        this.port = null;
+        this._port = null;
 
-        for (const id of this.callbacks.keys()) {
+        for (const id of this._callbacks.keys()) {
-            this.onError(id, 'Disconnected');
+            this._onError(id, 'Disconnected');
         }
     }
 
-    onAck(id) {
+    _onAck(id) {
-        const info = this.callbacks.get(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) {
+    _onResult(id, data) {
-        const info = this.callbacks.get(id);
+        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) {
+    _onError(id, reason) {
-        const info = this.callbacks.get(id);
+        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));
     }

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) {