Bug 1267278 - remove the usage of some add-on SDK modules, and refactoring the unit tests; r=jryans
authorMatteo Ferretti <mferretti@mozilla.com>
Thu, 19 May 2016 12:04:51 +0200
changeset 341112 c2f28446c94a222c145c436d6d9e78ec68b7e17a
parent 341111 fe57228e70aa503323bf177093e2cecb438a39cc
child 341113 91879ebba4d61fde72af13223708faa01109a4d9
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1267278
milestone49.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 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.
  *