From 49395e2d3470422973a350853be268e62b8c249c Mon Sep 17 00:00:00 2001 From: Greg Tatum Date: Fri, 15 Apr 2016 09:36:00 +0200 Subject: [PATCH] Bug 1242628 - Add ability to remove a single snapshot from the list. r=fitzgen --HG-- extra : rebase_source : 59021ac5bb294442180a064eaa4ad0ee7a65a3ec --- .../client/locales/en-US/memory.properties | 4 + devtools/client/memory/actions/snapshot.js | 27 +++++- devtools/client/memory/app.js | 2 + .../memory/components/snapshot-list-item.js | 14 +++- devtools/client/memory/test/chrome/chrome.ini | 2 + devtools/client/memory/test/chrome/head.js | 84 +++++++++++-------- .../memory/test/chrome/test_List_01.html | 74 ++++++++++++++++ .../test/chrome/test_SnapshotListItem_01.html | 53 ++++++++++++ devtools/client/themes/memory.css | 21 +++++ 9 files changed, 240 insertions(+), 41 deletions(-) create mode 100644 devtools/client/memory/test/chrome/test_List_01.html create mode 100644 devtools/client/memory/test/chrome/test_SnapshotListItem_01.html diff --git a/devtools/client/locales/en-US/memory.properties b/devtools/client/locales/en-US/memory.properties index 230eb88055b9..8a4d625bc6c7 100644 --- a/devtools/client/locales/en-US/memory.properties +++ b/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 diff --git a/devtools/client/memory/actions/snapshot.js b/devtools/client/memory/actions/snapshot.js index 98e5c99c2bf8..1752df0afb6e 100644 --- a/devtools/client/memory/actions/snapshot.js +++ b/devtools/client/memory/actions/snapshot.js @@ -482,6 +482,9 @@ const refreshSelectedCensus = exports.refreshSelectedCensus = function (heapWork const refreshSelectedTreeMap = exports.refreshSelectedTreeMap = function (heapWorker) { return function*(dispatch, getState) { let snapshot = getState().snapshots.find(s => s.selected); + if (!snapshot || snapshot.state !== states.READ) { + return; + } // Intermediate snapshot states will get handled by the task action that is // orchestrating them. For example, if the snapshot census's state is @@ -489,8 +492,7 @@ const refreshSelectedTreeMap = exports.refreshSelectedTreeMap = function (heapWo // the inverted property matches the inverted state. If the snapshot is // still in the process of being saved or read, the takeSnapshotAndCensus // task action will follow through and ensure that a census is taken. - if (snapshot && - (snapshot.treeMap && snapshot.treeMap.state === treeMapState.SAVED) || + if ((snapshot.treeMap && snapshot.treeMap.state === treeMapState.SAVED) || !snapshot.treeMap) { yield dispatch(takeTreeMap(heapWorker, snapshot.id)); } @@ -761,6 +763,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. * diff --git a/devtools/client/memory/app.js b/devtools/client/memory/app.js index 10a1930efb69..5dd63bda69c5 100644 --- a/devtools/client/memory/app.js +++ b/devtools/client/memory/app.js @@ -30,6 +30,7 @@ const { selectSnapshotAndRefresh, takeSnapshotAndCensus, clearSnapshots, + deleteSnapshot, fetchImmediatelyDominated, expandCensusNode, collapseCensusNode, @@ -210,6 +211,7 @@ const MemoryApp = createClass({ itemComponent: SnapshotListItem, items: snapshots, onSave: snapshot => dispatch(pickFileAndExportSnapshot(snapshot)), + onDelete: snapshot => dispatch(deleteSnapshot(heapWorker, snapshot)), onClick: onClickSnapshotListItem, diffing, }), diff --git a/devtools/client/memory/components/snapshot-list-item.js b/devtools/client/memory/components/snapshot-list-item.js index 91f0c02bf1f1..37db81d13be9 100644 --- a/devtools/client/memory/components/snapshot-list-item.js +++ b/devtools/client/memory/components/snapshot-list-item.js @@ -26,12 +26,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; @@ -88,13 +89,20 @@ const SnapshotListItem = module.exports = createClass({ let saveLink = !snapshot.path ? void 0 : dom.a({ onClick: () => onSave(snapshot), className: "save", - }, L10N.getFormatStr("snapshot.io.save")); + }, L10N.getStr("snapshot.io.save")); + + let deleteButton = !snapshot.path ? void 0 : dom.button({ + onClick: () => onDelete(snapshot), + className: "devtools-button delete", + title: L10N.getStr("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, diff --git a/devtools/client/memory/test/chrome/chrome.ini b/devtools/client/memory/test/chrome/chrome.ini index 0f0ebc1796fd..7803bcda35f1 100644 --- a/devtools/client/memory/test/chrome/chrome.ini +++ b/devtools/client/memory/test/chrome/chrome.ini @@ -12,7 +12,9 @@ support-files = [test_Heap_03.html] [test_Heap_04.html] [test_Heap_05.html] +[test_List_01.html] [test_ShortestPaths_01.html] [test_ShortestPaths_02.html] +[test_SnapshotListItem_01.html] [test_Toolbar_01.html] [test_TreeMap_01.html] diff --git a/devtools/client/memory/test/chrome/head.js b/devtools/client/memory/test/chrome/head.js index 63a9308d2ef1..8e7145408609 100644 --- a/devtools/client/memory/test/chrome/head.js +++ b/devtools/client/memory/test/chrome/head.js @@ -53,6 +53,8 @@ var DominatorTreeComponent = React.createFactory(require("devtools/client/memory var DominatorTreeItem = React.createFactory(require("devtools/client/memory/components/dominator-tree-item")); var ShortestPaths = React.createFactory(require("devtools/client/memory/components/shortest-paths")); var TreeMap = React.createFactory(require("devtools/client/memory/components/tree-map")); +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. @@ -163,6 +165,43 @@ var TEST_SHORTEST_PATHS_PROPS = Object.freeze({ }), }); +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 }), + }), + display: Object.freeze({ + displayName: "Test Display", + tooltip: "Test display tooltup", + inverted: false, + 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 }), + }), + }), + state: censusState.SAVED, + inverted: false, + filter: null, + expanded: new Set(), + focused: null, + parentMap: Object.freeze(Object.create(null)) + }), + dominatorTree: TEST_DOMINATOR_TREE, + error: null, + imported: false, + creationTime: 0, + state: snapshotState.READ, +}); + var TEST_HEAP_PROPS = Object.freeze({ onSnapshotClick: noop, onLoadMoreSiblings: noop, @@ -175,42 +214,7 @@ var TEST_HEAP_PROPS = Object.freeze({ onViewSourceInDebugger: noop, diffing: null, view: { state: 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 }), - }), - display: Object.freeze({ - displayName: "Test Display", - tooltip: "Test display tooltup", - inverted: false, - 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 }), - }), - }), - state: censusState.SAVED, - inverted: false, - filter: null, - expanded: new Set(), - focused: null, - parentMap: Object.freeze(Object.create(null)) - }), - dominatorTree: TEST_DOMINATOR_TREE, - error: null, - imported: false, - creationTime: 0, - state: snapshotState.READ, - }), + snapshot: TEST_SNAPSHOT, sizes: Object.freeze({ shortestPathsSize: .5 }), onShortestPathsResize: noop, }); @@ -285,6 +289,14 @@ var TEST_TREE_MAP_PROPS = Object.freeze({ }) }); +var TEST_SNAPSHOT_LIST_ITEM_PROPS = Object.freeze({ + onClick: noop, + onSave: noop, + onDelete: noop, + item: TEST_SNAPSHOT, + index: 1234, +}); + function onNextAnimationFrame(fn) { return () => requestAnimationFrame(() => diff --git a/devtools/client/memory/test/chrome/test_List_01.html b/devtools/client/memory/test/chrome/test_List_01.html new file mode 100644 index 000000000000..911a7bc77f8e --- /dev/null +++ b/devtools/client/memory/test/chrome/test_List_01.html @@ -0,0 +1,74 @@ + + + + + + Tree component test + + + + +
+ + 
+        
+        
+
+ + diff --git a/devtools/client/memory/test/chrome/test_SnapshotListItem_01.html b/devtools/client/memory/test/chrome/test_SnapshotListItem_01.html new file mode 100644 index 000000000000..0081496cec15 --- /dev/null +++ b/devtools/client/memory/test/chrome/test_SnapshotListItem_01.html @@ -0,0 +1,53 @@ + + + + + + Tree component test + + + + +
+ + 
+        
+        
+
+ + diff --git a/devtools/client/themes/memory.css b/devtools/client/themes/memory.css index 74f71fa7a4db..7032e45d2c88 100644 --- a/devtools/client/themes/memory.css +++ b/devtools/client/themes/memory.css @@ -197,11 +197,32 @@ 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; + position: relative; + min-height: 1em; + min-width: 1.3em; +} + +.theme-light .snapshot-list-item.selected .delete { + filter: invert(100%); +} + +.snapshot-list-item .delete::before { + background-image: url("chrome://devtools/skin/images/close.svg"); + background-position: 0.2em 0; +} + .snapshot-list-item > .snapshot-title { margin-bottom: 14px; }