Fix #322, focus tab on content script failure

Also do a wide-ranging refactor of how services are handled, registered, and run on the content side.

extension/background/serviceList.js

@@ -1,3 +1,5 @@
+/* globals content */
+
 this.services = {};
 
 this.serviceList = (function() {
@@ -90,7 +92,29 @@ this.serviceList = (function() {
   };
 
   exports.Service = class Service {
-    constructor() {}
+    constructor(context) {
+      this.context = context;
+      this.tab = null;
+      this.context.onError = this.onError.bind(this);
+    }
+
+    get baseUrl() {
+      return this.constructor.baseUrl;
+    }
+
+    onError(message) {
+      if (this.tab) {
+        this.activateTab();
+      }
+    }
+
+    async activateTab() {
+      if (!this.tab) {
+        throw new Error("No tab to activate");
+      }
+      await browser.tabs.update(this.tab.id, { active: true });
+    }
+
     get matchPatterns() {
       const url = new URL(this.baseUrl);
       if (url.pathname && url.pathname !== "/") {
@@ -102,11 +126,13 @@ this.serviceList = (function() {
       }
       return [`${url.protocol}//${url.hostname}/*`];
     }
+
     async activateOrOpen() {
       return this.getTab(true);
     }
+
     async getTab(activate = false) {
-      const tabs = await browser.tabs.query({ url: this.matchPatterns });
+      const tabs = await this.getAllTabs();
       if (!tabs.length) {
         return browser.tabs.create({ url: this.baseUrl, active: activate });
       }
@@ -115,6 +141,37 @@ this.serviceList = (function() {
       }
       return tabs[0];
     }
+
+    async getAllTabs() {
+      return browser.tabs.query({ url: this.matchPatterns });
+    }
+
+    async initTab(scripts) {
+      this.tab = await this.getTab();
+      await content.lazyInject(this.tab.id, scripts);
+    }
+
+    async callTab(name, args) {
+      args = args || {};
+      const response = browser.tabs.sendMessage(this.tab.id, {
+        type: name,
+        ...args,
+      });
+      if (
+        response &&
+        typeof response === "object" &&
+        response.status === "error"
+      ) {
+        const e = new Error(response.message);
+        for (const name in response) {
+          if (name !== "status" && name !== "message") {
+            e[name] = response[name];
+          }
+        }
+        throw e;
+      }
+      return response;
+    }
   };
 
   exports.detectServiceFromHistory = async function(services) {
@@ -124,6 +181,9 @@ this.serviceList = (function() {
     let bestScore = 0;
     for (const name in services) {
       const service = services[name];
+      if (!service.baseUrl) {
+        throw new Error(`Service ${service.name} has no .baseUrl`);
+      }
       const history = await browser.history.search({
         text: service.baseUrl,
         startTime: oneMonth,

extension/content/communicate.js

@@ -18,7 +18,19 @@ this.communicate = (function() {
       return HANDLERS[message.type](message, sender);
     } catch (e) {
       log.error(`Error in ${message.type} handler: ${e}`, e.stack);
-      throw e;
+      const response = {
+        status: "error",
+        message: String(e),
+        name: e.name,
+        stack: e.stack,
+        reason: e.reason,
+      };
+      for (const name in e) {
+        if (!(name in response)) {
+          response[name] = e[name];
+        }
+      }
+      return response;
     }
   };
   return exports;

extension/content/helpers.js

@@ -1,38 +1,78 @@
+/* globals communicate */
+
 this.helpers = (function() {
   const exports = {};
 
-  exports.waitForSelector = function(selector, options) {
-    const interval = (options && options.interval) || 50;
-    const timeout = (options && options.timeout) || 1000;
-    return new Promise((resolve, reject) => {
-      const start = Date.now();
-      const id = setInterval(() => {
-        const result = document.querySelector(selector);
-        if (result) {
-          clearTimeout(id);
-          resolve(result);
-          return;
-        }
-        if (Date.now() > start + timeout) {
-          const e = new Error(`Timeout waiting for ${selector}`);
-          e.name = "TimeoutError";
-          clearTimeout(id);
-          reject(e);
-        }
-      }, interval);
-    });
+  exports.Runner = class Runner {
+    constructor() {
+      this._logMessages = [];
+    }
+
+    log(...args) {
+      this._logMessages.push(args);
+    }
+
+    querySelector(selector) {
+      const element = document.querySelector(selector);
+      if (!element) {
+        const e = new Error(`Could not find element ${selector}`);
+        e.name = "ElementNotFound";
+        throw e;
+      }
+      return element;
+    }
+
+    setReactInputValue(input, value) {
+      // See https://hustle.bizongo.in/simulate-react-on-change-on-controlled-components-baa336920e04
+      // for the why of this
+      const nativeInputValueSetter = Object.getOwnPropertyDescriptor(
+        window.HTMLInputElement.prototype,
+        "value"
+      ).set;
+      nativeInputValueSetter.call(input, value);
+      const inputEvent = new Event("input", { bubbles: true });
+      input.dispatchEvent(inputEvent);
+    }
+
+    waitForSelector(selector, options) {
+      const interval = (options && options.interval) || 50;
+      const timeout = (options && options.timeout) || 1000;
+      return new Promise((resolve, reject) => {
+        const start = Date.now();
+        const id = setInterval(() => {
+          const result = document.querySelector(selector);
+          if (result) {
+            clearTimeout(id);
+            resolve(result);
+            return;
+          }
+          if (Date.now() > start + timeout) {
+            const e = new Error(`Timeout waiting for ${selector}`);
+            e.name = "TimeoutError";
+            clearTimeout(id);
+            reject(e);
+          }
+        }, interval);
+      });
+    }
   };
 
-  exports.setReactInputValue = function(input, value) {
-    // See https://hustle.bizongo.in/simulate-react-on-change-on-controlled-components-baa336920e04
-    // for the why of this
-    const nativeInputValueSetter = Object.getOwnPropertyDescriptor(
-      window.HTMLInputElement.prototype,
-      "value"
-    ).set;
-    nativeInputValueSetter.call(input, value);
-    const inputEvent = new Event("input", { bubbles: true });
-    input.dispatchEvent(inputEvent);
+  exports.Runner.register = function() {
+    const Class = this;
+    for (const name of Object.getOwnPropertyNames(Class.prototype)) {
+      if (name.startsWith("action_")) {
+        const actionName = name.substr("action_".length);
+        communicate.register(actionName, message => {
+          const instance = new Class();
+          try {
+            return instance[name](message);
+          } catch (e) {
+            e.log = instance._logMessages;
+            throw e;
+          }
+        });
+      }
+    }
   };
 
   return exports;

extension/intents/music/music.js

@@ -4,7 +4,7 @@ this.intents.music = (function() {
   const exports = {};
   const SERVICES = {};
   exports.register = function(service) {
-    SERVICES[service.name] = service;
+    SERVICES[service.id] = service;
   };
 
   intentRunner.registerIntent({
@@ -14,7 +14,8 @@ this.intents.music = (function() {
     play [query]
     `,
     async run(context) {
-      const service = await serviceList.getService("music", SERVICES);
+      const Service = await serviceList.getService("music", SERVICES);
+      const service = new Service(context);
       await service.playQuery(context.slots.query);
     },
   });
@@ -27,7 +28,8 @@ this.intents.music = (function() {
     stop music
     `,
     async run(context) {
-      const service = await serviceList.getService("music", SERVICES);
+      const Service = await serviceList.getService("music", SERVICES);
+      const service = new Service(context);
       await service.pause();
     },
   });
@@ -42,7 +44,8 @@ this.intents.music = (function() {
     `,
     priority: "high",
     async run(context) {
-      const service = await serviceList.getService("music", SERVICES);
+      const Service = await serviceList.getService("music", SERVICES);
+      const service = new Service(context);
       await service.unpause();
     },
   });
@@ -56,7 +59,8 @@ this.intents.music = (function() {
     focus music
     `,
     async run(context) {
-      const service = await serviceList.getService("music", SERVICES);
+      const Service = await serviceList.getService("music", SERVICES);
+      const service = new Service(context);
       await service.activateOrOpen();
     },
   });

extension/services/spotify/player.js

@@ -1,26 +1,30 @@
-/* globals communicate, helpers */
+/* globals helpers */
 
 this.player = (function() {
-  communicate.register("play", message => {
-    const button = document.querySelector("button[title='Play']");
-    button.click();
-  });
+  class Player extends helpers.Runner {
+    action_play() {
+      const button = document.querySelector("button[title='Play']");
+      button.click();
+    }
+
+    async action_search({ query, thenPlay }) {
+      const searchButton = this.querySelector("a[aria-label='Search']");
+      searchButton.click();
+      const input = await this.waitForSelector(".SearchInputBox input");
+      this.setReactInputValue(input, query);
+      if (thenPlay) {
+        const player = await this.waitForSelector(".tracklist-play-pause", {
+          timeout: 2000,
+        });
+        player.click();
+      }
+    }
 
-  communicate.register("search", async message => {
-    const searchButton = document.querySelector("a[aria-label='Search']");
-    searchButton.click();
-    const input = await helpers.waitForSelector(".SearchInputBox input");
-    helpers.setReactInputValue(input, message.query);
-    if (message.thenPlay) {
-      const player = await helpers.waitForSelector(".tracklist-play-pause", {
-        timeout: 2000,
-      });
-      player.click();
+    action_pause() {
+      const button = document.querySelector("button[title='Pause']");
+      button.click();
     }
-  });
+  }
 
-  communicate.register("pause", message => {
-    const button = document.querySelector("button[title='Pause']");
-    button.click();
-  });
+  Player.register();
 })();

extension/services/spotify/spotify.js

@@ -1,37 +1,28 @@
-/* globals intents, serviceList, content */
+/* globals intents, serviceList */
 
 this.services.spotify = (function() {
   class Spotify extends serviceList.Service {
     async playQuery(query) {
-      const tab = await this.getTab();
-      await content.lazyInject(tab.id, "/services/spotify/player.js");
-      await browser.tabs.sendMessage(tab.id, {
-        type: "search",
-        query,
-        thenPlay: true,
-      });
+      await this.initTab("/services/spotify/player.js");
+      await this.callTab("search", { query, thenPlay: true });
     }
 
     async pause() {
-      const tab = await this.getTab();
-      await content.lazyInject(tab.id, "/services/spotify/player.js");
-      await browser.tabs.sendMessage(tab.id, {
-        type: "pause",
-      });
+      await this.initTab("/services/spotify/player.js");
+      await this.callTab("pause");
     }
 
     async unpause() {
-      const tab = await this.getTab();
-      await content.lazyInject(tab.id, "/services/spotify/player.js");
-      await browser.tabs.sendMessage(tab.id, {
-        type: "play",
-      });
+      await this.initTab("/services/spotify/player.js");
+      await this.callTab("unpause");
     }
   }
-  Object.assign(Spotify.prototype, {
-    name: "spotify",
+
+  Object.assign(Spotify, {
+    id: "spotify",
     title: "Spotify",
     baseUrl: "https://open.spotify.com/",
   });
-  intents.music.register(new Spotify());
+
+  intents.music.register(Spotify);
 })();