Popup factory proxy creation (#745)

* Change getOrCreatePopup to async

* Rename parentFrameId to frameId, expose frameId property

* Update how proxy popups are created

ext/fg/js/frontend.js

@@ -17,8 +17,6 @@
 
 /* global
  * DocumentUtil
- * FrameOffsetForwarder
- * PopupProxy
  * TextScanner
  * TextSourceElement
  * api
@@ -58,7 +56,6 @@ class Frontend {
         this._isSearchPage = isSearchPage;
         this._depth = depth;
         this._frameId = frameId;
-        this._frameOffsetForwarder = new FrameOffsetForwarder(frameId);
         this._popupFactory = popupFactory;
         this._allowRootFramePopupProxy = allowRootFramePopupProxy;
         this._popupCache = new Map();
@@ -83,8 +80,6 @@ class Frontend {
     }
 
     async prepare() {
-        this._frameOffsetForwarder.prepare();
-
         await this.updateOptions();
         try {
             const {zoomFactor} = await api.getZoom();
@@ -333,13 +328,20 @@ class Frontend {
             return null;
         }
 
-        return this._popupFactory.getOrCreatePopup({depth: this._depth, ownerFrameId: this._frameId});
+        return await this._popupFactory.getOrCreatePopup({
+            frameId: this._frameId,
+            ownerFrameId: this._frameId,
+            depth: this._depth
+        });
     }
 
     async _getProxyPopup() {
-        const popup = new PopupProxy(null, this._depth, this._parentPopupId, this._parentFrameId, this._frameId);
+        return await this._popupFactory.getOrCreatePopup({
-        await popup.prepare();
+            frameId: this._parentFrameId,
-        return popup;
+            ownerFrameId: this._frameId,
+            depth: this._depth,
+            parentPopupId: this._parentPopupId
+        });
     }
 
     async _getIframeProxyPopup() {
@@ -351,13 +353,15 @@ class Frontend {
             return null;
         }
 
-        const popup = new PopupProxy(popupId, 0, null, targetFrameId, this._frameId, this._frameOffsetForwarder);
+        const popup = await this._popupFactory.getOrCreatePopup({
+            frameId: targetFrameId,
+            ownerFrameId: this._frameId,
+            id: popupId
+        });
         popup.on('offsetNotFound', () => {
             this._allowRootFramePopupProxy = false;
             this._updatePopup();
         });
-        await popup.prepare();
-
         return popup;
     }
 

ext/fg/js/popup-factory.js

@@ -16,21 +16,25 @@
  */
 
 /* global
+ * FrameOffsetForwarder
  * Popup
+ * PopupProxy
  * api
  */
 
 class PopupFactory {
     constructor(frameId) {
-        this._popups = new Map();
         this._frameId = frameId;
+        this._frameOffsetForwarder = new FrameOffsetForwarder(frameId);
+        this._popups = new Map();
     }
 
     // Public functions
 
     prepare() {
+        this._frameOffsetForwarder.prepare();
         api.crossFrame.registerHandlers([
-            ['getOrCreatePopup',     {async: false, handler: this._onApiGetOrCreatePopup.bind(this)}],
+            ['getOrCreatePopup',     {async: true,  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)}],
@@ -46,7 +50,7 @@ class PopupFactory {
         ]);
     }
 
-    getOrCreatePopup({id=null, parentId=null, ownerFrameId=null, depth=null}) {
+    async getOrCreatePopup({frameId=null, ownerFrameId=null, id=null, parentPopupId=null, depth=null}) {
         // Find by existing id
         if (id !== null) {
             const popup = this._popups.get(id);
@@ -57,8 +61,8 @@ class PopupFactory {
 
         // Find by existing parent id
         let parent = null;
-        if (parentId !== null) {
+        if (parentPopupId !== null) {
-            parent = this._popups.get(parentId);
+            parent = this._popups.get(parentPopupId);
             if (typeof parent !== 'undefined') {
                 const popup = parent.child;
                 if (popup !== null) {
@@ -69,12 +73,7 @@ class PopupFactory {
             }
         }
 
-        // New unique id
+        // Depth
-        if (id === null) {
-            id = yomichan.generateId(16);
-        }
-
-        // Create new popup
         if (parent !== null) {
             if (depth !== null) {
                 throw new Error('Depth cannot be set when parent exists');
@@ -83,7 +82,13 @@ class PopupFactory {
         } else if (depth === null) {
             depth = 0;
         }
-        const popup = new Popup(id, depth, this._frameId, ownerFrameId);
+
+        if (frameId === this._frameId) {
+            // New unique id
+            if (id === null) {
+                id = yomichan.generateId(16);
+            }
+            const popup = new Popup(id, depth, frameId, ownerFrameId);
             if (parent !== null) {
                 if (parent.child !== null) {
                     throw new Error('Parent popup already has a child');
@@ -94,14 +99,23 @@ class PopupFactory {
             this._popups.set(id, popup);
             popup.prepare();
             return popup;
+        } else {
+            const useFrameOffsetForwarder = (parentPopupId === null);
+            ({id, depth, frameId} = await api.crossFrame.invoke(frameId, 'getOrCreatePopup', {id, parentPopupId, frameId, ownerFrameId}));
+            const popup = new PopupProxy(id, depth, frameId, ownerFrameId, useFrameOffsetForwarder ? this._frameOffsetForwarder : null);
+            this._popups.set(id, popup);
+            return popup;
+        }
     }
 
     // API message handlers
 
-    _onApiGetOrCreatePopup({id, parentId, ownerFrameId}) {
+    async _onApiGetOrCreatePopup({id, parentPopupId, frameId, ownerFrameId}) {
-        const popup = this.getOrCreatePopup({id, parentId, ownerFrameId});
+        const popup = await this.getOrCreatePopup({id, parentPopupId, frameId, ownerFrameId});
         return {
-            id: popup.id
+            id: popup.id,
+            depth: popup.depth,
+            frameId: popup.frameId
         };
     }
 

ext/fg/js/popup-proxy.js

@@ -20,12 +20,11 @@
  */
 
 class PopupProxy extends EventDispatcher {
-    constructor(id, depth, parentPopupId, parentFrameId, ownerFrameId, frameOffsetForwarder=null) {
+    constructor(id, depth, frameId, ownerFrameId, frameOffsetForwarder=null) {
         super();
         this._id = id;
         this._depth = depth;
-        this._parentPopupId = parentPopupId;
+        this._frameId = frameId;
-        this._parentFrameId = parentFrameId;
         this._ownerFrameId = ownerFrameId;
         this._frameOffsetForwarder = frameOffsetForwarder;
 
@@ -69,13 +68,12 @@ class PopupProxy extends EventDispatcher {
         return null;
     }
 
-    // Public functions
+    get frameId() {
-
+        return this._frameId;
-    async prepare() {
-        const {id} = await this._invoke('getOrCreatePopup', {id: this._id, parentId: this._parentPopupId, ownerFrameId: this._ownerFrameId});
-        this._id = id;
     }
 
+    // Public functions
+
     setOptionsContext(optionsContext, source) {
         return this._invokeSafe('setOptionsContext', {id: this._id, optionsContext, source});
     }
@@ -148,7 +146,7 @@ class PopupProxy extends EventDispatcher {
     // Private
 
     _invoke(action, params={}) {
-        return api.crossFrame.invoke(this._parentFrameId, action, params);
+        return api.crossFrame.invoke(this._frameId, action, params);
     }
 
     async _invokeSafe(action, params={}, defaultReturnValue) {

ext/fg/js/popup.js

@@ -88,6 +88,10 @@ class Popup {
         return this._container;
     }
 
+    get frameId() {
+        return this._frameId;
+    }
+
     // Public functions
 
     prepare() {