From b612bd8b8dc62a83e6800b052cc5f673a287dbe8 Mon Sep 17 00:00:00 2001
From: toasted-nutbread <toasted-nutbread@users.noreply.github.com>
Date: Sun, 14 Jun 2020 14:06:52 -0400
Subject: [PATCH] PopupProxy refactor (#609)

* Remove setDisabled member; replace with an event

* Pass frameOffsetForwarder directly to PopupProxy

* Replace .start with .prepare

* Make onMessage private

* Make message safer and handle unexpected inputs
---
 ext/fg/js/content-script-main.js    |  7 +++----
 ext/fg/js/frame-offset-forwarder.js | 30 ++++++++++++++++++-----------
 ext/fg/js/popup-proxy.js            | 18 ++++++++---------
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/ext/fg/js/content-script-main.js b/ext/fg/js/content-script-main.js
index cebda2d7..c31cde3f 100644
--- a/ext/fg/js/content-script-main.js
+++ b/ext/fg/js/content-script-main.js
@@ -49,9 +49,8 @@ async function createIframePopupProxy(frameOffsetForwarder, setDisabled) {
     api.broadcastTab('rootPopupRequestInformationBroadcast');
     const {popupId, frameId: parentFrameId} = await rootPopupInformationPromise;
 
-    const getFrameOffset = frameOffsetForwarder.getOffset.bind(frameOffsetForwarder);
-
-    const popup = new PopupProxy(popupId, 0, null, parentFrameId, getFrameOffset, setDisabled);
+    const popup = new PopupProxy(popupId, 0, null, parentFrameId, frameOffsetForwarder);
+    popup.on('offsetNotFound', setDisabled);
     await popup.prepare();
 
     return popup;
@@ -115,7 +114,7 @@ async function createPopupProxy(depth, id, parentFrameId) {
 
         if (!proxy && frameOffsetForwarder === null) {
             frameOffsetForwarder = new FrameOffsetForwarder();
-            frameOffsetForwarder.start();
+            frameOffsetForwarder.prepare();
         }
 
         let popup;
diff --git a/ext/fg/js/frame-offset-forwarder.js b/ext/fg/js/frame-offset-forwarder.js
index 10e3b5be..f692364a 100644
--- a/ext/fg/js/frame-offset-forwarder.js
+++ b/ext/fg/js/frame-offset-forwarder.js
@@ -21,8 +21,7 @@
 
 class FrameOffsetForwarder {
     constructor() {
-        this._started = false;
-
+        this._isPrepared = false;
         this._cacheMaxSize = 1000;
         this._frameCache = new Set();
         this._unreachableContentWindowCache = new Set();
@@ -38,10 +37,10 @@ class FrameOffsetForwarder {
         ]);
     }
 
-    start() {
-        if (this._started) { return; }
-        window.addEventListener('message', this.onMessage.bind(this), false);
-        this._started = true;
+    prepare() {
+        if (this._isPrepared) { return; }
+        window.addEventListener('message', this._onMessage.bind(this), false);
+        this._isPrepared = true;
     }
 
     async getOffset() {
@@ -69,11 +68,20 @@ class FrameOffsetForwarder {
         return offset;
     }
 
-    onMessage(e) {
-        const {action, params} = e.data;
-        const handler = this._windowMessageHandlers.get(action);
-        if (typeof handler !== 'function') { return; }
-        handler(params, e);
+    // Private
+
+    _onMessage(event) {
+        const data = event.data;
+        if (data === null || typeof data !== 'object') { return; }
+
+        try {
+            const {action, params} = event.data;
+            const handler = this._windowMessageHandlers.get(action);
+            if (typeof handler !== 'function') { return; }
+            handler(params, event);
+        } catch (e) {
+            // NOP
+        }
     }
 
     _onGetFrameOffset(offset, uniqueId, e) {
diff --git a/ext/fg/js/popup-proxy.js b/ext/fg/js/popup-proxy.js
index 3387d941..14ddcafb 100644
--- a/ext/fg/js/popup-proxy.js
+++ b/ext/fg/js/popup-proxy.js
@@ -19,14 +19,14 @@
  * api
  */
 
-class PopupProxy {
-    constructor(id, depth, parentPopupId, parentFrameId, getFrameOffset=null, setDisabled=null) {
+class PopupProxy extends EventDispatcher {
+    constructor(id, depth, parentPopupId, parentFrameId, frameOffsetForwarder=null) {
+        super();
         this._id = id;
         this._depth = depth;
         this._parentPopupId = parentPopupId;
         this._parentFrameId = parentFrameId;
-        this._getFrameOffset = getFrameOffset;
-        this._setDisabled = setDisabled;
+        this._frameOffsetForwarder = frameOffsetForwarder;
 
         this._frameOffset = null;
         this._frameOffsetPromise = null;
@@ -75,7 +75,7 @@ class PopupProxy {
     }
 
     async containsPoint(x, y) {
-        if (this._getFrameOffset !== null) {
+        if (this._frameOffsetForwarder !== null) {
             await this._updateFrameOffset();
             [x, y] = this._applyFrameOffset(x, y);
         }
@@ -84,7 +84,7 @@ class PopupProxy {
 
     async showContent(elementRect, writingMode, type, details, context) {
         let {x, y, width, height} = elementRect;
-        if (this._getFrameOffset !== null) {
+        if (this._frameOffsetForwarder !== null) {
             await this._updateFrameOffset();
             [x, y] = this._applyFrameOffset(x, y);
         }
@@ -134,12 +134,12 @@ class PopupProxy {
     }
 
     async _updateFrameOffsetInner(now) {
-        this._frameOffsetPromise = this._getFrameOffset();
+        this._frameOffsetPromise = this._frameOffsetForwarder.getOffset();
         try {
             const offset = await this._frameOffsetPromise;
             this._frameOffset = offset !== null ? offset : [0, 0];
-            if (offset === null && this._setDisabled !== null) {
-                this._setDisabled();
+            if (offset === null) {
+                this.trigger('offsetNotFound');
                 return;
             }
             this._frameOffsetUpdatedAt = now;