Fix RPC mechanism being broken

The problem was that in trying to selectively answer to messages from
the neovim frame, the content script was made deaf to any request from
the backround script. This is fixed by ordering functions into
categories, e.g. global, active-content and "frame" and performing
checks tied to the categories before answering.

src/FirenvimElement.ts

@@ -66,6 +66,16 @@ export class FirenvimElement {
     // the FirenvimElement. It is called when the detach() function is called,
     // after all Firenvim elements have been removed from the page.
     private onDetach: (id: number) => any;
+    // bufferInfo: a [url, selector, cursor, lang] tuple indicating the page
+    // the last iframe was created on, the selector of the corresponding
+    // textarea and the column/line number of the cursor.
+    // Note that these are __default__ values. Real values must be created with
+    // prepareBufferInfo(). The reason we're not doing this from the
+    // constructor is that it's expensive and disruptive - getting this
+    // information requires evaluating code in the page's context.
+    private bufferInfo = (Promise.resolve(["", "", [1, 1], undefined]) as
+                          Promise<[string, string, [number, number], string]>);
+
 
     // elem is the element that received the focusEvent.
     // Nvimify is the function that listens for focus events. We need to know
@@ -212,6 +222,10 @@ export class FirenvimElement {
         }
     }
 
+    getBufferInfo () {
+        return this.bufferInfo;
+    }
+
     getEditor () {
         return this.editor;
     }
@@ -245,6 +259,15 @@ export class FirenvimElement {
             || document.activeElement === this.iframe;
     }
 
+    prepareBufferInfo () {
+        this.bufferInfo = new Promise(async r => r([
+            document.location.href,
+            this.getSelector(),
+            await (this.editor.getCursor().catch(() => [1, 1])),
+            await (this.editor.getLanguage().catch(() => undefined))
+        ]));
+    }
+
     pressKeys (keys: KeyboardEvent[]) {
         keys.forEach(ev => this.originalElement.dispatchEvent(ev));
         this.focus();

src/NeovimFrame.ts

@@ -9,7 +9,7 @@ const frameIdPromise = browser
     .runtime
     .sendMessage({ funcName: ["publishFrameId"] })
     .then((f: number) => (window as any).frameId = f);
-const infoPromise = frameIdPromise.then(_ => page.getEditorInfo());
+const infoPromise = frameIdPromise.then(() => page.getEditorInfo());
 const connectionPromise = browser.runtime.sendMessage({ funcName: ["getNeovimInstance"] });
 
 export const isReady = new Promise((resolve, reject) => {

src/background.ts

@@ -284,6 +284,7 @@ Object.assign(window, {
         return result.then(({ password, port }) => ({ password, port }));
     },
     getNvimPluginVersion: () => nvimPluginVersion,
+    getOwnFrameId: (sender: any) => sender.frameId,
     getTab: (sender: any) => sender.tab,
     getTabValue: (sender: any, args: any) => getTabValue(sender.tab.id, args[0]),
     getTabValueFor: (_: any, args: any) => getTabValue(args[0], args[1]),

src/content.ts

@@ -1,5 +1,5 @@
 import { autofill } from "./autofill";
-import { getFunctions } from "./page/functions";
+import { getNeovimFrameFunctions, getActiveContentFunctions, getTabFunctions } from "./page/functions";
 import { confReady, getConf } from "./utils/configuration";
 import { FirenvimElement } from "./FirenvimElement";
 
@@ -10,9 +10,6 @@ if (document.location.href === "https://github.com/glacambre/firenvim/issues/new
 // Promise used to implement a locking mechanism preventing concurrent creation
 // of neovim frames
 let frameIdLock = Promise.resolve();
-// Promise-resolution function called when a frameId is received from the
-// background script
-let frameIdResolve = (_: number): void => undefined;
 
 const global = {
     // Whether Firenvim is disabled in this tab
@@ -23,10 +20,17 @@ const global = {
         // Note: this relies on setDisabled existing in the object returned by
         // getFunctions and attached to the window object
         .then((disabled: boolean) => (window as any).setDisabled(!!disabled)),
-    // lastBufferInfo: a [url, selector, cursor] tuple indicating the page
-    // the last iframe was created on, the selector of the corresponding
-    // textarea and the column/line number of the cursor.
-    lastBufferInfo: ["", "", [1, 1], undefined] as [string, string, [number, number], string],
+    // Promise-resolution function called when a frameId is received from the
+    // background script
+    frameIdResolve: (_: number): void => undefined,
+    // lastFocusedContentScript keeps track of the last content frame that has
+    // been focused. This is necessary in pages that contain multiple frames
+    // (and thus multiple content scripts): for example, if users press the
+    // global keyboard shortcut <C-n>, the background script sends a "global"
+    // message to all of the active tab's content scripts. For a content script
+    // to know if it should react to a global message, it just needs to check
+    // if it is the last active content script.
+    lastFocusedContentScript: 0,
     // nvimify: triggered when an element is focused, takes care of creating
     // the editor iframe, appending it to the page and focusing it.
     nvimify: async (evt: { target: EventTarget }) => {
@@ -67,8 +71,7 @@ const global = {
                 }
         }
 
-        const cursorPromise = editor.getCursor();
-        const languagePromise = editor.getLanguage();
+        firenvim.prepareBufferInfo();
 
         // When creating new frames, we need to know their frameId in order to
         // communicate with them. This can't be retrieved through a
@@ -85,17 +88,11 @@ const global = {
             await frameIdLock;
         }
         frameIdLock = new Promise(async (unlock: any) => {
-            global.lastBufferInfo = [
-                document.location.href,
-                firenvim.getSelector(),
-                await (cursorPromise.catch(() => [1, 1])),
-                await (languagePromise.catch(() => undefined))
-            ];
-
             // TODO: make this timeout the same as the one in background.ts
             const frameIdPromise = new Promise((resolve: (_: number) => void, reject) => {
-                frameIdResolve = (frameId: number) => {
+                global.frameIdResolve = (frameId: number) => {
                     global.firenvimElems.set(frameId, firenvim);
+                    global.frameIdResolve = () => undefined;
                     resolve(frameId);
                 };
                 setTimeout(reject, 10000);
@@ -110,33 +107,52 @@ const global = {
     firenvimElems: new Map<number, FirenvimElement>(),
 };
 
-// This works as an rpc mechanism, allowing the frame script to perform calls
-// in the content script.
-const functions = getFunctions(global);
-Object.assign(window, functions);
-browser.runtime.onMessage.addListener(async (
-    request: { funcName: string[], args: any[] },
-    sender: any,
-    sendResponse: any,
-) => {
-    // We need to treat registerNewFrameId differently from other RPC messages
-    // because other RPC messages rely on frameId existing in firenvimElems!
-    if (request.funcName[0] === "registerNewFrameId") {
-        frameIdResolve(request.args[0]);
-        frameIdResolve = () => undefined;
-        return;
+let ownFrameId: number;
+browser.runtime.sendMessage({ args: [], funcName: ["getOwnFrameId"] })
+    .then((frameId: number) => { ownFrameId = frameId });
+window.addEventListener("focus", async () => {
+    const frameId = ownFrameId;
+    global.lastFocusedContentScript = frameId;
+    browser.runtime.sendMessage({
+        args: {
+            args: [ frameId ],
+            funcName: ["setLastFocusedContentScript"]
+        },
+        funcName: ["messagePage"]
+    });
+});
+
+const frameFunctions = getNeovimFrameFunctions(global);
+const activeFunctions = getActiveContentFunctions(global);
+const tabFunctions = getTabFunctions(global);
+Object.assign(window, frameFunctions, activeFunctions, tabFunctions);
+browser.runtime.onMessage.addListener(async (request: { funcName: string[], args: any[] }) => {
+    // All content scripts must react to tab functions
+    let fn = request.funcName.reduce((acc: any, cur: string) => acc[cur], tabFunctions);
+    if (fn !== undefined) {
+        return fn(...request.args);
     }
 
-    const fn = request.funcName.reduce((acc: any, cur: string) => acc[cur], window);
-    if (!fn) {
-        throw new Error(`Error: unhandled content request: ${JSON.stringify(request)}.`);
+    // The only content script that should react to activeFunctions is the active one
+    fn = request.funcName.reduce((acc: any, cur: string) => acc[cur], activeFunctions);
+    if (fn !== undefined) {
+        if (global.lastFocusedContentScript === ownFrameId) {
+            return fn(...request.args);
+        }
+        return new Promise(() => undefined);
     }
 
-    if (global.firenvimElems.get(request.args[0]) !== undefined) {
-        return fn(...request.args);
+    // The only content script that should react to frameFunctions is the one
+    // that owns the frame that sent the request
+    fn = request.funcName.reduce((acc: any, cur: string) => acc[cur], frameFunctions);
+    if (fn !== undefined) {
+        if (global.firenvimElems.get(request.args[0]) !== undefined) {
+            return fn(...request.args);
+        }
+        return new Promise(() => undefined);
     }
-    // If the message wasn't for us, return undefined to let other frames answer
-    return new Promise(():void => undefined);
+
+    throw new Error(`Error: unhandled content request: ${JSON.stringify(request)}.`);
 });
 
 function setupListeners(selector: string) {
@@ -207,13 +223,13 @@ function setupListeners(selector: string) {
         for (const mr of changes) {
             for (const node of mr.addedNodes) {
                 if (shouldNvimify(node)) {
-                    functions.forceNvimify();
+                    activeFunctions.forceNvimify();
                     return;
                 }
                 const walker = document.createTreeWalker(node, NodeFilter.SHOW_ELEMENT);
                 while (walker.nextNode()) {
                     if (shouldNvimify(walker.currentNode)) {
-                        functions.forceNvimify();
+                        activeFunctions.forceNvimify();
                         return;
                     }
                 }

src/page/functions.ts

@@ -4,10 +4,11 @@ import { FirenvimElement } from "../FirenvimElement";
 import { executeInPage } from "../utils/utils";
 
 interface IGlobalState {
-    lastBufferInfo: [string, string, [number, number], string];
-    nvimify: (evt: FocusEvent) => void;
-    firenvimElems: Map<number, FirenvimElement>;
     disabled: boolean | Promise<boolean>;
+    lastFocusedContentScript: number;
+    firenvimElems: Map<number, FirenvimElement>;
+    frameIdResolve: (_: number) => void;
+    nvimify: (evt: FocusEvent) => void;
 }
 
 function _focusInput(global: IGlobalState, firenvim: FirenvimElement, addListener: boolean) {
@@ -29,22 +30,25 @@ function getFocusedElement (firenvimElems: Map<number, FirenvimElement>) {
         .find(instance => instance.isFocused());
 }
 
-export function getFunctions(global: IGlobalState) {
+// Tab functions are functions all content scripts should react to
+export function getTabFunctions(global: IGlobalState) {
     return {
-        evalInPage: (_: number, js: string) => executeInPage(js),
-        focusInput: (frameId: number) => {
-            let firenvimElement;
-            if (frameId === undefined) {
-                firenvimElement = getFocusedElement(global.firenvimElems);
-            } else {
-                firenvimElement = global.firenvimElems.get(frameId);
-            }
-            _focusInput(global, firenvimElement, true);
+        getActiveInstanceCount : () => global.firenvimElems.size,
+        registerNewFrameId: (frameId: number) => {
+            global.frameIdResolve(frameId);
         },
-        focusPage: () => {
-            (document.activeElement as any).blur();
-            document.documentElement.focus();
+        setDisabled: (disabled: boolean) => {
+            global.disabled = disabled;
         },
+        setLastFocusedContentScript: (frameId: number) => {
+            global.lastFocusedContentScript = frameId;
+        }
+    };
+}
+
+// ActiveContent functions are functions only the active content script should react to
+export function getActiveContentFunctions(global: IGlobalState) {
+    return {
         forceNvimify: () => {
             let elem = document.activeElement;
             if (!elem || elem === document.documentElement || elem === document.body) {
@@ -65,10 +69,39 @@ export function getFunctions(global: IGlobalState) {
             }
             global.nvimify({ target: elem } as any);
         },
-        getActiveInstanceCount : () => global.firenvimElems.size,
-        getEditorInfo: () => {
-            return global.lastBufferInfo;
+        sendKey: (key: string) => {
+            const firenvim = getFocusedElement(global.firenvimElems);
+            if (firenvim !== undefined) {
+                firenvim.sendKey(key);
+            } else {
+                // It's important to throw this error as the background script
+                // will execute a fallback
+                throw new Error("No firenvim frame selected");
+            }
         },
+    };
+}
+
+export function getNeovimFrameFunctions(global: IGlobalState) {
+    return {
+        evalInPage: (_: number, js: string) => executeInPage(js),
+        focusInput: (frameId: number) => {
+            let firenvimElement;
+            if (frameId === undefined) {
+                firenvimElement = getFocusedElement(global.firenvimElems);
+            } else {
+                firenvimElement = global.firenvimElems.get(frameId);
+            }
+            _focusInput(global, firenvimElement, true);
+        },
+        focusPage: () => {
+            (document.activeElement as any).blur();
+            document.documentElement.focus();
+        },
+        getEditorInfo: (frameId: number) => global
+            .firenvimElems
+            .get(frameId)
+            .getBufferInfo(),
         getElementContent: (frameId: number) => global
             .firenvimElems
             .get(frameId)
@@ -93,19 +126,6 @@ export function getFunctions(global: IGlobalState) {
             elem.resizeTo(width, height, true);
             elem.putEditorCloseToInputOriginAfterResizeFromFrame();
         },
-        sendKey: (key: string) => {
-            const firenvim = getFocusedElement(global.firenvimElems);
-            if (firenvim !== undefined) {
-                firenvim.sendKey(key);
-            } else {
-                // It's important to throw this error as the background script
-                // will execute a fallback
-                throw new Error("No firenvim frame selected");
-            }
-        },
-        setDisabled: (disabled: boolean) => {
-            global.disabled = disabled;
-        },
         setElementContent: (frameId: number, text: string) => {
             return global.firenvimElems.get(frameId).setPageElementContent(text);
         },

src/page/proxy.ts

@@ -1,8 +1,8 @@
-import { getFunctions } from "./functions";
+import { getNeovimFrameFunctions } from "./functions";
 
 // We don't need to give real values to getFunctions since we're only trying to
 // get the name of functions that exist in the page.
-const functions = getFunctions({} as any);
+const functions = getNeovimFrameFunctions({} as any);
 
 type ft = typeof functions;
 // The proxy automatically appends the frameId to the request, so we hide that from users