Anki screenshot refactor (#791)

* Use more consistent style for injectScreenshot

* Move screenshot generation into AnkiNoteBuilder/Backend

* Get optionsContext before await

5 files changed, 68 insertions, 63 deletions

diff --git a/ext/bg/js/anki-note-builder.js b/ext/bg/js/anki-note-builder.js
index 4afb2d40..19e352f1 100644
--- a/ext/bg/js/anki-note-builder.js
+++ b/ext/bg/js/anki-note-builder.js
@@ -20,11 +20,12 @@
  */
 
 class AnkiNoteBuilder {
-    constructor({anki, audioSystem, renderTemplate, getClipboardImage=null}) {
+    constructor({anki, audioSystem, renderTemplate, getClipboardImage=null, getScreenshot=null}) {
         this._anki = anki;
         this._audioSystem = audioSystem;
         this._renderTemplate = renderTemplate;
         this._getClipboardImage = getClipboardImage;
+        this._getScreenshot = getScreenshot;
     }
 
     async createNote({
@@ -130,18 +131,23 @@ class AnkiNoteBuilder {
     async injectScreenshot(definition, fields, screenshot) {
         if (!this._containsMarker(fields, 'screenshot')) { return; }
 
+        const reading = definition.reading;
         const now = new Date(Date.now());
-        let fileName = `yomichan_browser_screenshot_${definition.reading}_${this._dateToString(now)}.${screenshot.format}`;
-        fileName = AnkiNoteBuilder.replaceInvalidFileNameCharacters(fileName);
-        const data = screenshot.dataUrl.replace(/^data:[\w\W]*?,/, '');
 
         try {
+            const {windowId, tabId, ownerFrameId, format, quality} = screenshot;
+            const dataUrl = await this._getScreenshot(windowId, tabId, ownerFrameId, format, quality);
+
+            let fileName = `yomichan_browser_screenshot_${reading}_${this._dateToString(now)}.${format}`;
+            fileName = AnkiNoteBuilder.replaceInvalidFileNameCharacters(fileName);
+            const data = dataUrl.replace(/^data:[\w\W]*?,/, '');
+
             await this._anki.storeMediaFile(fileName, data);
+
+            definition.screenshotFileName = fileName;
         } catch (e) {
-            return;
+            // NOP
         }
-
-        definition.screenshotFileName = fileName;
     }
 
     async injectClipboardImage(definition, fields) {
diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js
index 832dbc3a..47e68072 100644
--- a/ext/bg/js/backend.js
+++ b/ext/bg/js/backend.js
@@ -61,7 +61,8 @@ class Backend {
             anki: this._anki,
             audioSystem: this._audioSystem,
             renderTemplate: this._renderTemplate.bind(this),
-            getClipboardImage: this._onApiClipboardImageGet.bind(this)
+            getClipboardImage: this._onApiClipboardImageGet.bind(this),
+            getScreenshot: this._getScreenshot.bind(this)
         });
         this._templateRenderer = new TemplateRenderer();
 
@@ -442,7 +443,7 @@ class Backend {
         return results;
     }
 
-    async _onApiDefinitionAdd({definition, mode, context, details, optionsContext}) {
+    async _onApiDefinitionAdd({definition, mode, context, ownerFrameId, optionsContext}, sender) {
         const options = this.getOptions(optionsContext);
         const templates = this._getTemplates(options);
         const fields = (
@@ -463,13 +464,13 @@ class Backend {
 
         await this._ankiNoteBuilder.injectClipboardImage(definition, fields);
 
-        if (details && details.screenshot) {
-            await this._ankiNoteBuilder.injectScreenshot(
-                definition,
-                fields,
-                details.screenshot
-            );
-        }
+        const {id: tabId, windowId} = (sender && sender.tab ? sender.tab : {});
+        const {format, quality} = options.anki.screenshot;
+        await this._ankiNoteBuilder.injectScreenshot(
+            definition,
+            fields,
+            {windowId, tabId, ownerFrameId, format, quality}
+        );
 
         const note = await this._createNote(definition, mode, context, options, templates);
         return this._anki.addNote(note);
@@ -1626,4 +1627,40 @@ class Backend {
             modeOptions
         });
     }
+
+    async _getScreenshot(windowId, tabId, ownerFrameId, format, quality) {
+        if (typeof windowId !== 'number') {
+            throw new Error('Invalid window ID');
+        }
+
+        let token = null;
+        try {
+            if (typeof tabId === 'number' && typeof ownerFrameId === 'number') {
+                const action = 'setAllVisibleOverride';
+                const params = {value: false, priority: 0, awaitFrame: true};
+                token = await this._sendMessageTab(tabId, {action, params}, {frameId: ownerFrameId});
+            }
+
+            return await new Promise((resolve, reject) => {
+                chrome.tabs.captureVisibleTab(windowId, {format, quality}, (result) => {
+                    const e = chrome.runtime.lastError;
+                    if (e) {
+                        reject(new Error(e.message));
+                    } else {
+                        resolve(result);
+                    }
+                });
+            });
+        } finally {
+            if (token !== null) {
+                const action = 'clearAllVisibleOverride';
+                const params = {token};
+                try {
+                    await this._sendMessageTab(tabId, {action, params}, {frameId: ownerFrameId});
+                } catch (e) {
+                    // NOP
+                }
+            }
+        }
+    }
 }
diff --git a/ext/fg/js/frontend.js b/ext/fg/js/frontend.js
index e92feaf9..3ddf0d25 100644
--- a/ext/fg/js/frontend.js
+++ b/ext/fg/js/frontend.js
@@ -67,7 +67,9 @@ class Frontend {
         this._isPointerOverPopup = false;
 
         this._runtimeMessageHandlers = new Map([
-            ['requestFrontendReadyBroadcast',        {async: false, handler: this._onMessageRequestFrontendReadyBroadcast.bind(this)}]
+            ['requestFrontendReadyBroadcast', {async: false, handler: this._onMessageRequestFrontendReadyBroadcast.bind(this)}],
+            ['setAllVisibleOverride',         {async: true,  handler: this._onApiSetAllVisibleOverride.bind(this)}],
+            ['clearAllVisibleOverride',       {async: true,  handler: this._onApiClearAllVisibleOverride.bind(this)}]
         ]);
     }
 
@@ -117,9 +119,7 @@ class Frontend {
             ['closePopup',              {async: false, handler: this._onApiClosePopup.bind(this)}],
             ['copySelection',           {async: false, handler: this._onApiCopySelection.bind(this)}],
             ['getPopupInfo',            {async: false, handler: this._onApiGetPopupInfo.bind(this)}],
-            ['getDocumentInformation',  {async: false, handler: this._onApiGetDocumentInformation.bind(this)}],
-            ['setAllVisibleOverride',   {async: true,  handler: this._onApiSetAllVisibleOverride.bind(this)}],
-            ['clearAllVisibleOverride', {async: true,  handler: this._onApiClearAllVisibleOverride.bind(this)}]
+            ['getDocumentInformation',  {async: false, handler: this._onApiGetDocumentInformation.bind(this)}]
         ]);
 
         this._updateContentScale();
diff --git a/ext/mixed/js/api.js b/ext/mixed/js/api.js
index 63b3a3c0..701caba8 100644
--- a/ext/mixed/js/api.js
+++ b/ext/mixed/js/api.js
@@ -77,8 +77,8 @@ const api = (() => {
             return this._invoke('kanjiFind', {text, optionsContext});
         }
 
-        definitionAdd(definition, mode, context, details, optionsContext) {
-            return this._invoke('definitionAdd', {definition, mode, context, details, optionsContext});
+        definitionAdd(definition, mode, context, ownerFrameId, optionsContext) {
+            return this._invoke('definitionAdd', {definition, mode, context, ownerFrameId, optionsContext});
         }
 
         definitionsAddable(definitions, modes, context, optionsContext) {
diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js
index 2bb85f1f..70b3895a 100644
--- a/ext/mixed/js/display.js
+++ b/ext/mixed/js/display.js
@@ -1047,18 +1047,10 @@ class Display extends EventDispatcher {
         try {
             this.setSpinnerVisible(true);
 
-            const details = {};
-            if (this._noteUsesScreenshot(mode)) {
-                try {
-                    const screenshot = await this._getScreenshot();
-                    details.screenshot = screenshot;
-                } catch (e) {
-                    // NOP
-                }
-            }
-
+            const ownerFrameId = this._ownerFrameId;
+            const optionsContext = this.getOptionsContext();
             const noteContext = await this._getNoteContext();
-            const noteId = await api.definitionAdd(definition, mode, noteContext, details, this.getOptionsContext());
+            const noteId = await api.definitionAdd(definition, mode, noteContext, ownerFrameId, optionsContext);
             if (noteId) {
                 const index = this._definitions.indexOf(definition);
                 const adderButton = this._adderButtonFind(index, mode);
@@ -1136,36 +1128,6 @@ class Display extends EventDispatcher {
         }
     }
 
-    _noteUsesScreenshot(mode) {
-        const optionsAnki = this._options.anki;
-        const fields = (mode === 'kanji' ? optionsAnki.kanji : optionsAnki.terms).fields;
-        for (const fieldValue of Object.values(fields)) {
-            if (fieldValue.includes('{screenshot}')) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    async _getScreenshot() {
-        const ownerFrameId = this._ownerFrameId;
-        let token = null;
-        try {
-            if (ownerFrameId !== null) {
-                token = await api.crossFrame.invoke(ownerFrameId, 'setAllVisibleOverride', {value: false, priority: 0, awaitFrame: true});
-            }
-
-            const {format, quality} = this._options.anki.screenshot;
-            const dataUrl = await api.screenshotGet({format, quality});
-
-            return {dataUrl, format};
-        } finally {
-            if (token !== null) {
-                await api.crossFrame.invoke(ownerFrameId, 'clearAllVisibleOverride', {token});
-            }
-        }
-    }
-
     _getFirstExpressionIndex() {
         return this._options.general.resultOutputMode === 'merge' ? 0 : -1;
     }