Bug 1242628 - Add ability to remove a single snapshot from the list -r=fitzgen

devtools/client/locales/en-US/memory.properties

@@ -27,6 +27,10 @@ memory.tooltip=Memory
 # snapshot to disk.
 snapshot.io.save=Save
 
+# LOCALIZATION NOTE (snapshot.io.delete): The label for the link that deletes
+# a snapshot
+snapshot.io.delete=Delete
+
 # LOCALIZATION NOTE (snapshot.io.save.window): The title for the window
 # displayed when saving a snapshot to disk.
 snapshot.io.save.window=Save Heap Snapshot

devtools/client/memory/actions/snapshot.js

@@ -440,6 +440,27 @@ const clearSnapshots = exports.clearSnapshots = function (heapWorker) {
   };
 };
 
+/**
+ * Delete a snapshot
+ *
+ * @param {HeapAnalysesClient} heapWorker
+ * @param {snapshotModel} snapshot
+ */
+const deleteSnapshot = exports.deleteSnapshot = function (heapWorker, snapshot) {
+  return function*(dispatch, getState) {
+    dispatch({ type: actions.DELETE_SNAPSHOTS_START, ids: [snapshot.id] });
+
+    try {
+      yield heapWorker.deleteHeapSnapshot(snapshot.path);
+    } catch (error) {
+      reportException("deleteSnapshot", error);
+      dispatch({ type: actions.SNAPSHOT_ERROR, id: snapshot.id, error });
+    }
+
+    dispatch({ type: actions.DELETE_SNAPSHOTS_END, ids: [snapshot.id] });
+  };
+};
+
 /**
  * Expand the given node in the snapshot's census report.
  *

devtools/client/memory/app.js

@@ -24,6 +24,7 @@ const {
   selectSnapshotAndRefresh,
   takeSnapshotAndCensus,
   clearSnapshots,
+  deleteSnapshot,
   fetchImmediatelyDominated,
   expandCensusNode,
   collapseCensusNode,
@@ -166,6 +167,7 @@ const MemoryApp = createClass({
             itemComponent: SnapshotListItem,
             items: snapshots,
             onSave: snapshot => dispatch(pickFileAndExportSnapshot(snapshot)),
+            onDelete: snapshot => dispatch(deleteSnapshot(heapWorker, snapshot)),
             onClick: onClickSnapshotListItem,
             diffing,
           }),

devtools/client/memory/components/snapshot-list-item.js

@@ -20,12 +20,13 @@ const SnapshotListItem = module.exports = createClass({
   propTypes: {
     onClick: PropTypes.func.isRequired,
     onSave: PropTypes.func.isRequired,
+    onDelete: PropTypes.func.isRequired,
     item: snapshotModel.isRequired,
     index: PropTypes.number.isRequired,
   },
 
   render() {
-    let { index, item: snapshot, onClick, onSave, diffing } = this.props;
+    let { index, item: snapshot, onClick, onSave, onDelete, diffing } = this.props;
     let className = `snapshot-list-item ${snapshot.selected ? " selected" : ""}`;
     let statusText = getStatusText(snapshot.state);
     let wantThrobber = !!statusText;
@@ -77,11 +78,18 @@ const SnapshotListItem = module.exports = createClass({
       className: "save",
     }, L10N.getFormatStr("snapshot.io.save"));
 
+    let deleteButton = !snapshot.path ? void 0 : dom.button({
+      onClick: () => onDelete(snapshot),
+      className: "devtools-button delete",
+      title: L10N.getFormatStr("snapshot.io.delete")
+    });
+
     return (
       dom.li({ className, onClick },
         dom.span({ className: `snapshot-title ${wantThrobber ? " devtools-throbber" : ""}` },
           checkbox,
-          title
+          title,
+          deleteButton
         ),
         dom.span({ className: "snapshot-info" },
           details,

devtools/client/memory/test/chrome/chrome.ini

@@ -10,5 +10,7 @@ support-files =
 [test_Heap_02.html]
 [test_Heap_03.html]
 [test_Heap_04.html]
+[test_List_01.html]
+[test_SnapshotListItem_01.html]
 [test_Toolbar_01.html]
 [test_Toolbar_02.html]

devtools/client/memory/test/chrome/head.js

@@ -37,6 +37,8 @@ var ReactDOM = require("devtools/client/shared/vendor/react-dom");
 var Heap = React.createFactory(require("devtools/client/memory/components/heap"));
 var DominatorTreeComponent = React.createFactory(require("devtools/client/memory/components/dominator-tree"));
 var DominatorTreeItem = React.createFactory(require("devtools/client/memory/components/dominator-tree-item"));
+var SnapshotListItem = React.createFactory(require("devtools/client/memory/components/snapshot-list-item"));
+var List = React.createFactory(require("devtools/client/memory/components/list"));
 var Toolbar = React.createFactory(require("devtools/client/memory/components/toolbar"));
 
 // All tests are asynchronous.
@@ -105,6 +107,36 @@ var TEST_DOMINATOR_TREE_PROPS = Object.freeze({
   onCollapse: noop,
 });
 
+var TEST_SNAPSHOT = Object.freeze({
+  id: 1337,
+  selected: true,
+  path: "/fake/path/to/snapshot",
+  census: Object.freeze({
+    report: Object.freeze({
+      objects: Object.freeze({ count: 4, bytes: 400 }),
+      scripts: Object.freeze({ count: 3, bytes: 300 }),
+      strings: Object.freeze({ count: 2, bytes: 200 }),
+      other: Object.freeze({ count: 1, bytes: 100 }),
+    }),
+    breakdown: Object.freeze({
+      by: "coarseType",
+      objects: Object.freeze({ by: "count", count: true, bytes: true }),
+      scripts: Object.freeze({ by: "count", count: true, bytes: true }),
+      strings: Object.freeze({ by: "count", count: true, bytes: true }),
+      other: Object.freeze({ by: "count", count: true, bytes: true }),
+    }),
+    inverted: false,
+    filter: null,
+    expanded: new Set(),
+    focused: null,
+  }),
+  dominatorTree: TEST_DOMINATOR_TREE,
+  error: null,
+  imported: false,
+  creationTime: 0,
+  state: snapshotState.SAVED_CENSUS,
+});
+
 var TEST_HEAP_PROPS = Object.freeze({
   onSnapshotClick: noop,
   onLoadMoreSiblings: noop,
@@ -117,35 +149,7 @@ var TEST_HEAP_PROPS = Object.freeze({
   onViewSourceInDebugger: noop,
   diffing: null,
   view: viewState.CENSUS,
-  snapshot: Object.freeze({
-    id: 1337,
-    selected: true,
-    path: "/fake/path/to/snapshot",
-    census: Object.freeze({
-      report: Object.freeze({
-        objects: Object.freeze({ count: 4, bytes: 400 }),
-        scripts: Object.freeze({ count: 3, bytes: 300 }),
-        strings: Object.freeze({ count: 2, bytes: 200 }),
-        other: Object.freeze({ count: 1, bytes: 100 }),
-      }),
-      breakdown: Object.freeze({
-        by: "coarseType",
-        objects: Object.freeze({ by: "count", count: true, bytes: true }),
-        scripts: Object.freeze({ by: "count", count: true, bytes: true }),
-        strings: Object.freeze({ by: "count", count: true, bytes: true }),
-        other: Object.freeze({ by: "count", count: true, bytes: true }),
-      }),
-      inverted: false,
-      filter: null,
-      expanded: new Set(),
-      focused: null,
-    }),
-    dominatorTree: TEST_DOMINATOR_TREE,
-    error: null,
-    imported: false,
-    creationTime: 0,
-    state: snapshotState.SAVED_CENSUS,
-  }),
+  snapshot: TEST_SNAPSHOT
 });
 
 var TEST_TOOLBAR_PROPS = Object.freeze({
@@ -168,6 +172,14 @@ var TEST_TOOLBAR_PROPS = Object.freeze({
   snapshots: [],
 });
 
+var TEST_SNAPSHOT_LIST_ITEM_PROPS = Object.freeze({
+  onClick: noop,
+  onSave: noop,
+  onDelete: noop,
+  item: TEST_SNAPSHOT,
+  index: 1234,
+});
+
 function onNextAnimationFrame(fn) {
   return () =>
     requestAnimationFrame(() =>

devtools/client/memory/test/chrome/test_List_01.html

@@ -0,0 +1,74 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Test to verify the delete button calls the onDelete handler for an item
+-->
+<head>
+    <meta charset="utf-8">
+    <title>Tree component test</title>
+    <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+</head>
+<body>
+    <div id="container"></div>
+
+    <pre id="test">
+        <script src="head.js" type="application/javascript;version=1.8"></script>
+        <script type="application/javascript;version=1.8">
+         window.onload = Task.async(function* () {
+          try {
+            const container = document.getElementById("container");
+
+            let deletedSnapshots = [];
+
+            let snapshots = [ TEST_SNAPSHOT, TEST_SNAPSHOT, TEST_SNAPSHOT ]
+              .map((snapshot, index) => immutableUpdate(snapshot, {
+                index: snapshot.index + index
+              }));
+
+            yield renderComponent(
+              List({
+                itemComponent: SnapshotListItem,
+                onClick: noop,
+                onDelete: (item) => deletedSnapshots.push(item),
+                items: snapshots
+              }),
+              container
+            );
+
+            let deleteButtons = container.querySelectorAll('.delete');
+
+            is(container.querySelectorAll('.snapshot-list-item').length, 3,
+              "There are 3 list items\n");
+            is(deletedSnapshots.length, 0,
+              "Not snapshots have been deleted\n");
+
+            deleteButtons[1].click();
+
+            is(deletedSnapshots.length, 1, "One snapshot was deleted\n");
+            is(deletedSnapshots[0], snapshots[1],
+              "Deleted snapshot was added to the deleted list\n");
+
+            deleteButtons[0].click();
+
+            is(deletedSnapshots.length, 2, "Two snapshots were deleted\n");
+            is(deletedSnapshots[1], snapshots[0],
+              "Deleted snapshot was added to the deleted list\n");
+
+            deleteButtons[2].click();
+
+            is(deletedSnapshots.length, 3, "Three snapshots were deleted\n");
+            is(deletedSnapshots[2], snapshots[2],
+              "Deleted snapshot was added to the deleted list\n");
+
+
+          } catch(e) {
+            ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
+          } finally {
+            SimpleTest.finish();
+          }
+         });
+        </script>
+    </pre>
+</body>
+</html>

devtools/client/memory/test/chrome/test_SnapshotListItem_01.html

@@ -0,0 +1,53 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Test to verify that the delete button only shows up for a snapshot when it has a
+path.
+-->
+<head>
+    <meta charset="utf-8">
+    <title>Tree component test</title>
+    <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+</head>
+<body>
+    <div id="container"></div>
+
+    <pre id="test">
+        <script src="head.js" type="application/javascript;version=1.8"></script>
+        <script type="application/javascript;version=1.8">
+         window.onload = Task.async(function* () {
+          try {
+            const container = document.getElementById("container");
+
+            yield renderComponent(
+              SnapshotListItem(TEST_SNAPSHOT_LIST_ITEM_PROPS),
+              container
+            );
+
+            ok(container.querySelector('.delete'),
+               "Should have delete button when there is a path");
+
+            const pathlessProps = immutableUpdate(
+                TEST_SNAPSHOT_LIST_ITEM_PROPS,
+                {item: immutableUpdate(TEST_SNAPSHOT, {path: null})}
+            );
+
+            yield renderComponent(
+              SnapshotListItem(pathlessProps),
+              container
+            );
+
+            ok(!container.querySelector('.delete'),
+               "No delete button should be found if there is no path\n");
+
+          } catch(e) {
+            ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
+          } finally {
+            SimpleTest.finish();
+          }
+         });
+        </script>
+    </pre>
+</body>
+</html>

devtools/client/themes/memory.css

@@ -194,11 +194,38 @@ html, body, #app, #memory-tool {
   font-size: 90%;
 }
 
+.snapshot-list-item .snapshot-title {
+  display: flex;
+  justify-content: space-between;
+}
+
 .snapshot-list-item .save {
   text-decoration: underline;
   cursor: pointer;
 }
 
+.snapshot-list-item .delete {
+  cursor: pointer;
+  min-width: 20px;
+  position: relative;
+  top:-2px;
+  left:4px;
+}
+
+.theme-light .snapshot-list-item.selected .delete {
+  filter: invert(100%);
+}
+
+.snapshot-list-item .delete::before {
+  background-image: url(images/close.png);
+}
+
+@media (min-resolution: 1.1dppx) {
+  .snapshot-list-item .delete::before {
+    background-image: url(images/close@2x.png);
+  }
+}
+
 .snapshot-list-item > .snapshot-title {
   margin-bottom: 14px;
 }