Application data refactor (#699)

* Pass tabId and frameId to Application * Remove casts * Remove redundant frameInformationGet calls * Expose tabId and frameId * Remove unsed * Simplify * Update FrameAncestryHandler to not need a direct frameId * Remove frameId from FrameOffsetForwarder * Remove frameId from PopupFactory * Remove frameId from Frontend * Remove frameId from PopupPreviewFrame * Fix PopupFactory and Frontend constructor * Remove frameId from Display * Remove frameId from SearchDisplayController * Restore if check
yomidevs · Feb 18, 2024 · 7e9f7e2 · 7e9f7e2
1 parent 4aaa9f1
commit 7e9f7e2
Show file tree

Hide file tree

Showing 17 changed files with 99 additions and 136 deletions.
diff --git a/ext/js/app/content-script-main.js b/ext/js/app/content-script-main.js
@@ -22,21 +22,14 @@ import {Frontend} from './frontend.js';
 import {PopupFactory} from './popup-factory.js';
 
 await Application.main(async (application) => {
-    const {tabId, frameId} = await application.api.frameInformationGet();
-    if (typeof frameId !== 'number') {
-        throw new Error('Failed to get frameId');
-    }
-
     const hotkeyHandler = new HotkeyHandler();
     hotkeyHandler.prepare(application.crossFrame);
 
-    const popupFactory = new PopupFactory(application, frameId);
+    const popupFactory = new PopupFactory(application);
     popupFactory.prepare();
 
     const frontend = new Frontend({
         application,
-        tabId,
-        frameId,
         popupFactory,
         depth: 0,
         parentPopupId: null,

diff --git a/ext/js/app/frontend.js b/ext/js/app/frontend.js
@@ -39,8 +39,6 @@ export class Frontend {
         pageType,
         popupFactory,
         depth,
-        tabId,
-        frameId,
         parentPopupId,
         parentFrameId,
         useProxyPopup,
@@ -57,10 +55,6 @@ export class Frontend {
         this._popupFactory = popupFactory;
         /** @type {number} */
         this._depth = depth;
-        /** @type {number|undefined} */
-        this._tabId = tabId;
-        /** @type {number} */
-        this._frameId = frameId;
         /** @type {?string} */
         this._parentPopupId = parentPopupId;
         /** @type {?number} */
@@ -588,8 +582,13 @@ export class Frontend {
             return null;
         }
 
+        const {frameId} = this._application;
+        if (frameId === null) {
+            return null;
+        }
+
         return await this._popupFactory.getOrCreatePopup({
-            frameId: this._frameId,
+            frameId,
             depth: this._depth,
             childrenSupported: this._childrenSupported
         });
@@ -703,12 +702,10 @@ export class Frontend {
         };
         if (sentence !== null) { detailsState.sentence = sentence; }
         if (documentTitle !== null) { detailsState.documentTitle = documentTitle; }
+        const {tabId, frameId} = this._application;
         /** @type {import('display').HistoryContent} */
         const detailsContent = {
-            contentOrigin: {
-                tabId: this._tabId,
-                frameId: this._frameId
-            }
+            contentOrigin: {tabId, frameId}
         };
         if (dictionaryEntries !== null) {
             detailsContent.dictionaryEntries = dictionaryEntries;
@@ -819,7 +816,7 @@ export class Frontend {
      */
     _signalFrontendReady(targetFrameId) {
         /** @type {import('application').ApiMessageNoFrameId<'frontendReady'>} */
-        const message = {action: 'frontendReady', params: {frameId: this._frameId}};
+        const message = {action: 'frontendReady', params: {frameId: this._application.frameId}};
         if (targetFrameId === null) {
             this._application.api.broadcastTab(message);
         } else {
@@ -867,7 +864,7 @@ export class Frontend {
             }
 
             chrome.runtime.onMessage.addListener(onMessage);
-            this._application.api.broadcastTab({action: 'frontendRequestReadyBroadcast', params: {frameId: this._frameId}});
+            this._application.api.broadcastTab({action: 'frontendRequestReadyBroadcast', params: {frameId: this._application.frameId}});
         });
     }
 

diff --git a/ext/js/app/popup-factory.js b/ext/js/app/popup-factory.js
@@ -29,15 +29,12 @@ export class PopupFactory {
     /**
      * Creates a new instance.
      * @param {import('../application.js').Application} application
-     * @param {number} frameId The frame ID of the host frame.
      */
-    constructor(application, frameId) {
+    constructor(application) {
         /** @type {import('../application.js').Application} */
         this._application = application;
-        /** @type {number} */
-        this._frameId = frameId;
         /** @type {FrameOffsetForwarder} */
-        this._frameOffsetForwarder = new FrameOffsetForwarder(application.crossFrame, frameId);
+        this._frameOffsetForwarder = new FrameOffsetForwarder(application.crossFrame);
         /** @type {Map<string, import('popup').PopupAny>} */
         this._popups = new Map();
         /** @type {Map<string, {popup: import('popup').PopupAny, token: string}[]>} */
@@ -115,6 +112,9 @@ export class PopupFactory {
             depth = 0;
         }
 
+        const currentFrameId = this._application.frameId;
+        if (currentFrameId === null) { throw new Error('Cannot create popup: no frameId'); }
+
         if (popupWindow) {
             // New unique id
             if (id === null) {
@@ -124,11 +124,11 @@ export class PopupFactory {
                 application: this._application,
                 id,
                 depth,
-                frameId: this._frameId
+                frameId: currentFrameId
             });
             this._popups.set(id, popup);
             return popup;
-        } else if (frameId === this._frameId) {
+        } else if (frameId === currentFrameId) {
             // New unique id
             if (id === null) {
                 id = generateId(16);
@@ -137,7 +137,7 @@ export class PopupFactory {
                 application: this._application,
                 id,
                 depth,
-                frameId: this._frameId,
+                frameId: currentFrameId,
                 childrenSupported
             });
             if (parent !== null) {
@@ -155,13 +155,12 @@ export class PopupFactory {
                 throw new Error('Invalid frameId');
             }
             const useFrameOffsetForwarder = (parentPopupId === null);
-            /** @type {{id: string, depth: number, frameId: number}} */
-            const info = await this._application.crossFrame.invoke(frameId, 'popupFactoryGetOrCreatePopup', /** @type {import('popup-factory').GetOrCreatePopupDetails} */ ({
+            const info = await this._application.crossFrame.invoke(frameId, 'popupFactoryGetOrCreatePopup', {
                 id,
                 parentPopupId,
                 frameId,
                 childrenSupported
-            }));
+            });
             id = info.id;
             const popup = new PopupProxy({
                 application: this._application,

diff --git a/ext/js/application.js b/ext/js/application.js
@@ -108,7 +108,6 @@ export class Application extends EventDispatcher {
      * @type {API}
      */
     get api() {
-        if (this._api === null) { throw new Error('Not prepared'); }
         return this._api;
     }
 
@@ -118,10 +117,23 @@ export class Application extends EventDispatcher {
      * @type {CrossFrameAPI}
      */
     get crossFrame() {
-        if (this._crossFrame === null) { throw new Error('Not prepared'); }
         return this._crossFrame;
     }
 
+    /**
+     * @type {?number}
+     */
+    get tabId() {
+        return this._crossFrame.tabId;
+    }
+
+    /**
+     * @type {?number}
+     */
+    get frameId() {
+        return this._crossFrame.frameId;
+    }
+
     /**
      * Prepares the instance for use.
      */
@@ -157,7 +169,7 @@ export class Application extends EventDispatcher {
         const webExtension = new WebExtension();
         const api = new API(webExtension);
         await this.waitForBackendReady(webExtension);
-        const {tabId = null, frameId = null} = await api.frameInformationGet();
+        const {tabId, frameId} = await api.frameInformationGet();
         const crossFrameApi = new CrossFrameAPI(api, tabId, frameId);
         crossFrameApi.prepare();
         const application = new Application(api, crossFrameApi);

diff --git a/ext/js/background/backend.js b/ext/js/background/backend.js
@@ -654,7 +654,10 @@ export class Backend {
         const tab = sender.tab;
         const tabId = tab ? tab.id : void 0;
         const frameId = sender.frameId;
-        return Promise.resolve({tabId, frameId});
+        return {
+            tabId: typeof tabId === 'number' ? tabId : null,
+            frameId: typeof frameId === 'number' ? frameId : null
+        };
     }
 
     /** @type {import('api').ApiHandler<'injectStylesheet'>} */

diff --git a/ext/js/comm/cross-frame-api.js b/ext/js/comm/cross-frame-api.js
@@ -313,6 +313,20 @@ export class CrossFrameAPI {
         this._frameId = frameId;
     }
 
+    /**
+     * @type {?number}
+     */
+    get tabId() {
+        return this._tabId;
+    }
+
+    /**
+     * @type {?number}
+     */
+    get frameId() {
+        return this._frameId;
+    }
+
     /** */
     prepare() {
         chrome.runtime.onConnect.addListener(this._onConnect.bind(this));

diff --git a/ext/js/comm/frame-ancestry-handler.js b/ext/js/comm/frame-ancestry-handler.js
@@ -28,13 +28,10 @@ export class FrameAncestryHandler {
     /**
      * Creates a new instance.
      * @param {import('../comm/cross-frame-api.js').CrossFrameAPI} crossFrameApi
-     * @param {number} frameId The frame ID of the current frame the instance is instantiated in.
      */
-    constructor(crossFrameApi, frameId) {
+    constructor(crossFrameApi) {
         /** @type {import('../comm/cross-frame-api.js').CrossFrameAPI} */
         this._crossFrameApi = crossFrameApi;
-        /** @type {number} */
-        this._frameId = frameId;
         /** @type {boolean} */
         this._isPrepared = false;
         /** @type {string} */
@@ -47,14 +44,6 @@ export class FrameAncestryHandler {
         this._responseHandlers = new Map();
     }
 
-    /**
-     * Gets the frame ID that the instance is instantiated in.
-     * @type {number}
-     */
-    get frameId() {
-        return this._frameId;
-    }
-
     /**
      * Initializes event event listening.
      */
@@ -116,8 +105,9 @@ export class FrameAncestryHandler {
      */
     _getFrameAncestryInfo(timeout = 5000) {
         return new Promise((resolve, reject) => {
+            const {frameId} = this._crossFrameApi;
             const targetWindow = window.parent;
-            if (window === targetWindow) {
+            if (frameId === null || window === targetWindow) {
                 resolve([]);
                 return;
             }
@@ -141,11 +131,10 @@ export class FrameAncestryHandler {
                 if (params.nonce !== nonce) { return null; }
 
                 // Add result
-                const {frameId, more} = params;
-                results.push(frameId);
+                results.push(params.frameId);
                 nonce = generateId(16);
 
-                if (!more) {
+                if (!params.more) {
                     // Cleanup
                     cleanup();
 
@@ -167,7 +156,6 @@ export class FrameAncestryHandler {
             // Start
             this._addResponseHandler(uniqueId, onMessage);
             resetTimeout();
-            const frameId = this._frameId;
             this._requestFrameInfo(targetWindow, frameId, frameId, uniqueId, nonce);
         });
     }
@@ -208,7 +196,9 @@ export class FrameAncestryHandler {
                 return;
             }
 
-            const frameId = this._frameId;
+            const {frameId} = this._crossFrameApi;
+            if (frameId === null) { return; }
+
             const {parent} = window;
             const more = (window !== parent);
 

diff --git a/ext/js/comm/frame-offset-forwarder.js b/ext/js/comm/frame-offset-forwarder.js
@@ -21,15 +21,12 @@ import {FrameAncestryHandler} from './frame-ancestry-handler.js';
 export class FrameOffsetForwarder {
     /**
      * @param {import('../comm/cross-frame-api.js').CrossFrameAPI} crossFrameApi
-     * @param {number} frameId
      */
-    constructor(crossFrameApi, frameId) {
+    constructor(crossFrameApi) {
         /** @type {import('../comm/cross-frame-api.js').CrossFrameAPI} */
         this._crossFrameApi = crossFrameApi;
-        /** @type {number} */
-        this._frameId = frameId;
         /** @type {FrameAncestryHandler} */
-        this._frameAncestryHandler = new FrameAncestryHandler(crossFrameApi, frameId);
+        this._frameAncestryHandler = new FrameAncestryHandler(crossFrameApi);
     }
 
     /**
@@ -50,15 +47,18 @@ export class FrameOffsetForwarder {
             return [0, 0];
         }
 
+        const {frameId} = this._crossFrameApi;
+        if (frameId === null) { return null; }
+
         try {
             const ancestorFrameIds = await this._frameAncestryHandler.getFrameAncestryInfo();
 
-            let childFrameId = this._frameId;
+            let childFrameId = frameId;
             /** @type {Promise<?import('frame-offset-forwarder').ChildFrameRect>[]} */
             const promises = [];
-            for (const frameId of ancestorFrameIds) {
-                promises.push(this._crossFrameApi.invoke(frameId, 'frameOffsetForwarderGetChildFrameRect', {frameId: childFrameId}));
-                childFrameId = frameId;
+            for (const ancestorFrameId of ancestorFrameIds) {
+                promises.push(this._crossFrameApi.invoke(ancestorFrameId, 'frameOffsetForwarderGetChildFrameRect', {frameId: childFrameId}));
+                childFrameId = ancestorFrameId;
             }
 
             const results = await Promise.all(promises);