Improve popup window ownership (#1364)

* Update frameInformationGet to also return the tab ID

* Add tabId to Frontend

* Pass tabId/frameId to Display

* Pass ownership information using setContent

* Remove ownerFrameId for Popup classes

* Use frameId instead of ownerFrameId for screenshotting

* Use contentOrigin instead of owner

* Update _invokeContentOrigin implementation

ext/bg/js/backend.js

@@ -525,8 +525,10 @@ class Backend {
     }
 
     _onApiFrameInformationGet(params, sender) {
+        const tab = sender.tab;
+        const tabId = tab ? tab.id : void 0;
         const frameId = sender.frameId;
-        return Promise.resolve({frameId});
+        return Promise.resolve({tabId, frameId});
     }
 
     _onApiInjectStylesheet({type, value}, sender) {
@@ -1505,17 +1507,17 @@ class Backend {
         return isValidTab ? tab : null;
     }
 
-    async _getScreenshot(windowId, tabId, ownerFrameId, format, quality) {
+    async _getScreenshot(windowId, tabId, frameId, format, quality) {
         if (typeof windowId !== 'number') {
             throw new Error('Invalid window ID');
         }
 
         let token = null;
         try {
-            if (typeof tabId === 'number' && typeof ownerFrameId === 'number') {
+            if (typeof tabId === 'number' && typeof frameId === 'number') {
                 const action = 'setAllVisibleOverride';
                 const params = {value: false, priority: 0, awaitFrame: true};
-                token = await this._sendMessageTabPromise(tabId, {action, params}, {frameId: ownerFrameId});
+                token = await this._sendMessageTabPromise(tabId, {action, params}, {frameId});
             }
 
             return await new Promise((resolve, reject) => {
@@ -1533,7 +1535,7 @@ class Backend {
                 const action = 'clearAllVisibleOverride';
                 const params = {token};
                 try {
-                    await this._sendMessageTabPromise(tabId, {action, params}, {frameId: ownerFrameId});
+                    await this._sendMessageTabPromise(tabId, {action, params}, {frameId});
                 } catch (e) {
                     // NOP
                 }
@@ -1634,8 +1636,8 @@ class Backend {
     }
 
     async _injectAnkNoteScreenshot(ankiConnect, timestamp, definitionDetails, details) {
-        const {windowId, tabId, ownerFrameId, format, quality} = details;
+        const {windowId, tabId, frameId, format, quality} = details;
-        const dataUrl = await this._getScreenshot(windowId, tabId, ownerFrameId, format, quality);
+        const dataUrl = await this._getScreenshot(windowId, tabId, frameId, format, quality);
 
         const {mediaType, data} = this._getDataUrlInfo(dataUrl);
         const extension = this._mediaUtility.getFileExtensionFromImageMediaType(mediaType);

ext/bg/js/search-main.js

@@ -32,12 +32,14 @@
         api.forwardLogsToBackend();
         await yomichan.backendReady();
 
+        const {tabId, frameId} = await api.frameInformationGet();
+
         const japaneseUtil = new JapaneseUtil(wanakana);
 
         const hotkeyHandler = new HotkeyHandler();
         hotkeyHandler.prepare();
 
-        const displaySearch = new DisplaySearch(japaneseUtil, documentFocusController, hotkeyHandler);
+        const displaySearch = new DisplaySearch(tabId, frameId, japaneseUtil, documentFocusController, hotkeyHandler);
         await displaySearch.prepare();
 
         document.documentElement.dataset.loaded = 'true';

ext/bg/js/search.js

@@ -23,8 +23,8 @@
  */
 
 class DisplaySearch extends Display {
-    constructor(japaneseUtil, documentFocusController, hotkeyHandler) {
+    constructor(tabId, frameId, japaneseUtil, documentFocusController, hotkeyHandler) {
-        super('search', japaneseUtil, documentFocusController, hotkeyHandler);
+        super('search', tabId, frameId, japaneseUtil, documentFocusController, hotkeyHandler);
         this._searchButton = document.querySelector('#search-button');
         this._queryInput = document.querySelector('#search-textbox');
         this._introElement = document.querySelector('#intro');
@@ -353,7 +353,11 @@ class DisplaySearch extends Display {
             },
             content: {
                 definitions: null,
-                animate
+                animate,
+                contentOrigin: {
+                    tabId: this.tabId,
+                    frameId: this.frameId
+                }
             }
         };
         if (!lookup) { details.params.lookup = 'false'; }

ext/bg/js/settings/popup-preview-frame-main.js

@@ -26,7 +26,7 @@
     try {
         api.forwardLogsToBackend();
 
-        const {frameId} = await api.frameInformationGet();
+        const {tabId, frameId} = await api.frameInformationGet();
 
         const hotkeyHandler = new HotkeyHandler();
         hotkeyHandler.prepare();
@@ -34,7 +34,7 @@
         const popupFactory = new PopupFactory(frameId);
         popupFactory.prepare();
 
-        const preview = new PopupPreviewFrame(frameId, popupFactory, hotkeyHandler);
+        const preview = new PopupPreviewFrame(tabId, frameId, popupFactory, hotkeyHandler);
         await preview.prepare();
 
         document.documentElement.dataset.loaded = 'true';

ext/bg/js/settings/popup-preview-frame.js

@@ -23,7 +23,8 @@
  */
 
 class PopupPreviewFrame {
-    constructor(frameId, popupFactory, hotkeyHandler) {
+    constructor(tabId, frameId, popupFactory, hotkeyHandler) {
+        this._tabId = tabId;
         this._frameId = frameId;
         this._popupFactory = popupFactory;
         this._hotkeyHandler = hotkeyHandler;
@@ -67,6 +68,7 @@ class PopupPreviewFrame {
 
         // Overwrite frontend
         this._frontend = new Frontend({
+            tabId: this._tabId,
             frameId: this._frameId,
             popupFactory: this._popupFactory,
             depth: 0,

ext/fg/js/content-script-main.js

@@ -27,7 +27,7 @@
         api.forwardLogsToBackend();
         await yomichan.backendReady();
 
-        const {frameId} = await api.frameInformationGet();
+        const {tabId, frameId} = await api.frameInformationGet();
         if (typeof frameId !== 'number') {
             throw new Error('Failed to get frameId');
         }
@@ -39,6 +39,7 @@
         popupFactory.prepare();
 
         const frontend = new Frontend({
+            tabId,
             frameId,
             popupFactory,
             depth: 0,

ext/fg/js/float-main.js

@@ -32,12 +32,14 @@
         api.forwardLogsToBackend();
         await yomichan.backendReady();
 
+        const {tabId, frameId} = await api.frameInformationGet();
+
         const japaneseUtil = new JapaneseUtil(null);
 
         const hotkeyHandler = new HotkeyHandler();
         hotkeyHandler.prepare();
 
-        const display = new Display('popup', japaneseUtil, documentFocusController, hotkeyHandler);
+        const display = new Display(tabId, frameId, 'popup', japaneseUtil, documentFocusController, hotkeyHandler);
         await display.prepare();
         const displayProfileSelection = new DisplayProfileSelection(display);
         displayProfileSelection.prepare();

ext/fg/js/frontend.js

@@ -28,6 +28,7 @@ class Frontend {
         pageType,
         popupFactory,
         depth,
+        tabId,
         frameId,
         parentPopupId,
         parentFrameId,
@@ -40,6 +41,7 @@ class Frontend {
         this._pageType = pageType;
         this._popupFactory = popupFactory;
         this._depth = depth;
+        this._tabId = tabId;
         this._frameId = frameId;
         this._parentPopupId = parentPopupId;
         this._parentFrameId = parentFrameId;
@@ -437,7 +439,6 @@ class Frontend {
 
         return await this._popupFactory.getOrCreatePopup({
             frameId: this._frameId,
-            ownerFrameId: this._frameId,
             depth: this._depth,
             childrenSupported: this._childrenSupported
         });
@@ -446,7 +447,6 @@ class Frontend {
     async _getProxyPopup() {
         return await this._popupFactory.getOrCreatePopup({
             frameId: this._parentFrameId,
-            ownerFrameId: this._frameId,
             depth: this._depth,
             parentPopupId: this._parentPopupId,
             childrenSupported: this._childrenSupported
@@ -469,7 +469,6 @@ class Frontend {
 
         const popup = await this._popupFactory.getOrCreatePopup({
             frameId: targetFrameId,
-            ownerFrameId: this._frameId,
             id: popupId,
             childrenSupported: this._childrenSupported
         });
@@ -482,7 +481,6 @@ class Frontend {
 
     async _getPopupWindow() {
         return await this._popupFactory.getOrCreatePopup({
-            ownerFrameId: this._frameId,
             depth: this._depth,
             popupWindow: true,
             childrenSupported: this._childrenSupported
@@ -537,7 +535,11 @@ class Frontend {
                 documentTitle
             },
             content: {
-                definitions
+                definitions,
+                contentOrigin: {
+                    tabId: this._tabId,
+                    frameId: this._frameId
+                }
             }
         };
         if (textSource instanceof TextSourceElement && textSource.fullContent !== query) {

ext/fg/js/popup-factory.js

@@ -56,7 +56,6 @@ class PopupFactory {
 
     async getOrCreatePopup({
         frameId=null,
-        ownerFrameId=null,
         id=null,
         parentPopupId=null,
         depth=null,
@@ -103,8 +102,7 @@ class PopupFactory {
             const popup = new PopupWindow({
                 id,
                 depth,
-                frameId: this._frameId,
+                frameId: this._frameId
-                ownerFrameId
             });
             this._popups.set(id, popup);
             return popup;
@@ -117,7 +115,6 @@ class PopupFactory {
                 id,
                 depth,
                 frameId: this._frameId,
-                ownerFrameId,
                 childrenSupported
             });
             if (parent !== null) {
@@ -139,14 +136,12 @@ class PopupFactory {
                 id,
                 parentPopupId,
                 frameId,
-                ownerFrameId,
                 childrenSupported
             }));
             const popup = new PopupProxy({
                 id,
                 depth,
                 frameId,
-                ownerFrameId,
                 frameOffsetForwarder: useFrameOffsetForwarder ? this._frameOffsetForwarder : null
             });
             this._popups.set(id, popup);

ext/fg/js/popup-proxy.js

@@ -24,14 +24,12 @@ class PopupProxy extends EventDispatcher {
         id,
         depth,
         frameId,
-        ownerFrameId,
         frameOffsetForwarder
     }) {
         super();
         this._id = id;
         this._depth = depth;
         this._frameId = frameId;
-        this._ownerFrameId = ownerFrameId;
         this._frameOffsetForwarder = frameOffsetForwarder;
 
         this._frameOffset = [0, 0];

ext/fg/js/popup-window.js

@@ -23,14 +23,12 @@ class PopupWindow extends EventDispatcher {
     constructor({
         id,
         depth,
-        frameId,
+        frameId
-        ownerFrameId
     }) {
         super();
         this._id = id;
         this._depth = depth;
         this._frameId = frameId;
-        this._ownerFrameId = ownerFrameId;
         this._popupTabId = null;
     }
 

ext/fg/js/popup.js

@@ -27,14 +27,12 @@ class Popup extends EventDispatcher {
         id,
         depth,
         frameId,
-        ownerFrameId,
         childrenSupported
     }) {
         super();
         this._id = id;
         this._depth = depth;
         this._frameId = frameId;
-        this._ownerFrameId = ownerFrameId;
         this._childrenSupported = childrenSupported;
         this._parent = null;
         this._child = null;
@@ -275,7 +273,6 @@ class Popup extends EventDispatcher {
             depth: this._depth,
             parentPopupId: this._id,
             parentFrameId: this._frameId,
-            ownerFrameId: this._ownerFrameId,
             childrenSupported: this._childrenSupported,
             scale: this._contentScale,
             optionsContext: this._optionsContext

ext/mixed/js/display.js

@@ -36,8 +36,10 @@
  */
 
 class Display extends EventDispatcher {
-    constructor(pageType, japaneseUtil, documentFocusController, hotkeyHandler) {
+    constructor(tabId, frameId, pageType, japaneseUtil, documentFocusController, hotkeyHandler) {
         super();
+        this._tabId = tabId;
+        this._frameId = frameId;
         this._pageType = pageType;
         this._japaneseUtil = japaneseUtil;
         this._documentFocusController = documentFocusController;
@@ -98,7 +100,8 @@ class Display extends EventDispatcher {
         this._depth = 0;
         this._parentPopupId = null;
         this._parentFrameId = null;
-        this._ownerFrameId = null;
+        this._contentOriginTabId = tabId;
+        this._contentOriginFrameId = frameId;
         this._childrenSupported = true;
         this._frameEndpoint = (pageType === 'popup' ? new FrameEndpoint() : null);
         this._browser = null;
@@ -199,6 +202,14 @@ class Display extends EventDispatcher {
         return this._progressIndicatorVisible;
     }
 
+    get tabId() {
+        return this._tabId;
+    }
+
+    get frameId() {
+        return this._frameId;
+    }
+
     async prepare() {
         // State setup
         const {documentElement} = document;
@@ -246,6 +257,13 @@ class Display extends EventDispatcher {
         }
     }
 
+    getContentOrigin() {
+        return {
+            tabId: this._contentOriginTabId,
+            frameId: this._contentOriginFrameId
+        };
+    }
+
     initializeState() {
         this._onStateChanged();
         if (this._frameEndpoint !== null) {
@@ -394,7 +412,7 @@ class Display extends EventDispatcher {
     close() {
         switch (this._pageType) {
             case 'popup':
-                this._invokeOwner('closePopup');
+                this._invokeContentOrigin('closePopup');
                 break;
             case 'search':
                 this._closeTab();
@@ -427,7 +445,8 @@ class Display extends EventDispatcher {
             params: this._createSearchParams(type, query, false),
             state,
             content: {
-                definitions: null
+                definitions: null,
+                contentOrigin: this.getContentOrigin()
             }
         };
         this.setContent(details);
@@ -494,14 +513,10 @@ class Display extends EventDispatcher {
         this._setContentScale(scale);
     }
 
-    async _onMessageConfigure({depth, parentPopupId, parentFrameId, ownerFrameId, childrenSupported, scale, optionsContext}) {
+    async _onMessageConfigure({depth, parentPopupId, parentFrameId, childrenSupported, scale, optionsContext}) {
         this._depth = depth;
         this._parentPopupId = parentPopupId;
         this._parentFrameId = parentFrameId;
-        this._ownerFrameId = ownerFrameId;
-        if (this._pageType === 'popup') {
-            this._hotkeyHandler.forwardFrameId = ownerFrameId;
-        }
         this._childrenSupported = childrenSupported;
         this._setContentScale(scale);
         await this.setOptionsContext(optionsContext);
@@ -615,7 +630,8 @@ class Display extends EventDispatcher {
                 cause: 'queryParser'
             },
             content: {
-                definitions
+                definitions,
+                contentOrigin: this.getContentOrigin()
             }
         };
         this.setContent(details);
@@ -629,7 +645,12 @@ class Display extends EventDispatcher {
             history: false,
             params: {type},
             state: {},
-            content: {}
+            content: {
+                contentOrigin: {
+                    tabId: this._tabId,
+                    frameId: this._frameId
+                }
+            }
         };
         this.setContent(details);
     }
@@ -691,7 +712,8 @@ class Display extends EventDispatcher {
                     documentTitle
                 },
                 content: {
-                    definitions
+                    definitions,
+                    contentOrigin: this.getContentOrigin()
                 }
             };
             this.setContent(details);
@@ -886,6 +908,24 @@ class Display extends EventDispatcher {
             changeHistory = true;
         }
 
+        let contentOriginValid = false;
+        const {contentOrigin} = content;
+        if (typeof contentOrigin === 'object' && contentOrigin !== null) {
+            const {tabId, frameId} = contentOrigin;
+            if (typeof tabId === 'number' && typeof frameId === 'number') {
+                this._contentOriginTabId = tabId;
+                this._contentOriginFrameId = frameId;
+                if (this._pageType === 'popup') {
+                    this._hotkeyHandler.forwardFrameId = (tabId === this._tabId ? frameId : null);
+                }
+                contentOriginValid = true;
+            }
+        }
+        if (!contentOriginValid) {
+            content.contentOrigin = this.getContentOrigin();
+            changeHistory = true;
+        }
+
         await this._setOptionsContextIfDifferent(optionsContext);
         if (this._setContentToken !== token) { return; }
 
@@ -1499,10 +1539,10 @@ class Display extends EventDispatcher {
         } = options;
 
         const timestamp = Date.now();
-        const ownerFrameId = this._ownerFrameId;
+        const screenshotFrameId = this._contentOriginFrameId;
         const definitionDetails = this._getDefinitionDetailsForNote(definition);
         const audioDetails = (mode !== 'kanji' && this._ankiNoteBuilder.containsMarker(fields, 'audio') ? {sources, customSourceUrl, customSourceType} : null);
-        const screenshotDetails = (this._ankiNoteBuilder.containsMarker(fields, 'screenshot') ? {ownerFrameId, format, quality} : null);
+        const screenshotDetails = (this._ankiNoteBuilder.containsMarker(fields, 'screenshot') ? {frameId: screenshotFrameId, format, quality} : null);
         const clipboardDetails = {
             image: this._ankiNoteBuilder.containsMarker(fields, 'clipboard-image'),
             text: this._ankiNoteBuilder.containsMarker(fields, 'clipboard-text')
@@ -1583,8 +1623,6 @@ class Display extends EventDispatcher {
             parentFrameId: this._parentFrameId
         };
 
-        const {frameId} = await api.frameInformationGet();
-
         await dynamicLoader.loadScripts([
             '/mixed/js/text-scanner.js',
             '/mixed/js/frame-client.js',
@@ -1597,12 +1635,13 @@ class Display extends EventDispatcher {
             '/fg/js/frontend.js'
         ]);
 
-        const popupFactory = new PopupFactory(frameId);
+        const popupFactory = new PopupFactory(this._frameId);
         popupFactory.prepare();
 
         Object.assign(setupNestedPopupsOptions, {
             depth: this._depth + 1,
-            frameId,
+            tabId: this._tabId,
+            frameId: this._frameId,
             popupFactory,
             pageType: this._pageType,
             allowRootFramePopupProxy: true,
@@ -1615,15 +1654,15 @@ class Display extends EventDispatcher {
         await frontend.prepare();
     }
 
-    async _invokeOwner(action, params={}) {
+    async _invokeContentOrigin(action, params={}) {
-        if (this._ownerFrameId === null) {
+        if (this._contentOriginTabId === this._tabId && this._contentOriginFrameId === this._frameId) {
-            throw new Error('No owner frame');
+            throw new Error('Content origin is same page');
         }
-        return await api.crossFrame.invoke(this._ownerFrameId, action, params);
+        return await api.crossFrame.invokeTab(this._contentOriginTabId, this._contentOriginFrameId, action, params);
     }
 
     _copyHostSelection() {
-        if (this._ownerFrameId === null || window.getSelection().toString()) { return false; }
+        if (this._contentOriginFrameId === null || window.getSelection().toString()) { return false; }
         this._copyHostSelectionInner();
         return true;
     }
@@ -1635,7 +1674,7 @@ class Display extends EventDispatcher {
                 {
                     let text;
                     try {
-                        text = await this._invokeOwner('getSelectionText');
+                        text = await this._invokeContentOrigin('getSelectionText');
                     } catch (e) {
                         break;
                     }
@@ -1643,7 +1682,7 @@ class Display extends EventDispatcher {
                 }
                 break;
             default:
-                await this._invokeOwner('copySelection');
+                await this._invokeContentOrigin('copySelection');
                 break;
         }
     }
@@ -1760,7 +1799,8 @@ class Display extends EventDispatcher {
                 documentTitle
             },
             content: {
-                definitions
+                definitions,
+                contentOrigin: this.getContentOrigin()
             }
         };
         this._definitionTextScanner.clearSelection(true);
@@ -1816,7 +1856,7 @@ class Display extends EventDispatcher {
     }
 
     async _initializeFrameResize(token) {
-        const size = await this._invokeOwner('getFrameSize');
+        const size = await this._invokeContentOrigin('getFrameSize');
         if (this._frameResizeToken !== token) { return; }
         this._frameResizeStartSize = size;
     }
@@ -1842,7 +1882,7 @@ class Display extends EventDispatcher {
         height += y - this._frameResizeStartOffset.y;
         width = Math.max(Math.max(0, handleSize.width), width);
         height = Math.max(Math.max(0, handleSize.height), height);
-        await this._invokeOwner('setFrameSize', {width, height});
+        await this._invokeContentOrigin('setFrameSize', {width, height});
     }
 
     _updateHotkeys(options) {