From b936c3e4b1bc993e535b02dee91bf6afc15a3564 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Fri, 8 May 2020 19:04:53 -0400 Subject: [PATCH] Popup proxy host refactor (#516) * Rename PopupProxyHost to PopupFactory * Update FrontendApiReceiver to support non-async handlers * Make some functions non-async * Make setCustomCss non-async * Make setContentScale non-async * Remove static * Rename variables * Pass frameId into PopupFactory's constructor * Change FrontendApiReceiver source from popup-proxy-host to popup-factor * Rename _invokeHostApi to _invoke * Rename PopupProxy.getHostUrl to getUrl --- ext/bg/js/search-main.js | 2 +- ext/bg/js/settings/popup-preview-frame.js | 11 ++-- ext/bg/settings-popup-preview.html | 2 +- ext/fg/js/content-script-main.js | 28 ++++++--- ext/fg/js/frontend-api-receiver.js | 27 ++++++-- .../{popup-proxy-host.js => popup-factory.js} | 61 +++++++++---------- ext/fg/js/popup-proxy.js | 32 +++++----- ext/fg/js/popup.js | 2 +- ext/manifest.json | 2 +- 9 files changed, 94 insertions(+), 73 deletions(-) rename ext/fg/js/{popup-proxy-host.js => popup-factory.js} (69%) diff --git a/ext/bg/js/search-main.js b/ext/bg/js/search-main.js index 1075d46e..6e092fbc 100644 --- a/ext/bg/js/search-main.js +++ b/ext/bg/js/search-main.js @@ -31,7 +31,7 @@ async function injectSearchFrontend() { '/fg/js/frontend-api-receiver.js', '/fg/js/frame-offset-forwarder.js', '/fg/js/popup.js', - '/fg/js/popup-proxy-host.js', + '/fg/js/popup-factory.js', '/fg/js/frontend.js', '/fg/js/content-script-main.js' ]); diff --git a/ext/bg/js/settings/popup-preview-frame.js b/ext/bg/js/settings/popup-preview-frame.js index c7b0c218..8901a0c4 100644 --- a/ext/bg/js/settings/popup-preview-frame.js +++ b/ext/bg/js/settings/popup-preview-frame.js @@ -18,8 +18,9 @@ /* global * Frontend * Popup - * PopupProxyHost + * PopupFactory * TextSourceRange + * apiFrameInformationGet * apiOptionsGet */ @@ -56,10 +57,12 @@ class SettingsPopupPreview { window.apiOptionsGet = this.apiOptionsGet.bind(this); // Overwrite frontend - const popupHost = new PopupProxyHost(); - await popupHost.prepare(); + const {frameId} = await apiFrameInformationGet(); - this.popup = popupHost.getOrCreatePopup(); + const popupFactory = new PopupFactory(frameId); + await popupFactory.prepare(); + + this.popup = popupFactory.getOrCreatePopup(); this.popup.setChildrenSupported(false); this.popupSetCustomOuterCssOld = this.popup.setCustomOuterCss; diff --git a/ext/bg/settings-popup-preview.html b/ext/bg/settings-popup-preview.html index a332fb22..3d7f6455 100644 --- a/ext/bg/settings-popup-preview.html +++ b/ext/bg/settings-popup-preview.html @@ -127,7 +127,7 @@ - + diff --git a/ext/fg/js/content-script-main.js b/ext/fg/js/content-script-main.js index 277e6567..3afc9648 100644 --- a/ext/fg/js/content-script-main.js +++ b/ext/fg/js/content-script-main.js @@ -19,10 +19,11 @@ * DOM * FrameOffsetForwarder * Frontend + * PopupFactory * PopupProxy - * PopupProxyHost * apiBroadcastTab * apiForwardLogsToBackend + * apiFrameInformationGet * apiOptionsGet */ @@ -47,10 +48,17 @@ async function createIframePopupProxy(frameOffsetForwarder, setDisabled) { } async function getOrCreatePopup(depth) { - const popupHost = new PopupProxyHost(); - await popupHost.prepare(); + const {frameId} = await apiFrameInformationGet(); + if (typeof frameId !== 'number') { + const error = new Error('Failed to get frameId'); + yomichan.logError(error); + throw error; + } - const popup = popupHost.getOrCreatePopup(null, null, depth); + const popupFactory = new PopupFactory(frameId); + await popupFactory.prepare(); + + const popup = popupFactory.getOrCreatePopup(null, null, depth); return popup; } @@ -89,20 +97,20 @@ async function createPopupProxy(depth, id, parentFrameId) { }; let urlUpdatedAt = 0; - let proxyHostUrlCached = url; - const getProxyHostUrl = async () => { + let popupProxyUrlCached = url; + const getPopupProxyUrl = async () => { const now = Date.now(); if (popups.proxy !== null && now - urlUpdatedAt > 500) { - proxyHostUrlCached = await popups.proxy.getHostUrl(); + popupProxyUrlCached = await popups.proxy.getUrl(); urlUpdatedAt = now; } - return proxyHostUrlCached; + return popupProxyUrlCached; }; const applyOptions = async () => { const optionsContext = { depth: isSearchPage ? 0 : depth, - url: proxy ? await getProxyHostUrl() : window.location.href + url: proxy ? await getPopupProxyUrl() : window.location.href }; const options = await apiOptionsGet(optionsContext); @@ -124,7 +132,7 @@ async function createPopupProxy(depth, id, parentFrameId) { } if (frontend === null) { - const getUrl = proxy ? getProxyHostUrl : null; + const getUrl = proxy ? getPopupProxyUrl : null; frontend = new Frontend(popup, getUrl); frontendPreparePromise = frontend.prepare(); await frontendPreparePromise; diff --git a/ext/fg/js/frontend-api-receiver.js b/ext/fg/js/frontend-api-receiver.js index c5bb58af..3fa9e8b6 100644 --- a/ext/fg/js/frontend-api-receiver.js +++ b/ext/fg/js/frontend-api-receiver.js @@ -17,9 +17,9 @@ class FrontendApiReceiver { - constructor(source='', handlers=new Map()) { + constructor(source, messageHandlers) { this._source = source; - this._handlers = handlers; + this._messageHandlers = messageHandlers; } prepare() { @@ -35,14 +35,29 @@ class FrontendApiReceiver { _onMessage(port, {id, action, params, target, senderId}) { if (target !== this._source) { return; } - const handler = this._handlers.get(action); - if (typeof handler !== 'function') { return; } + const messageHandler = this._messageHandlers.get(action); + if (typeof messageHandler === 'undefined') { return; } + + const {handler, async} = messageHandler; this._sendAck(port, id, senderId); - this._invokeHandler(handler, params, port, id, senderId); + if (async) { + this._invokeHandlerAsync(handler, params, port, id, senderId); + } else { + this._invokeHandler(handler, params, port, id, senderId); + } } - async _invokeHandler(handler, params, port, id, senderId) { + _invokeHandler(handler, params, port, id, senderId) { + try { + const result = handler(params); + this._sendResult(port, id, senderId, {result}); + } catch (error) { + this._sendResult(port, id, senderId, {error: errorToJson(error)}); + } + } + + async _invokeHandlerAsync(handler, params, port, id, senderId) { try { const result = await handler(params); this._sendResult(port, id, senderId, {result}); diff --git a/ext/fg/js/popup-proxy-host.js b/ext/fg/js/popup-factory.js similarity index 69% rename from ext/fg/js/popup-proxy-host.js rename to ext/fg/js/popup-factory.js index 39150bc7..21e64dd0 100644 --- a/ext/fg/js/popup-proxy-host.js +++ b/ext/fg/js/popup-factory.js @@ -18,34 +18,29 @@ /* global * FrontendApiReceiver * Popup - * apiFrameInformationGet */ -class PopupProxyHost { - constructor() { +class PopupFactory { + constructor(frameId) { this._popups = new Map(); - this._frameId = null; + this._frameId = frameId; } // Public functions async prepare() { - const {frameId} = await apiFrameInformationGet(); - if (typeof frameId !== 'number') { return; } - this._frameId = frameId; - - 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)], - ['isVisible', this._onApiIsVisibleAsync.bind(this)], - ['setVisibleOverride', this._onApiSetVisibleOverride.bind(this)], - ['containsPoint', this._onApiContainsPoint.bind(this)], - ['showContent', this._onApiShowContent.bind(this)], - ['setCustomCss', this._onApiSetCustomCss.bind(this)], - ['clearAutoPlayTimer', this._onApiClearAutoPlayTimer.bind(this)], - ['setContentScale', this._onApiSetContentScale.bind(this)], - ['getHostUrl', this._onApiGetHostUrl.bind(this)] + const apiReceiver = new FrontendApiReceiver(`popup-factory#${this._frameId}`, new Map([ + ['getOrCreatePopup', {async: false, handler: this._onApiGetOrCreatePopup.bind(this)}], + ['setOptionsContext', {async: true, handler: this._onApiSetOptionsContext.bind(this)}], + ['hide', {async: false, handler: this._onApiHide.bind(this)}], + ['isVisible', {async: true, handler: this._onApiIsVisibleAsync.bind(this)}], + ['setVisibleOverride', {async: true, handler: this._onApiSetVisibleOverride.bind(this)}], + ['containsPoint', {async: true, handler: this._onApiContainsPoint.bind(this)}], + ['showContent', {async: true, handler: this._onApiShowContent.bind(this)}], + ['setCustomCss', {async: false, handler: this._onApiSetCustomCss.bind(this)}], + ['clearAutoPlayTimer', {async: false, handler: this._onApiClearAutoPlayTimer.bind(this)}], + ['setContentScale', {async: false, handler: this._onApiSetContentScale.bind(this)}], + ['getUrl', {async: false, handler: this._onApiGetUrl.bind(this)}] ])); apiReceiver.prepare(); } @@ -97,7 +92,7 @@ class PopupProxyHost { // API message handlers - async _onApiGetOrCreatePopup({id, parentId}) { + _onApiGetOrCreatePopup({id, parentId}) { const popup = this.getOrCreatePopup(id, parentId); return { id: popup.id @@ -109,7 +104,7 @@ class PopupProxyHost { return await popup.setOptionsContext(optionsContext, source); } - async _onApiHide({id, changeFocus}) { + _onApiHide({id, changeFocus}) { const popup = this._getPopup(id); return popup.hide(changeFocus); } @@ -126,33 +121,33 @@ class PopupProxyHost { async _onApiContainsPoint({id, x, y}) { const popup = this._getPopup(id); - [x, y] = PopupProxyHost._convertPopupPointToRootPagePoint(popup, x, y); + [x, y] = this._convertPopupPointToRootPagePoint(popup, x, y); return await popup.containsPoint(x, y); } async _onApiShowContent({id, elementRect, writingMode, type, details, context}) { const popup = this._getPopup(id); - elementRect = PopupProxyHost._convertJsonRectToDOMRect(popup, elementRect); - if (!PopupProxyHost._popupCanShow(popup)) { return; } + elementRect = this._convertJsonRectToDOMRect(popup, elementRect); + if (!this._popupCanShow(popup)) { return; } return await popup.showContent(elementRect, writingMode, type, details, context); } - async _onApiSetCustomCss({id, css}) { + _onApiSetCustomCss({id, css}) { const popup = this._getPopup(id); return popup.setCustomCss(css); } - async _onApiClearAutoPlayTimer({id}) { + _onApiClearAutoPlayTimer({id}) { const popup = this._getPopup(id); return popup.clearAutoPlayTimer(); } - async _onApiSetContentScale({id, scale}) { + _onApiSetContentScale({id, scale}) { const popup = this._getPopup(id); return popup.setContentScale(scale); } - async _onApiGetHostUrl() { + _onApiGetUrl() { return window.location.href; } @@ -166,12 +161,12 @@ class PopupProxyHost { return popup; } - static _convertJsonRectToDOMRect(popup, jsonRect) { - const [x, y] = PopupProxyHost._convertPopupPointToRootPagePoint(popup, jsonRect.x, jsonRect.y); + _convertJsonRectToDOMRect(popup, jsonRect) { + const [x, y] = this._convertPopupPointToRootPagePoint(popup, jsonRect.x, jsonRect.y); return new DOMRect(x, y, jsonRect.width, jsonRect.height); } - static _convertPopupPointToRootPagePoint(popup, x, y) { + _convertPopupPointToRootPagePoint(popup, x, y) { if (popup.parent !== null) { const popupRect = popup.parent.getContainerRect(); x += popupRect.x; @@ -180,7 +175,7 @@ class PopupProxyHost { return [x, y]; } - static _popupCanShow(popup) { + _popupCanShow(popup) { return popup.parent === null || popup.parent.isVisibleSync(); } } diff --git a/ext/fg/js/popup-proxy.js b/ext/fg/js/popup-proxy.js index 93418202..6a84e000 100644 --- a/ext/fg/js/popup-proxy.js +++ b/ext/fg/js/popup-proxy.js @@ -51,7 +51,7 @@ class PopupProxy { // Public functions async prepare() { - const {id} = await this._invokeHostApi('getOrCreatePopup', {id: this._id, parentId: this._parentId}); + const {id} = await this._invoke('getOrCreatePopup', {id: this._id, parentId: this._parentId}); this._id = id; } @@ -60,19 +60,19 @@ class PopupProxy { } async setOptionsContext(optionsContext, source) { - return await this._invokeHostApi('setOptionsContext', {id: this._id, optionsContext, source}); + return await this._invoke('setOptionsContext', {id: this._id, optionsContext, source}); } hide(changeFocus) { - this._invokeHostApi('hide', {id: this._id, changeFocus}); + this._invoke('hide', {id: this._id, changeFocus}); } async isVisible() { - return await this._invokeHostApi('isVisible', {id: this._id}); + return await this._invoke('isVisible', {id: this._id}); } setVisibleOverride(visible) { - this._invokeHostApi('setVisibleOverride', {id: this._id, visible}); + this._invoke('setVisibleOverride', {id: this._id, visible}); } async containsPoint(x, y) { @@ -80,7 +80,7 @@ class PopupProxy { await this._updateFrameOffset(); [x, y] = this._applyFrameOffset(x, y); } - return await this._invokeHostApi('containsPoint', {id: this._id, x, y}); + return await this._invoke('containsPoint', {id: this._id, x, y}); } async showContent(elementRect, writingMode, type, details, context) { @@ -90,32 +90,32 @@ class PopupProxy { [x, y] = this._applyFrameOffset(x, y); } elementRect = {x, y, width, height}; - return await this._invokeHostApi('showContent', {id: this._id, elementRect, writingMode, type, details, context}); + return await this._invoke('showContent', {id: this._id, elementRect, writingMode, type, details, context}); } - async setCustomCss(css) { - return await this._invokeHostApi('setCustomCss', {id: this._id, css}); + setCustomCss(css) { + this._invoke('setCustomCss', {id: this._id, css}); } clearAutoPlayTimer() { - this._invokeHostApi('clearAutoPlayTimer', {id: this._id}); + this._invoke('clearAutoPlayTimer', {id: this._id}); } - async setContentScale(scale) { - this._invokeHostApi('setContentScale', {id: this._id, scale}); + setContentScale(scale) { + this._invoke('setContentScale', {id: this._id, scale}); } - async getHostUrl() { - return await this._invokeHostApi('getHostUrl', {}); + async getUrl() { + return await this._invoke('getUrl', {}); } // Private - _invokeHostApi(action, params={}) { + _invoke(action, params={}) { if (typeof this._parentFrameId !== 'number') { return Promise.reject(new Error('Invalid frame')); } - return this._apiSender.invoke(action, params, `popup-proxy-host#${this._parentFrameId}`); + return this._apiSender.invoke(action, params, `popup-factory#${this._parentFrameId}`); } async _updateFrameOffset() { diff --git a/ext/fg/js/popup.js b/ext/fg/js/popup.js index 7db53f0d..79b37251 100644 --- a/ext/fg/js/popup.js +++ b/ext/fg/js/popup.js @@ -139,7 +139,7 @@ class Popup { this._invokeApi('setContent', {type, details}); } - async setCustomCss(css) { + setCustomCss(css) { this._invokeApi('setCustomCss', {css}); } diff --git a/ext/manifest.json b/ext/manifest.json index 3cb634f0..80823fc4 100644 --- a/ext/manifest.json +++ b/ext/manifest.json @@ -44,9 +44,9 @@ "fg/js/frontend-api-receiver.js", "fg/js/popup.js", "fg/js/source.js", + "fg/js/popup-factory.js", "fg/js/frame-offset-forwarder.js", "fg/js/popup-proxy.js", - "fg/js/popup-proxy-host.js", "fg/js/frontend.js", "fg/js/content-script-main.js" ],