Bug 1300811 - Part 3 - Update the browser action API to use TabContext r=mixedpuppy
authorMatthew Wein <mwein@mozilla.com>
Tue, 13 Jun 2017 23:15:06 -0400
changeset 363972 b8834309f85bd3e70acf5a9a0260db04904e6bdc
parent 363971 ab64e2df938f1ba3e4e43e9bb5d3d38c3a6c9ea6
child 363973 76b7cff0e3b2779451535b38086f82034c7c176c
push id32029
push userarchaeopteryx@coole-files.de
push dateThu, 15 Jun 2017 08:54:25 +0000
treeherdermozilla-central@dced94de288a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1300811
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();
   },
 
   /**