Bug 1267278 - remove the usage of some add-on SDK modules, and refactoring the unit tests; r=jryans draft
authorMatteo Ferretti <mferretti@mozilla.com>
Thu, 19 May 2016 12:04:51 +0200
changeset 374440 7d01e392d69123abc1d9589fb2297b306aa8e386
parent 374439 e74192b1a9c82ebcb431e5b4c28775a8111c8864
child 522631 aef8cca61a62d252ec380577abeef086a485bb31
push id20018
push userbmo:zer0@mozilla.com
push dateThu, 02 Jun 2016 11:49:47 +0000
reviewersjryans
bugs1267278
milestone49.0a1
Bug 1267278 - remove the usage of some add-on SDK modules, and refactoring the unit tests; r=jryans - removed responsive.html/events.js - added `isActiveForWindow` method to `ResponsiveUIManager` - simplified how the check for RDM menu is done - re-enabled tests for OS X debug in e10s - refactored tests using `BrowserTestUtils` MozReview-Commit-ID: 1TADh1dRVcU
devtools/client/responsive.html/events.js
devtools/client/responsive.html/manager.js
devtools/client/responsive.html/moz.build
devtools/client/responsive.html/test/browser/browser.ini
devtools/client/responsive.html/test/browser/browser_menu_item_01.js
devtools/client/responsive.html/test/browser/browser_menu_item_02.js
devtools/client/responsive.html/test/browser/head.js
deleted file mode 100644
--- a/devtools/client/responsive.html/events.js
+++ /dev/null
@@ -1,38 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-const { filter, map, merge } = require("sdk/event/utils");
-const { events: browserEvents } = require("sdk/browser/events");
-const { events: tabEvents } = require("sdk/tab/events");
-const { getTabs, getActiveTab, getOwnerWindow } = require("sdk/tabs/utils");
-
-const tabSelect = filter(tabEvents, e => e.type === "TabSelect");
-const tabClose = filter(tabEvents, e => e.type === "TabClose");
-const windowOpen = filter(browserEvents, e => e.type === "load");
-const windowClose = filter(browserEvents, e => e.type === "close");
-
-// The `activate` event stream, observes when any tab is activated.
-// It has the activated `tab` as argument.
-const activate = merge([
-  map(tabSelect, ({target}) => target),
-  map(windowOpen, ({target}) => getActiveTab(target))
-]);
-exports.activate = activate;
-
-// The `close` event stream, observes when any tab or any window is closed.
-// The event has an object as argument, with the `tabs` involved in the closing
-// process and their owner window.
-const close = merge([
-  map(tabClose, ({target}) => ({
-    window: getOwnerWindow(target),
-    tabs: [target]
-  })),
-  map(windowClose, ({target}) => ({
-    window: target,
-    tabs: getTabs(target)
-  }))
-]);
-exports.close = close;
--- a/devtools/client/responsive.html/manager.js
+++ b/devtools/client/responsive.html/manager.js
@@ -4,19 +4,17 @@
 
 "use strict";
 
 const promise = require("promise");
 const { Task } = require("devtools/shared/task");
 const EventEmitter = require("devtools/shared/event-emitter");
 const { TouchEventSimulator } = require("devtools/shared/touch/simulator");
 const { getOwnerWindow } = require("sdk/tabs/utils");
-const { on, off } = require("sdk/event/core");
 const { startup } = require("sdk/window/helpers");
-const events = require("./events");
 const message = require("./utils/message");
 const { swapToInnerBrowser } = require("./browser/swap");
 
 const TOOL_URL = "chrome://devtools/content/responsive.html/index.xhtml";
 
 /**
  * ResponsiveUIManager is the external API for the browser UI, etc. to use when
  * opening and closing the responsive UI.
@@ -58,26 +56,25 @@ const ResponsiveUIManager = exports.Resp
    *         Resolved to the ResponsiveUI instance for this tab when opening is
    *         complete.
    */
   openIfNeeded: Task.async(function* (window, tab) {
     if (!tab.linkedBrowser.isRemoteBrowser) {
       return promise.reject(new Error("RDM only available for remote tabs."));
     }
     if (!this.isActiveForTab(tab)) {
-      if (!this.activeTabs.size) {
-        on(events.activate, "data", onActivate);
-        on(events.close, "data", onClose);
-      }
+      this.initMenuCheckListenerFor(window);
+
       let ui = new ResponsiveUI(window, tab);
       this.activeTabs.set(tab, ui);
-      yield setMenuCheckFor(tab, window);
+      yield this.setMenuCheckFor(tab, window);
       yield ui.inited;
       this.emit("on", { tab });
     }
+
     return this.getResponsiveUIForTab(tab);
   }),
 
   /**
    * Closes the responsive UI, if not already closed.
    *
    * @param window
    *        The main browser chrome window.
@@ -92,37 +89,48 @@ const ResponsiveUIManager = exports.Resp
     if (this.isActiveForTab(tab)) {
       let ui = this.activeTabs.get(tab);
       let destroyed = yield ui.destroy(options);
       if (!destroyed) {
         // Already in the process of destroying, abort.
         return;
       }
       this.activeTabs.delete(tab);
-      if (!this.activeTabs.size) {
-        off(events.activate, "data", onActivate);
-        off(events.close, "data", onClose);
+
+      if (!this.isActiveForWindow(window)) {
+        this.removeMenuCheckListenerFor(window);
       }
       this.emit("off", { tab });
-      yield setMenuCheckFor(tab, window);
+      yield this.setMenuCheckFor(tab, window);
     }
   }),
 
   /**
    * Returns true if responsive UI is active for a given tab.
    *
    * @param tab
    *        The browser tab.
    * @return boolean
    */
   isActiveForTab(tab) {
     return this.activeTabs.has(tab);
   },
 
   /**
+   * Returns true if responsive UI is active in any tab in the given window.
+   *
+   * @param window
+   *        The main browser chrome window.
+   * @return boolean
+   */
+  isActiveForWindow(window) {
+    return [...this.activeTabs.keys()].some(t => getOwnerWindow(t) === window);
+  },
+
+  /**
    * Return the responsive UI controller for a tab.
    *
    * @param tab
    *        The browser tab.
    * @return ResponsiveUI
    *         The UI instance for this tab.
    */
   getResponsiveUIForTab(tab) {
@@ -136,17 +144,17 @@ const ResponsiveUIManager = exports.Resp
    *        The main browser chrome window.
    * @param tab
    *        The browser tab.
    * @param command
    *        The GCLI command name.
    * @param args
    *        The GCLI command arguments.
    */
-  handleGcliCommand: function (window, tab, command, args) {
+  handleGcliCommand(window, tab, command, args) {
     let completed;
     switch (command) {
       case "resize to":
         completed = this.openIfNeeded(window, tab);
         this.activeTabs.get(tab).setViewportSize(args.width, args.height);
         break;
       case "resize on":
         completed = this.openIfNeeded(window, tab);
@@ -155,17 +163,42 @@ const ResponsiveUIManager = exports.Resp
         completed = this.closeIfNeeded(window, tab);
         break;
       case "resize toggle":
         completed = this.toggle(window, tab);
         break;
       default:
     }
     completed.catch(e => console.error(e));
-  }
+  },
+
+  handleMenuCheck({target}) {
+    ResponsiveUIManager.setMenuCheckFor(target);
+  },
+
+  initMenuCheckListenerFor(window) {
+    let { tabContainer } = window.gBrowser;
+    tabContainer.addEventListener("TabSelect", this.handleMenuCheck);
+  },
+
+  removeMenuCheckListenerFor(window) {
+    if (window && window.gBrowser && window.gBrowser.tabContainer) {
+      let { tabContainer } = window.gBrowser;
+      tabContainer.removeEventListener("TabSelect", this.handleMenuCheck);
+    }
+  },
+
+  setMenuCheckFor: Task.async(function* (tab, window = getOwnerWindow(tab)) {
+    yield startup(window);
+
+    let menu = window.document.getElementById("menu_responsiveUI");
+    if (menu) {
+      menu.setAttribute("checked", this.isActiveForTab(tab));
+    }
+  })
 };
 
 // GCLI commands in ../responsivedesign/resize-commands.js listen for events
 // from this object to know when the UI for a tab has opened or closed.
 EventEmitter.decorate(ResponsiveUIManager);
 
 /**
  * ResponsiveUI manages the responsive design tool for a specific tab.  The
@@ -393,27 +426,8 @@ ResponsiveUI.prototype = {
    */
   getViewportBrowser() {
     return this.toolWindow.getViewportBrowser();
   },
 
 };
 
 EventEmitter.decorate(ResponsiveUI.prototype);
-
-const onActivate = (tab) => setMenuCheckFor(tab);
-
-const onClose = ({ window, tabs }) => {
-  for (let tab of tabs) {
-    ResponsiveUIManager.closeIfNeeded(window, tab);
-  }
-};
-
-const setMenuCheckFor = Task.async(
-  function* (tab, window = getOwnerWindow(tab)) {
-    yield startup(window);
-
-    let menu = window.document.getElementById("menu_responsiveUI");
-    if (menu) {
-      menu.setAttribute("checked", ResponsiveUIManager.isActiveForTab(tab));
-    }
-  }
-);
--- a/devtools/client/responsive.html/moz.build
+++ b/devtools/client/responsive.html/moz.build
@@ -13,17 +13,16 @@ DIRS += [
     'reducers',
     'utils',
 ]
 
 DevToolsModules(
     'app.js',
     'constants.js',
     'devices.js',
-    'events.js',
     'index.css',
     'manager.js',
     'reducers.js',
     'responsive-ua.css',
     'store.js',
     'types.js',
 )
 
--- a/devtools/client/responsive.html/test/browser/browser.ini
+++ b/devtools/client/responsive.html/test/browser/browser.ini
@@ -1,30 +1,28 @@
 [DEFAULT]
 tags = devtools
 subsuite = devtools
 # !e10s: RDM only works for remote tabs
-# OSX debug e10s: Bug 1268319 - Leaking the world
-skip-if = !e10s || (os == 'mac' && e10s && debug)
+skip-if = !e10s
 support-files =
   devices.json
   doc_page_state.html
   head.js
   !/devtools/client/commandline/test/helpers.js
   !/devtools/client/framework/test/shared-head.js
   !/devtools/client/framework/test/shared-redux-head.js
 
 [browser_device_modal_exit.js]
 [browser_device_modal_submit.js]
 [browser_device_width.js]
 [browser_exit_button.js]
 [browser_frame_script_active.js]
 [browser_menu_item_01.js]
 [browser_menu_item_02.js]
-skip-if = (e10s && debug) # Bug 1267278: browser.xul leaks
 [browser_mouse_resize.js]
 [browser_page_state.js]
 [browser_resize_cmd.js]
 skip-if = true # GCLI target confused after swap, will fix in bug 1240907
 [browser_screenshot_button.js]
 [browser_shutdown_close_sync.js]
 [browser_touch_simulation.js]
 [browser_viewport_basics.js]
--- a/devtools/client/responsive.html/test/browser/browser_menu_item_01.js
+++ b/devtools/client/responsive.html/test/browser/browser_menu_item_01.js
@@ -4,21 +4,25 @@
 "use strict";
 
 // Test RDM menu item is checked when expected, on multiple tabs.
 
 const TEST_URL = "data:text/html;charset=utf-8,";
 
 const tabUtils = require("sdk/tabs/utils");
 const { startup } = require("sdk/window/helpers");
-const { activate } = require("devtools/client/responsive.html/events");
-const events = require("sdk/event/core");
 
 const activateTab = (tab) => new Promise(resolve => {
-  events.once(activate, "data", resolve);
+  let { tabContainer } = tabUtils.getOwnerWindow(tab).gBrowser;
+
+  tabContainer.addEventListener("TabSelect", function listener({type}) {
+    tabContainer.removeEventListener(type, listener);
+    resolve();
+  });
+
   tabUtils.activateTab(tab);
 });
 
 const isMenuChecked = () => {
   let menu = document.getElementById("menu_responsiveUI");
   return menu.getAttribute("checked") === "true";
 };
 
--- a/devtools/client/responsive.html/test/browser/browser_menu_item_02.js
+++ b/devtools/client/responsive.html/test/browser/browser_menu_item_02.js
@@ -2,48 +2,48 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Test RDM menu item is checked when expected, on multiple windows.
 
 const TEST_URL = "data:text/html;charset=utf-8,";
 
-const { getActiveTab } = require("sdk/tabs/utils");
 const { getMostRecentBrowserWindow } = require("sdk/window/utils");
-const { open, close, startup } = require("sdk/window/helpers");
-const { partial } = require("sdk/lang/functional");
-
-const openBrowserWindow = partial(open, null, { features: { toolbar: true } });
 
 const isMenuCheckedFor = ({document}) => {
   let menu = document.getElementById("menu_responsiveUI");
   return menu.getAttribute("checked") === "true";
 };
 
 add_task(function* () {
-  const window1 = yield openBrowserWindow();
-
-  yield startup(window1);
+  const window1 = yield BrowserTestUtils.openNewBrowserWindow();
+  let { gBrowser } = window1;
 
-  yield BrowserTestUtils.openNewForegroundTab(window1.gBrowser, TEST_URL);
+  yield BrowserTestUtils.withNewTab({ gBrowser, url: TEST_URL },
+    function* (browser) {
+      let tab = gBrowser.getTabForBrowser(browser);
 
-  const tab1 = getActiveTab(window1);
+      is(window1, getMostRecentBrowserWindow(),
+        "The new window is the active one");
+
+      ok(!isMenuCheckedFor(window1),
+        "RDM menu item is unchecked by default");
 
-  is(window1, getMostRecentBrowserWindow(),
-    "The new window is the active one");
+      yield openRDM(tab);
 
-  ok(!isMenuCheckedFor(window1),
-    "RDM menu item is unchecked by default");
+      ok(isMenuCheckedFor(window1),
+        "RDM menu item is checked with RDM open");
 
-  yield openRDM(tab1);
+      yield closeRDM(tab);
 
-  ok(isMenuCheckedFor(window1),
-    "RDM menu item is checked with RDM open");
+      ok(!isMenuCheckedFor(window1),
+        "RDM menu item is unchecked with RDM closed");
+    });
 
-  yield close(window1);
+  yield BrowserTestUtils.closeWindow(window1);
 
   is(window, getMostRecentBrowserWindow(),
     "The original window is the active one");
 
   ok(!isMenuCheckedFor(window),
     "RDM menu item is unchecked");
 });
--- a/devtools/client/responsive.html/test/browser/head.js
+++ b/devtools/client/responsive.html/test/browser/head.js
@@ -55,17 +55,17 @@ var openRDM = Task.async(function* (tab)
 });
 
 /**
  * Close responsive design mode for the given tab.
  */
 var closeRDM = Task.async(function* (tab, options) {
   info("Closing responsive design mode");
   let manager = ResponsiveUIManager;
-  yield manager.closeIfNeeded(window, tab, options);
+  yield manager.closeIfNeeded(getOwnerWindow(tab), tab, options);
   info("Responsive design mode closed");
 });
 
 /**
  * Adds a new test task that adds a tab with the given URL, opens responsive
  * design mode, runs the given generator, closes responsive design mode, and
  * removes the tab.
  *