Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 1, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6769ff5
create both cross-frame ports in the background
praschke Sep 30, 2023
97a6ced
remove yomichan.connect()
praschke Oct 1, 2023
be46fb6
remove stray debugging comment
praschke Oct 1, 2023
841d49c
wait for backend ready signal to prepare cross-frame api
praschke Oct 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 45 additions & 56 deletions ext/js/background/backend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([
]);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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}});
}
Expand Down Expand Up @@ -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};
}
}
4 changes: 4 additions & 0 deletions ext/js/comm/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ class API {
return this._invoke('loadExtensionScripts', {files});
}

openCrossFramePort(targetTabId, targetFrameId) {
return this._invoke('openCrossFramePort', {targetTabId, targetFrameId});
}

// Utilities

_createActionPort(timeout=5000) {
Expand Down
31 changes: 18 additions & 13 deletions ext/js/comm/cross-frame-api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,19 +224,22 @@ 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={}) {
return this.invokeTab(null, targetFrameId, action, params);
}

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

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)});
djahandarie marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down
24 changes: 3 additions & 21 deletions ext/js/yomichan.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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; }
Expand Down