Merge pull request #259 from praschke/chrome-death-fix

Fix Chrome death, by creating both cross-frame ports in the background

4 files changed, 70 insertions, 90 deletions

diff --git a/ext/js/background/backend.js b/ext/js/background/backend.js
index dd233abb..58387fd4 100644
--- a/ext/js/background/backend.js
+++ b/ext/js/background/backend.js
@@ -139,7 +139,8 @@ class Backend {
             ['textHasJapaneseCharacters',    {async: false, contentScript: true,  handler: this._onApiTextHasJapaneseCharacters.bind(this)}],
             ['getTermFrequencies',           {async: true,  contentScript: true,  handler: this._onApiGetTermFrequencies.bind(this)}],
             ['findAnkiNotes',                {async: true,  contentScript: true,  handler: this._onApiFindAnkiNotes.bind(this)}],
-            ['loadExtensionScripts',         {async: true,  contentScript: true,  handler: this._onApiLoadExtensionScripts.bind(this)}]
+            ['loadExtensionScripts',         {async: true,  contentScript: true,  handler: this._onApiLoadExtensionScripts.bind(this)}],
+            ['openCrossFramePort',           {async: false, contentScript: true,  handler: this._onApiOpenCrossFramePort.bind(this)}]
         ]);
         this._messageHandlersWithProgress = new Map([
         ]);
@@ -189,9 +190,6 @@ class Backend {
             chrome.tabs.onZoomChange.addListener(onZoomChange);
         }
 
-        const onConnect = this._onWebExtensionEventWrapper(this._onConnect.bind(this));
-        chrome.runtime.onConnect.addListener(onConnect);
-
         const onMessage = this._onMessageWrapper.bind(this);
         chrome.runtime.onMessage.addListener(onMessage);
 
@@ -331,58 +329,6 @@ class Backend {
         return invokeMessageHandler(messageHandler, params, callback, sender);
     }
 
-    _onConnect(port) {
-        try {
-            let details;
-            try {
-                details = JSON.parse(port.name);
-            } catch (e) {
-                return;
-            }
-            if (details.name !== 'background-cross-frame-communication-port') { return; }
-
-            const senderTabId = (port.sender && port.sender.tab ? port.sender.tab.id : null);
-            if (typeof senderTabId !== 'number') {
-                throw new Error('Port does not have an associated tab ID');
-            }
-            const senderFrameId = port.sender.frameId;
-            if (typeof senderFrameId !== 'number') {
-                throw new Error('Port does not have an associated frame ID');
-            }
-            let {targetTabId, targetFrameId} = details;
-            if (typeof targetTabId !== 'number') {
-                targetTabId = senderTabId;
-            }
-
-            const details2 = {
-                name: 'cross-frame-communication-port',
-                sourceTabId: senderTabId,
-                sourceFrameId: senderFrameId
-            };
-            let forwardPort = chrome.tabs.connect(targetTabId, {frameId: targetFrameId, name: JSON.stringify(details2)});
-
-            const cleanup = () => {
-                this._checkLastError(chrome.runtime.lastError);
-                if (forwardPort !== null) {
-                    forwardPort.disconnect();
-                    forwardPort = null;
-                }
-                if (port !== null) {
-                    port.disconnect();
-                    port = null;
-                }
-            };
-
-            port.onMessage.addListener((message) => { forwardPort.postMessage(message); });
-            forwardPort.onMessage.addListener((message) => { port.postMessage(message); });
-            port.onDisconnect.addListener(cleanup);
-            forwardPort.onDisconnect.addListener(cleanup);
-        } catch (e) {
-            port.disconnect();
-            log.error(e);
-        }
-    }
-
     _onZoomChange({tabId, oldZoomFactor, newZoomFactor}) {
         this._sendMessageTabIgnoreResponse(tabId, {action: 'Yomichan.zoomChanged', params: {oldZoomFactor, newZoomFactor}});
     }
@@ -2273,4 +2219,47 @@ class Backend {
         }
         return results;
     }
+
+    _onApiOpenCrossFramePort({targetTabId, targetFrameId}, sender) {
+        const sourceTabId = (sender && sender.tab ? sender.tab.id : null);
+        if (typeof sourceTabId !== 'number') {
+            throw new Error('Port does not have an associated tab ID');
+        }
+        const sourceFrameId = sender.frameId;
+        if (typeof sourceFrameId !== 'number') {
+            throw new Error('Port does not have an associated frame ID');
+        }
+
+        const sourceDetails = {
+            name: 'cross-frame-communication-port',
+            otherTabId: targetTabId,
+            otherFrameId: targetFrameId
+        };
+        const targetDetails = {
+            name: 'cross-frame-communication-port',
+            otherTabId: sourceTabId,
+            otherFrameId: sourceFrameId
+        };
+        let sourcePort = chrome.tabs.connect(sourceTabId, {frameId: sourceFrameId, name: JSON.stringify(sourceDetails)});
+        let targetPort = chrome.tabs.connect(targetTabId, {frameId: targetFrameId, name: JSON.stringify(targetDetails)});
+
+        const cleanup = () => {
+            this._checkLastError(chrome.runtime.lastError);
+            if (targetPort !== null) {
+                targetPort.disconnect();
+                targetPort = null;
+            }
+            if (sourcePort !== null) {
+                sourcePort.disconnect();
+                sourcePort = null;
+            }
+        };
+
+        sourcePort.onMessage.addListener((message) => { targetPort.postMessage(message); });
+        targetPort.onMessage.addListener((message) => { sourcePort.postMessage(message); });
+        sourcePort.onDisconnect.addListener(cleanup);
+        targetPort.onDisconnect.addListener(cleanup);
+
+        return {targetTabId, targetFrameId};
+    }
 }
diff --git a/ext/js/comm/api.js b/ext/js/comm/api.js
index de12bb6c..72d2ba07 100644
--- a/ext/js/comm/api.js
+++ b/ext/js/comm/api.js
@@ -181,6 +181,10 @@ class API {
         return this._invoke('loadExtensionScripts', {files});
     }
 
+    openCrossFramePort(targetTabId, targetFrameId) {
+        return this._invoke('openCrossFramePort', {targetTabId, targetFrameId});
+    }
+
     // Utilities
 
     _createActionPort(timeout=5000) {
diff --git a/ext/js/comm/cross-frame-api.js b/ext/js/comm/cross-frame-api.js
index 7892eb4c..fb2a1718 100644
--- a/ext/js/comm/cross-frame-api.js
+++ b/ext/js/comm/cross-frame-api.js
@@ -224,10 +224,13 @@ class CrossFrameAPI {
         this._commPorts = new Map();
         this._messageHandlers = new Map();
         this._onDisconnectBind = this._onDisconnect.bind(this);
+        this._tabId = null;
+        this._frameId = null;
     }
 
-    prepare() {
+    async prepare() {
         chrome.runtime.onConnect.addListener(this._onConnect.bind(this));
+        ({tabId: this._tabId, frameId: this._frameId} = await yomichan.api.frameInformationGet());
     }
 
     invoke(targetFrameId, action, params={}) {
@@ -235,8 +238,8 @@ class CrossFrameAPI {
     }
 
     async invokeTab(targetTabId, targetFrameId, action, params={}) {
-        if (typeof targetTabId !== 'number') { targetTabId = null; }
-        const commPort = this._getOrCreateCommPort(targetTabId, targetFrameId);
+        if (typeof targetTabId !== 'number') { targetTabId = this._tabId; }
+        const commPort = await this._getOrCreateCommPort(targetTabId, targetFrameId);
         return await commPort.invoke(action, params, this._ackTimeout, this._responseTimeout);
     }
 
@@ -265,8 +268,8 @@ class CrossFrameAPI {
             }
             if (details.name !== 'cross-frame-communication-port') { return; }
 
-            const otherTabId = details.sourceTabId;
-            const otherFrameId = details.sourceFrameId;
+            const otherTabId = details.otherTabId;
+            const otherFrameId = details.otherFrameId;
             this._setupCommPort(otherTabId, otherFrameId, port);
         } catch (e) {
             port.disconnect();
@@ -297,14 +300,16 @@ class CrossFrameAPI {
         return this._createCommPort(otherTabId, otherFrameId);
     }
 
-    _createCommPort(otherTabId, otherFrameId) {
-        const details = {
-            name: 'background-cross-frame-communication-port',
-            targetTabId: otherTabId,
-            targetFrameId: otherFrameId
-        };
-        const port = yomichan.connect(null, {name: JSON.stringify(details)});
-        return this._setupCommPort(otherTabId, otherFrameId, port);
+    async _createCommPort(otherTabId, otherFrameId) {
+        await yomichan.api.openCrossFramePort(otherTabId, otherFrameId);
+
+        const tabPorts = this._commPorts.get(otherTabId);
+        if (typeof tabPorts !== 'undefined') {
+            const commPort = tabPorts.get(otherFrameId);
+            if (typeof commPort !== 'undefined') {
+                return commPort;
+            }
+        }
     }
 
     _setupCommPort(otherTabId, otherFrameId, port) {
diff --git a/ext/js/yomichan.js b/ext/js/yomichan.js
index 6eea952e..5a55b3ea 100644
--- a/ext/js/yomichan.js
+++ b/ext/js/yomichan.js
@@ -129,12 +129,12 @@ class Yomichan extends EventDispatcher {
         if (!isBackground) {
             this._api = new API(this);
 
-            this._crossFrame = new CrossFrameAPI();
-            this._crossFrame.prepare();
-
             this.sendMessage({action: 'requestBackendReadySignal'});
             await this._isBackendReadyPromise;
 
+            this._crossFrame = new CrossFrameAPI();
+            await this._crossFrame.prepare();
+
             log.on('log', this._onForwardLog.bind(this));
         }
     }
@@ -172,24 +172,6 @@ class Yomichan extends EventDispatcher {
         }
     }
 
-    /**
-     * Runs `chrome.runtime.connect()` with additional exception handling events.
-     * @param {...*} args The arguments to be passed to `chrome.runtime.connect()`.
-     * @returns {Port} The resulting port.
-     * @throws {Error} Errors thrown by `chrome.runtime.connect()` are re-thrown.
-     */
-    connect(...args) {
-        try {
-            return chrome.runtime.connect(...args);
-        } catch (e) {
-            this.triggerExtensionUnloaded();
-            throw e;
-        }
-    }
-
-    /**
-     * Runs chrome.runtime.connect() with additional exception handling events.
-     */
     triggerExtensionUnloaded() {
         this._isExtensionUnloaded = true;
         if (this._isTriggeringExtensionUnloaded) { return; }