Bug 1250084 - The 'Responsive Design View' button from the Menu is not checked while the RDM is opened; r=jryans
authorMatteo Ferretti <mferretti@mozilla.com>
Sun, 24 Apr 2016 14:54:08 +0200
changeset 294740 352d365eb4eda3aab7c3680b0e80ce17e178b5ee
parent 294739 0f07f975526f3abda2f997bbb2feb0a25f771227
child 294741 0a5331a770552e879e9a5b3e0ab82d82b1e709dc
push id30210
push userkwierso@gmail.com
push dateMon, 25 Apr 2016 22:25:12 +0000
treeherdermozilla-central@79de998e7307 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1250084
milestone48.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 1250084 - The 'Responsive Design View' button from the Menu is not checked while the RDM is opened; r=jryans MozReview-Commit-ID: 9i9rYI48H4G
addon-sdk/source/lib/sdk/window/helpers.js
addon-sdk/source/lib/sdk/window/utils.js
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
--- a/addon-sdk/source/lib/sdk/window/helpers.js
+++ b/addon-sdk/source/lib/sdk/window/helpers.js
@@ -1,17 +1,17 @@
 /* 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 { defer, all } = require('../core/promise');
 const events = require('../system/events');
 const { open: openWindow, onFocus, getToplevelWindow,
-        isInteractive, getOuterId } = require('./utils');
+        isInteractive, isStartupFinished, getOuterId } = require('./utils');
 const { Ci } = require("chrome");
 
 function open(uri, options) {
   return promise(openWindow.apply(null, arguments), 'load').then(focus);
 }
 exports.open = open;
 
 function close(window) {
@@ -44,16 +44,34 @@ function ready(window) {
     resolve(window);
   else
     resolve(promise(window, 'DOMContentLoaded'));
 
   return result;
 }
 exports.ready = ready;
 
+function startup(window) {
+  let { promise: result, resolve } = defer();
+
+  if (isStartupFinished(window)) {
+    resolve(window);
+  } else {
+    events.on("browser-delayed-startup-finished", function listener({subject}) {
+      if (subject === window) {
+        events.off("browser-delayed-startup-finished", listener);
+        resolve(window);
+      }
+    });
+  }
+
+  return result;
+}
+exports.startup = startup;
+
 function promise(target, evt, capture) {
   let deferred = defer();
   capture = !!capture;
 
   target.addEventListener(evt, function eventHandler() {
     target.removeEventListener(evt, eventHandler, capture);
     deferred.resolve(target);
   }, capture);
--- a/addon-sdk/source/lib/sdk/window/utils.js
+++ b/addon-sdk/source/lib/sdk/window/utils.js
@@ -317,16 +317,27 @@ exports.windows = windows;
 const isInteractive = window =>
   window.document.readyState === "interactive" ||
   isDocumentLoaded(window) ||
   // XUL documents stays '"uninitialized"' until it's `readyState` becomes
   // `"complete"`.
   isXULDocumentWindow(window) && window.document.readyState === "interactive";
 exports.isInteractive = isInteractive;
 
+/**
+ * Check if the given browser window has finished the startup.
+ * @params {nsIDOMWindow} window
+ */
+const isStartupFinished = (window) =>
+  isBrowser(window) &&
+  window.gBrowserInit &&
+  window.gBrowserInit.delayedStartupFinished;
+
+exports.isStartupFinished = isStartupFinished;
+
 const isXULDocumentWindow = ({document}) =>
   document.documentElement &&
   document.documentElement.namespaceURI === XUL_NS;
 
 /**
  * Check if the given window is completely loaded.
  * i.e. if its "load" event has already been fired and all possible DOM content
  * is done loading (the whole DOM document, images content, ...)
new file mode 100644
--- /dev/null
+++ b/devtools/client/responsive.html/events.js
@@ -0,0 +1,38 @@
+/* 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,16 +4,20 @@
 
 "use strict";
 
 const { Ci, Cr } = require("chrome");
 const promise = require("promise");
 const { Task } = require("resource://gre/modules/Task.jsm");
 const { XPCOMUtils } = require("resource://gre/modules/XPCOMUtils.jsm");
 const EventEmitter = require("devtools/shared/event-emitter");
+const { getOwnerWindow } = require("sdk/tabs/utils");
+const { on, off } = require("sdk/event/core");
+const { startup } = require("sdk/window/helpers");
+const events = require("./events");
 
 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.
  *
  * While the HTML UI is in an experimental stage, the older ResponsiveUIManager
@@ -48,18 +52,23 @@ const ResponsiveUIManager = exports.Resp
    * @param tab
    *        The browser tab.
    * @return Promise
    *         Resolved to the ResponsiveUI instance for this tab when opening is
    *         complete.
    */
   openIfNeeded: Task.async(function* (window, tab) {
     if (!this.isActiveForTab(tab)) {
+      if (!this.activeTabs.size) {
+        on(events.activate, "data", onActivate);
+        on(events.close, "data", onClose);
+      }
       let ui = new ResponsiveUI(window, tab);
       this.activeTabs.set(tab, ui);
+      yield setMenuCheckFor(tab, window);
       yield ui.inited;
       this.emit("on", { tab });
     }
     return this.getResponsiveUIForTab(tab);
   }),
 
   /**
    * Closes the responsive UI, if not already closed.
@@ -68,19 +77,28 @@ const ResponsiveUIManager = exports.Resp
    *        The main browser chrome window.
    * @param tab
    *        The browser tab.
    * @return Promise
    *         Resolved (with no value) when closing is complete.
    */
   closeIfNeeded: Task.async(function* (window, tab) {
     if (this.isActiveForTab(tab)) {
-      yield this.activeTabs.get(tab).destroy();
+      let ui = this.activeTabs.get(tab);
       this.activeTabs.delete(tab);
+
+      if (!this.activeTabs.size) {
+        off(events.activate, "data", onActivate);
+        off(events.close, "data", onClose);
+      }
+
+      yield ui.destroy();
       this.emit("off", { tab });
+
+      yield setMenuCheckFor(tab, window);
     }
     return promise.resolve();
   }),
 
   /**
    * Returns true if responsive UI is active for a given tab.
    *
    * @param tab
@@ -135,16 +153,17 @@ const ResponsiveUIManager = exports.Resp
     }
     completed.catch(e => console.error(e));
   }
 };
 
 // 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
  * actual tool itself lives in a separate chrome:// document that is loaded into
  * the tab upon opening responsive design.  This object acts a helper to
  * integrate the tool into the surrounding browser UI as needed.
  */
 function ResponsiveUI(window, tab) {
   this.browserWindow = window;
@@ -304,8 +323,25 @@ function waitForDocLoadComplete(gBrowser
       }
     },
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIWebProgressListener,
                                            Ci.nsISupportsWeakReference])
   };
   gBrowser.addProgressListener(progressListener);
   return deferred.promise;
 }
+
+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");
+    menu.setAttribute("checked", ResponsiveUIManager.isActiveForTab(tab));
+  }
+);
--- a/devtools/client/responsive.html/moz.build
+++ b/devtools/client/responsive.html/moz.build
@@ -12,16 +12,17 @@ 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
@@ -8,12 +8,15 @@ support-files =
   !/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_menu_item_01.js]
+[browser_menu_item_02.js]
+skip-if = os == "win" && debug # Bug 1267278 - RDM's browser_menu_item_02 test is failing on Windows Debug
 [browser_mouse_resize.js]
 [browser_resize_cmd.js]
 [browser_screenshot_button.js]
 [browser_viewport_basics.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/responsive.html/test/browser/browser_menu_item_01.js
@@ -0,0 +1,58 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"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);
+  tabUtils.activateTab(tab);
+});
+
+const isMenuChecked = () => {
+  let menu = document.getElementById("menu_responsiveUI");
+  return menu.getAttribute("checked") === "true";
+};
+
+add_task(function* () {
+  yield startup(window);
+
+  ok(!isMenuChecked(),
+    "RDM menu item is unchecked by default");
+
+  const tab = yield addTab(TEST_URL);
+
+  ok(!isMenuChecked(),
+    "RDM menu item is unchecked for new tab");
+
+  yield openRDM(tab);
+
+  ok(isMenuChecked(),
+    "RDM menu item is checked with RDM open");
+
+  const tab2 = yield addTab(TEST_URL);
+
+  ok(!isMenuChecked(),
+    "RDM menu item is unchecked for new tab");
+
+  yield activateTab(tab);
+
+  ok(isMenuChecked(),
+    "RDM menu item is checked for the tab where RDM is open");
+
+  yield closeRDM(tab);
+
+  ok(!isMenuChecked(),
+    "RDM menu item is unchecked after RDM is closed");
+
+  yield removeTab(tab);
+  yield removeTab(tab2);
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/responsive.html/test/browser/browser_menu_item_02.js
@@ -0,0 +1,47 @@
+/* Any copyright is dedicated to the Public Domain.
+   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(TEST_URL);
+
+  yield startup(window1);
+
+  const tab1 = getActiveTab(window1);
+
+  is(window1, getMostRecentBrowserWindow(),
+    "The new window is the active one");
+
+  ok(!isMenuCheckedFor(window1),
+    "RDM menu item is unchecked by default");
+
+  yield openRDM(tab1);
+
+  ok(isMenuCheckedFor(window1),
+    "RDM menu item is checked with RDM open");
+
+  yield close(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
@@ -32,28 +32,29 @@ Services.prefs.setCharPref("devtools.dev
 Services.prefs.setBoolPref("devtools.responsive.html.enabled", true);
 
 registerCleanupFunction(() => {
   DevToolsUtils.testing = false;
   Services.prefs.clearUserPref("devtools.devices.url");
   Services.prefs.clearUserPref("devtools.responsive.html.enabled");
   Services.prefs.clearUserPref("devtools.responsive.html.displayedDeviceList");
 });
-const { ResponsiveUIManager } = Cu.import("resource://devtools/client/responsivedesign/responsivedesign.jsm", {});
+const { ResponsiveUIManager } = require("resource://devtools/client/responsivedesign/responsivedesign.jsm");
 const { loadDeviceList } = require("devtools/client/responsive.html/devices");
+const { getOwnerWindow } = require("sdk/tabs/utils");
 
 const OPEN_DEVICE_MODAL_VALUE = "OPEN_DEVICE_MODAL";
 
 /**
  * Open responsive design mode for the given tab.
  */
 var openRDM = Task.async(function* (tab) {
   info("Opening responsive design mode");
   let manager = ResponsiveUIManager;
-  let ui = yield manager.openIfNeeded(window, tab);
+  let ui = yield manager.openIfNeeded(getOwnerWindow(tab), tab);
   info("Responsive design mode opened");
   return { ui, manager };
 });
 
 /**
  * Close responsive design mode for the given tab.
  */
 var closeRDM = Task.async(function* (tab) {