Bug 1300811 - Part 3 - Update the browser action API to use TabContext r?mixedpuppy draft
authorMatthew Wein <mwein@mozilla.com>
Tue, 13 Jun 2017 23:15:06 -0400
changeset 593753 194e89c930125dcb79f9307fff6ccd345cb14009
parent 593752 9ccc5d5fc954519dbdb342420535e1c37253dc87
child 593754 2175405419993d027390a3fc43687e40aef2995c
child 593769 ae6f0c056125a5c099b22bd7159df0baeca62f04
push id63787
push usermwein@mozilla.com
push dateWed, 14 Jun 2017 03:28:10 +0000
reviewersmixedpuppy
bugs1300811
milestone56.0a1
Bug 1300811 - Part 3 - Update the browser action API to use TabContext r?mixedpuppy MozReview-Commit-ID: KSmtfI4t3Dn
mobile/android/components/extensions/ext-browserAction.js
mobile/android/components/extensions/test/mochitest/test_ext_browserAction_getTitle_setTitle.html
mobile/android/modules/BrowserActions.jsm
--- a/mobile/android/components/extensions/ext-browserAction.js
+++ b/mobile/android/components/extensions/ext-browserAction.js
@@ -10,101 +10,87 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 
 // Import the android BrowserActions module.
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserActions",
                                   "resource://gre/modules/BrowserActions.jsm");
 
 // WeakMap[Extension -> BrowserAction]
 var browserActionMap = new WeakMap();
 
-const {
-  DefaultMap,
-} = ExtensionUtils;
-
 class BrowserAction {
   constructor(options, extension) {
-    // Map[TabID -> Object]
-    this.tabIdToPropertyMap = new DefaultMap(() => ({}));
+    this.uuid = `{${extension.uuid}}`;
+
+    this.defaults = {
+      name: options.default_title || extension.name,
+    };
+
+    this.tabContext = new TabContext(tab => Object.create(this.defaults),
+                                     extension);
+
     this.tabManager = extension.tabManager;
-    this.uuid = `{${extension.uuid}}`;
-    this.name = options.default_title || extension.name;
 
-    GlobalEventDispatcher.registerListener(this, ["Tab:Selected"]);
-    GlobalEventDispatcher.registerListener(this, ["Tab:Closed"]);
+    this.tabContext.on("tab-selected", // eslint-disable-line mozilla/balanced-listeners
+                       (evt, tabId) => { this.onTabSelected(tabId); });
+    this.tabContext.on("tab-closed", // eslint-disable-line mozilla/balanced-listeners
+                       (evt, tabId) => { this.onTabClosed(tabId); });
 
     BrowserActions.register(this);
     EventEmitter.decorate(this);
   }
 
   /**
    * Retrieves the name for the active tab. Used for testing only.
    * @returns {string} the name used for the active tab.
    */
   get activeName() {
     let tab = tabTracker.activeTab;
-    return this.tabIdToPropertyMap.get(tab.id).name || this.name;
+    return this.tabContext.get(tab.id).name || this.defaults.name;
   }
 
   /**
    * Required by the BrowserActions module. This event will get
    * called whenever the browser action is clicked on.
    */
   onClicked() {
     this.emit("click", tabTracker.activeTab);
   }
 
   /**
-   * Required by the GlobalEventDispatcher module. This event will get
-   * called whenever one of the registered listeners fires.
-   * @param {string} event The event which fired.
-   * @param {object} data Information about the event which fired.
-   */
-  onEvent(event, data) {
-    switch (event) {
-      case "Tab:Selected":
-        this.onTabSelected(this.tabManager.get(data.id));
-        break;
-      case "Tab:Closed":
-        this.onTabClosed(this.tabManager.get(data.tabId));
-        break;
-    }
-  }
-
-  /**
    * Updates the browser action whenever a tab is selected.
    * @param {Object} tab The tab to update.
    */
   onTabSelected(tab) {
-    let name = this.tabIdToPropertyMap.get(tab.id).name || this.name;
+    let name = this.tabContext.get(tab.id).name || this.defaults.name;
     BrowserActions.update(this.uuid, {name});
   }
 
   /**
    * Removes the tab from the property map now that it is closed.
    * @param {Object} tab The tab which closed.
    */
   onTabClosed(tab) {
-    this.tabIdToPropertyMap.delete(tab.id);
+    this.tabContext.clear(tab.id);
   }
 
   /**
    * Sets a property for the browser action for the specified tab. If the property is set
    * for the active tab, the browser action is also updated.
    *
    * @param {Object} tab The tab to set. If null, the browser action's default value is set.
    * @param {string} prop The property to update. Currently only "name" is supported.
    * @param {string} value The value to set the property to.
    */
   setProperty(tab, prop, value) {
     if (tab == null) {
       if (value) {
-        this[prop] = value;
+        this.defaults[prop] = value;
       }
     } else {
-      let properties = this.tabIdToPropertyMap.get(tab.id);
+      let properties = this.tabContext.get(tab.id);
       if (value) {
         properties[prop] = value;
       } else {
         delete properties[prop];
       }
     }
 
     if (tab && tab.selected) {
@@ -117,26 +103,27 @@ class BrowserAction {
    *
    * @param {Object} tab The tab to retrieve the property from. If null, the default value is returned.
    * @param {string} prop The property to retreive. Currently only "name" is supported.
    * @returns {string} the value stored for the specified property. If the value is undefined, then the
    *    default value is returned.
    */
   getProperty(tab, prop) {
     if (tab == null) {
-      return this[prop];
+      return this.defaults[prop];
     }
 
-    return this.tabIdToPropertyMap.get(tab.id)[prop] || this[prop];
+    return this.tabContext.get(tab.id)[prop] || this.defaults[prop];
   }
 
   /**
    * Unregister the browser action from the BrowserActions module.
    */
   shutdown() {
+    this.tabContext.shutdown();
     BrowserActions.unregister(this.uuid);
   }
 }
 
 this.browserAction = class extends ExtensionAPI {
   onManifestEntry(entryName) {
     let {extension} = this;
     let {manifest} = extension;
--- a/mobile/android/components/extensions/test/mochitest/test_ext_browserAction_getTitle_setTitle.html
+++ b/mobile/android/components/extensions/test/mochitest/test_ext_browserAction_getTitle_setTitle.html
@@ -9,76 +9,77 @@
   <link rel="stylesheet" href="chrome://mochikit/contents/tests/SimpleTest/test.css"/>
 </head>
 <body>
 
 <script type="text/javascript">
 "use strict";
 
 var {BrowserActions} = SpecialPowers.Cu.import("resource://gre/modules/BrowserActions.jsm", {});
+var {ContentTaskUtils} = SpecialPowers.Cu.import("resource://testing-common/ContentTaskUtils.jsm", {});
 
 add_task(async function test_setTitle_and_getTitle() {
   async function background() {
     let tabCreatedPromise = new Promise(resolve => {
       let onTabCreated = tab => {
         browser.tabs.onCreated.removeListener(onTabCreated);
         resolve();
       };
       browser.tabs.onCreated.addListener(onTabCreated);
     });
 
     async function createAndTestNewTab(title, url) {
       // First make sure the default title is correct.
       let defaultTitle = await browser.browserAction.getTitle({});
-      browser.test.assertEq("Browser Action", defaultTitle, `Expected the default title to be ${defaultTitle}`);
+      browser.test.assertEq("Browser Action", defaultTitle, "Expected the default title to be returned");
 
       // Create a tab.
       let [tab] = await Promise.all([
         browser.tabs.create({url}),
         tabCreatedPromise,
       ]);
 
       // Test that the default title is returned before the title is set for the tab.
       let tabTitle = await browser.browserAction.getTitle({tabId: tab.id});
       browser.test.assertEq("Browser Action", tabTitle, "Expected the default title to be returned");
 
       // Set the title for the new tab and test that getTitle returns the correct title.
       await browser.browserAction.setTitle({tabId: tab.id, title});
       tabTitle = await browser.browserAction.getTitle({tabId: tab.id});
-      browser.test.assertEq(title, tabTitle, `Expected the new tab title to be ${title}`);
+      browser.test.assertEq(title, tabTitle, "Expected the new tab title to be returned");
 
       return tab;
     }
 
     // Create and test 3 new tabs.
     let tab1 = await createAndTestNewTab("tab 1", "about:blank");
     let tab2 = await createAndTestNewTab("tab 2", "about:blank");
     let tab3 = await createAndTestNewTab("tab 3", "about:blank");
 
     // Test the default title again.
     let title = await browser.browserAction.getTitle({});
-    browser.test.assertEq("Browser Action", title, `Expected the default title to be "Browser Action"`);
+    browser.test.assertEq("Browser Action", title, "Expected the default title to be returned");
 
     // Update the default title and confirm that the new title is returned.
     await browser.browserAction.setTitle({title: "Updated Title"});
     title = await browser.browserAction.getTitle({});
-    browser.test.assertEq("Updated Title", title, `Expected the default title to be "Updated Title"`);
+    browser.test.assertEq("Updated Title", title, "Expected the default title to be updated");
 
     // Try setting the default title to an empty string and confirm that the original title is still used.
     browser.browserAction.setTitle({title: ""});
     title = await browser.browserAction.getTitle({});
-    browser.test.assertEq("Updated Title", title, `Expected the default title to still be "Updated Title"`);
+    browser.test.assertEq("Updated Title", title, "Expected the default title to be returned");
 
     // Check all of the created tabs now.
     title = await browser.browserAction.getTitle({tabId: tab1.id});
-    browser.test.assertEq("tab 1", title, `Expected the first tab title to be ${title}`);
+    browser.test.assertEq("tab 1", title, "Expected the first tab title");
     title = await browser.browserAction.getTitle({tabId: tab2.id});
-    browser.test.assertEq("tab 2", title, `Expected the second tab title to be ${title}`);
+    browser.test.assertEq("tab 2", title, "Expected the second tab title");
     title = await browser.browserAction.getTitle({tabId: tab3.id});
-    browser.test.assertEq("tab 3", title, `Expected the third tab title to be ${title}`);
+    browser.test.assertEq("tab 3", title, "Expected the third tab title");
 
     // Unset the title for the first tab and confirm that it is unset.
     browser.browserAction.setTitle({tabId: tab1.id, title: ""});
     title = await browser.browserAction.getTitle({tabId: tab1.id});
     browser.test.assertEq("Updated Title", title, `Expected the default title to be returned`);
 
     browser.test.onMessage.addListener(async (msg, data) => {
       if (msg === "select-tab") {
@@ -106,17 +107,20 @@ add_task(async function test_setTitle_an
 
   await extension.startup();
 
   let tabs = await extension.awaitMessage("tabs");
 
   async function checkTab(tab, name) {
     extension.sendMessage("select-tab", tab.id);
     await extension.awaitMessage("tab-selected");
-    is(BrowserActions.getNameForActiveTab(`{${extension.uuid}}`), name, "Got the expected name for the active browser action");
+    // Wait until the browser action has updated to the correct title.
+    await ContentTaskUtils.waitForCondition(() => {
+      return BrowserActions.getNameForActiveTab(`{${extension.uuid}}`) === name;
+    });
   }
 
   await checkTab(tabs.tab1, "Updated Title");
   await checkTab(tabs.tab2, "tab 2");
   await checkTab(tabs.tab3, "tab 3");
   await checkTab(tabs.tab1, "Updated Title");
 
   extension.sendMessage("finish");
--- a/mobile/android/modules/BrowserActions.jsm
+++ b/mobile/android/modules/BrowserActions.jsm
@@ -64,17 +64,17 @@ var BrowserActions = {
    * Registers a new browser action.
    * @param {Object} browserAction The browser action to add.
    */
   register(browserAction) {
     EventDispatcher.instance.sendRequest({
       type: "Menu:AddBrowserAction",
       id: this._nextMenuId++,
       uuid: browserAction.uuid,
-      name: browserAction.name,
+      name: browserAction.defaults.name,
     });
 
     this._browserActions[browserAction.uuid] = browserAction;
 
     this._maybeRegisterListeners();
   },
 
   /**