Bug 1243131 - memory tool: select snapshot using ACCEL+{UP/DOWN};r=fitzgen
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 12 Feb 2016 11:03:34 +0100
changeset 284499 8618a85c5801ff76fd645d3d02fb47c14bb7d36d
parent 284498 6702685a691946eedbfdecc414c1d17f5768c57e
child 284500 829d5a1b43cc3b774d3606a66c01455906172b65
push id71993
push usercbook@mozilla.com
push dateWed, 17 Feb 2016 11:16:29 +0000
treeherdermozilla-inbound@60f020c84b23 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1243131
milestone47.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 1243131 - memory tool: select snapshot using ACCEL+{UP/DOWN};r=fitzgen Adds a keydown listener on the memory panel window. Select previous/next snapshot when user presses UP/DOWN with the accelKey modifier (metaKey on OSX, ctrlKey on windows). Keydown events with modifiers are no longer listened to by the tree node elements. Updated tree node test. Added new mochitest to test the new keyboard navigation on the census view. )
devtools/client/memory/app.js
devtools/client/memory/test/browser/browser.ini
devtools/client/memory/test/browser/browser_memory_keyboard-snapshot-list.js
devtools/client/memory/test/browser/head.js
devtools/client/shared/components/test/mochitest/test_tree_06.html
devtools/client/shared/components/tree.js
--- a/devtools/client/memory/app.js
+++ b/devtools/client/memory/app.js
@@ -1,13 +1,14 @@
 /* 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/. */
 
 const { assert } = require("devtools/shared/DevToolsUtils");
+const { appinfo } = require("Services");
 const { DOM: dom, createClass, createFactory, PropTypes } = require("devtools/client/shared/vendor/react");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
 const { breakdowns, diffingState, viewState } = require("./constants");
 const { toggleRecordingAllocationStacks } = require("./actions/allocations");
 const { setBreakdownAndRefresh } = require("./actions/breakdown");
 const { setDominatorTreeBreakdownAndRefresh } = require("./actions/dominatorTreeBreakdown");
 const {
   selectSnapshotForDiffingAndRefresh,
@@ -48,30 +49,62 @@ const MemoryApp = createClass({
   displayName: "MemoryApp",
 
   propTypes: appModel,
 
   getDefaultProps() {
     return {};
   },
 
+  componentDidMount() {
+    // Attach the keydown listener directly to the window. When an element that
+    // has the focus (such as a tree node) is removed from the DOM, the focus
+    // falls back to the body.
+    window.addEventListener("keydown", this.onKeyDown);
+  },
+
+  componentWillUnmount() {
+    window.removeEventListener("keydown", this.onKeyDown);
+  },
+
   childContextTypes: {
     front: PropTypes.any,
     heapWorker: PropTypes.any,
     toolbox: PropTypes.any,
   },
 
   getChildContext() {
     return {
       front: this.props.front,
       heapWorker: this.props.heapWorker,
       toolbox: this.props.toolbox,
     };
   },
 
+  onKeyDown(e) {
+    let { snapshots, dispatch, heapWorker } = this.props;
+    const selectedSnapshot = snapshots.find(s => s.selected);
+    const selectedIndex = snapshots.indexOf(selectedSnapshot);
+
+    let isOSX = appinfo.OS == "Darwin";
+    let isAccelKey = (isOSX && e.metaKey) || (!isOSX && e.ctrlKey);
+    // On ACCEL+UP, select previous snapshot.
+    if (isAccelKey && e.key === "ArrowUp") {
+      let previousIndex = Math.max(0, selectedIndex - 1);
+      let previousSnapshotId = snapshots[previousIndex].id;
+      dispatch(selectSnapshotAndRefresh(heapWorker, previousSnapshotId));
+    }
+    // On ACCEL+DOWN, select next snapshot.
+    if (isAccelKey && e.key === "ArrowDown") {
+      let nextIndex = Math.min(snapshots.length - 1, selectedIndex + 1);
+      let nextSnapshotId = snapshots[nextIndex].id;
+      dispatch(selectSnapshotAndRefresh(heapWorker, nextSnapshotId));
+    }
+  },
+
   render() {
     let {
       dispatch,
       snapshots,
       front,
       heapWorker,
       breakdown,
       allocations,
--- a/devtools/client/memory/test/browser/browser.ini
+++ b/devtools/client/memory/test/browser/browser.ini
@@ -10,14 +10,15 @@ support-files =
     skip-if = debug # bug 1219554
 [browser_memory_breakdowns_01.js]
 [browser_memory_clear_snapshots.js]
 [browser_memory_diff_01.js]
 [browser_memory_dominator_trees_01.js]
 [browser_memory_dominator_trees_02.js]
 [browser_memory_filter_01.js]
 [browser_memory_keyboard.js]
+[browser_memory_keyboard-snapshot-list.js]
 [browser_memory_no_allocation_stacks.js]
 [browser_memory_no_auto_expand.js]
     skip-if = debug # bug 1219554
 [browser_memory_percents_01.js]
 [browser_memory_simple_01.js]
 [browser_memory_transferHeapSnapshot_e10s_01.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/memory/test/browser/browser_memory_keyboard-snapshot-list.js
@@ -0,0 +1,94 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that using ACCEL+UP/DOWN, the user can navigate between snapshots.
+
+"use strict";
+
+const {
+  snapshotState
+} = require("devtools/client/memory/constants");
+const {
+  takeSnapshotAndCensus
+} = require("devtools/client/memory/actions/snapshot");
+
+const TEST_URL = "http://example.com/browser/devtools/client/memory/test/browser/doc_steady_allocation.html";
+
+this.test = makeMemoryTest(TEST_URL, function* ({ panel }) {
+  // Creating snapshots already takes ~25 seconds on linux 32 debug machines
+  // which makes the test very likely to go over the allowed timeout
+  requestLongerTimeout(2);
+
+  const heapWorker = panel.panelWin.gHeapAnalysesClient;
+  const front = panel.panelWin.gFront;
+  const store = panel.panelWin.gStore;
+  const { dispatch } = store;
+  const doc = panel.panelWin.document;
+
+  info("Take 3 snapshots");
+  dispatch(takeSnapshotAndCensus(front, heapWorker));
+  dispatch(takeSnapshotAndCensus(front, heapWorker));
+  dispatch(takeSnapshotAndCensus(front, heapWorker));
+
+  yield waitUntilState(store, state =>
+    state.snapshots.length == 3 &&
+    state.snapshots.every(s => s.state === snapshotState.SAVED_CENSUS));
+  ok(true, "All snapshots are in SAVED_CENSUS state");
+
+  yield waitUntilSnapshotSelected(store, 2);
+  ok(true, "Third snapshot selected after creating all snapshots.");
+
+  info("Press ACCEL+UP key, expect second snapshot selected.");
+  EventUtils.synthesizeKey("VK_UP", { accelKey: true }, panel.panelWin);
+  yield waitUntilSnapshotSelected(store, 1);
+  ok(true, "Second snapshot selected after alt+UP.");
+
+  info("Press ACCEL+UP key, expect first snapshot selected.");
+  EventUtils.synthesizeKey("VK_UP", { accelKey: true }, panel.panelWin);
+  yield waitUntilSnapshotSelected(store, 0);
+  ok(true, "First snapshot is selected after ACCEL+UP");
+
+  info("Check ACCEL+UP is a noop when the first snapshot is selected.");
+  EventUtils.synthesizeKey("VK_UP", { accelKey: true }, panel.panelWin);
+  // We assume the snapshot selection should be synchronous here.
+  is(getSelectedSnapshotIndex(store), 0, "First snapshot is still selected");
+
+  info("Press ACCEL+DOWN key, expect second snapshot selected.");
+  EventUtils.synthesizeKey("VK_DOWN", { accelKey: true }, panel.panelWin);
+  yield waitUntilSnapshotSelected(store, 1);
+  ok(true, "Second snapshot is selected after ACCEL+DOWN");
+
+  info("Click on first node.");
+  let firstNode = doc.querySelector(".tree .heap-tree-item-name");
+  EventUtils.synthesizeMouseAtCenter(firstNode, {}, panel.panelWin);
+  yield waitUntilState(store, state => state.snapshots[1].census.focused ===
+      state.snapshots[1].census.report.children[0]
+  );
+  ok(true, "First root is selected after click.");
+
+  info("Press DOWN key, expect second root focused.");
+  EventUtils.synthesizeKey("VK_DOWN", {}, panel.panelWin);
+  yield waitUntilState(store, state => state.snapshots[1].census.focused ===
+      state.snapshots[1].census.report.children[1]
+  );
+  ok(true, "Second root is selected after pressing DOWN.");
+  is(getSelectedSnapshotIndex(store), 1, "Second snapshot is still selected");
+
+  info("Press UP key, expect second root focused.");
+  EventUtils.synthesizeKey("VK_UP", {}, panel.panelWin);
+  yield waitUntilState(store, state => state.snapshots[1].census.focused ===
+      state.snapshots[1].census.report.children[0]
+  );
+  ok(true, "First root is selected after pressing UP.");
+  is(getSelectedSnapshotIndex(store), 1, "Second snapshot is still selected");
+
+  info("Press ACCEL+DOWN key, expect third snapshot selected.");
+  EventUtils.synthesizeKey("VK_DOWN", { accelKey: true }, panel.panelWin);
+  yield waitUntilSnapshotSelected(store, 2);
+  ok(true, "ThirdĖ† snapshot is selected after ACCEL+DOWN");
+
+  info("Check ACCEL+DOWN is a noop when the last snapshot is selected.");
+  EventUtils.synthesizeKey("VK_DOWN", { accelKey: true }, panel.panelWin);
+  // We assume the snapshot selection should be synchronous here.
+  is(getSelectedSnapshotIndex(store), 2, "Third snapshot is still selected");
+});
--- a/devtools/client/memory/test/browser/head.js
+++ b/devtools/client/memory/test/browser/head.js
@@ -137,8 +137,31 @@ function setBreakdown (window, type) {
  * displayed.
  *
  * @param {Document} document
  */
 function getDisplayedSnapshotStatus(document) {
   const status = document.querySelector(".snapshot-status");
   return status ? status.textContent.trim() : null;
 }
+
+/**
+ * Get the index of the currently selected snapshot.
+ *
+ * @return {Number}
+ */
+function getSelectedSnapshotIndex(store) {
+  let snapshots = store.getState().snapshots;
+  let selectedSnapshot = snapshots.find(s => s.selected);
+  return snapshots.indexOf(selectedSnapshot);
+}
+
+/**
+ * Returns a promise that will resolve when the snapshot with provided index
+ * becomes selected.
+ *
+ * @return {Promise}
+ */
+function waitUntilSnapshotSelected(store, snapshotIndex) {
+  return waitUntilState(store, state =>
+    state.snapshots[snapshotIndex] &&
+    state.snapshots[snapshotIndex].selected === true);
+}
--- a/devtools/client/shared/components/test/mochitest/test_tree_06.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_06.html
@@ -271,16 +271,45 @@ window.onload = Task.async(function* () 
       "--H:false",
       "--I:false",
       "-D:false",
       "--J:false",
       "M:false",
       "-N:false",
       "--O:false",
     ], "After the RIGHT, K should be focused.");
+
+    // Check that keys are ignored if any modifier is present.
+    let keysWithModifier = [
+      { key: "ArrowDown", altKey: true },
+      { key: "ArrowDown", ctrlKey: true },
+      { key: "ArrowDown", metaKey: true },
+      { key: "ArrowDown", shiftKey: true },
+    ];
+    for (let key of keysWithModifier) {
+      Simulate.keyDown(document.querySelector(".tree"), key);
+      yield forceRender(tree);
+      isRenderedTree(document.body.textContent, [
+        "A:false",
+        "-B:false",
+        "--E:false",
+        "---K:true",
+        "---L:false",
+        "--F:false",
+        "--G:false",
+        "-C:false",
+        "--H:false",
+        "--I:false",
+        "-D:false",
+        "--J:false",
+        "M:false",
+        "-N:false",
+        "--O:false",
+      ], "After DOWN + (alt|ctrl|meta|shift), K should remain focused.");
+    }
   } catch(e) {
     ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
   } finally {
     SimpleTest.finish();
   }
 });
 </script>
 </pre>
--- a/devtools/client/shared/components/tree.js
+++ b/devtools/client/shared/components/tree.js
@@ -400,16 +400,21 @@ const Tree = module.exports = createClas
    *
    * @param {Event} e
    */
   _onKeyDown(e) {
     if (this.props.focused == null) {
       return;
     }
 
+    // Allow parent nodes to use navigation arrows with modifiers.
+    if (e.altKey || e.ctrlKey || e.shiftKey || e.metaKey) {
+      return;
+    }
+
     // Prevent scrolling when pressing navigation keys. Guard against mocked
     // events received when testing.
     if (e.nativeEvent && e.nativeEvent.preventDefault) {
       ViewHelpers.preventScrolling(e.nativeEvent);
     }
 
     switch (e.key) {
       case "ArrowUp":