Bug 1255804 - fix dominator view infinite loading when importing snapshot;r=fitzgen
Extracted new snapshot action to compute snapshot data: take census + compute
dominator-tree if the current view is the dominator-view (logic extracted from
takeSnapshotAndCensus action). This action is reused when importing a snpashot.
Added a new unit test to test the import from the dominator tree view.
Modified exising import unit test to check snapshot.census.display instead of
snapshot.display (which was undefined).
MozReview-Commit-ID: 8JN1arfaI1H
--- a/devtools/client/memory/actions/io.js
+++ b/devtools/client/memory/actions/io.js
@@ -2,17 +2,17 @@
* 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 { immutableUpdate, reportException, assert } = require("devtools/shared/DevToolsUtils");
const { snapshotState: states, actions } = require("../constants");
const { L10N, openFilePicker, createSnapshot } = require("../utils");
const telemetry = require("../telemetry");
-const { readSnapshot, takeCensus, selectSnapshot } = require("./snapshot");
+const { selectSnapshot, computeSnapshotData, readSnapshot } = require("./snapshot");
const { OS } = require("resource://gre/modules/osfile.jsm");
const VALID_EXPORT_STATES = [states.SAVED, states.READ, states.SAVING_CENSUS, states.SAVED_CENSUS];
exports.pickFileAndExportSnapshot = function (snapshot) {
return function* (dispatch, getState) {
let outputFile = yield openFilePicker({
title: L10N.getFormatStr("snapshot.io.save.window"),
defaultName: OS.Path.basename(snapshot.path),
@@ -75,17 +75,17 @@ const importSnapshotAndCensus = exports.
});
const id = snapshot.id;
dispatch({ type: actions.IMPORT_SNAPSHOT_START, snapshot });
dispatch(selectSnapshot(snapshot.id));
try {
yield dispatch(readSnapshot(heapWorker, id));
- yield dispatch(takeCensus(heapWorker, id));
+ yield dispatch(computeSnapshotData(heapWorker, id));
} catch (error) {
reportException("importSnapshot", error);
dispatch({ type: actions.IMPORT_SNAPSHOT_ERROR, error, id });
}
dispatch({ type: actions.IMPORT_SNAPSHOT_END, id });
};
};
--- a/devtools/client/memory/actions/snapshot.js
+++ b/devtools/client/memory/actions/snapshot.js
@@ -27,22 +27,37 @@ const diffing = require("./diffing");
const takeSnapshotAndCensus = exports.takeSnapshotAndCensus = function (front, heapWorker) {
return function *(dispatch, getState) {
const id = yield dispatch(takeSnapshot(front));
if (id === null) {
return;
}
yield dispatch(readSnapshot(heapWorker, id));
- if (getSnapshot(getState(), id).state === states.READ) {
- yield dispatch(takeCensus(heapWorker, id));
+ yield dispatch(computeSnapshotData(heapWorker, id));
+ };
+};
- if (getState().view === viewState.DOMINATOR_TREE) {
- yield dispatch(computeAndFetchDominatorTree(heapWorker, id));
- }
+/**
+ * Create the census for the snapshot with the provided snapshot id. If the
+ * current view is the DOMINATOR_TREE view, create the dominator tree for this
+ * snapshot as well.
+ *
+ * @param {HeapAnalysesClient} heapWorker
+ * @param {snapshotId} id
+ */
+const computeSnapshotData = exports.computeSnapshotData = function(heapWorker, id) {
+ return function* (dispatch, getState) {
+ if (getSnapshot(getState(), id).state !== states.READ) {
+ return;
+ }
+
+ yield dispatch(takeCensus(heapWorker, id));
+ if (getState().view === viewState.DOMINATOR_TREE) {
+ yield dispatch(computeAndFetchDominatorTree(heapWorker, id));
}
};
};
/**
* Selects a snapshot and if the snapshot's census is using a different
* display, take a new census.
*
--- a/devtools/client/memory/test/unit/test_action-import-snapshot-and-census.js
+++ b/devtools/client/memory/test/unit/test_action-import-snapshot-and-census.js
@@ -46,27 +46,27 @@ add_task(function *() {
ok(true, `Found expected state ${expected[i]}`);
i++;
}
};
let unsubscribe = subscribe(expectStates);
dispatch(importSnapshotAndCensus(heapWorker, destPath));
- yield waitUntilState(store, () => i === 5);
+ yield waitUntilState(store, () => i === expected.length);
unsubscribe();
- equal(i, 5, "importSnapshotAndCensus() produces the correct sequence of states in a snapshot");
+ equal(i, expected.length, "importSnapshotAndCensus() produces the correct sequence of states in a snapshot");
equal(getState().snapshots[1].state, states.SAVED_CENSUS, "imported snapshot is in SAVED_CENSUS state");
ok(getState().snapshots[1].selected, "imported snapshot is selected");
// Check snapshot data
let snapshot1 = getState().snapshots[0];
let snapshot2 = getState().snapshots[1];
- equal(snapshot1.display, snapshot2.display,
+ equal(snapshot1.census.display, snapshot2.census.display,
"imported snapshot has correct display");
// Clone the census data so we can destructively remove the ID/parents to compare
// equal census data
let census1 = stripUnique(JSON.parse(JSON.stringify(snapshot1.census.report)));
let census2 = stripUnique(JSON.parse(JSON.stringify(snapshot2.census.report)));
equal(JSON.stringify(census1), JSON.stringify(census2), "Imported snapshot has correct census");
new file mode 100644
--- /dev/null
+++ b/devtools/client/memory/test/unit/test_action-import-snapshot-dominator-tree.js
@@ -0,0 +1,78 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+/**
+ * Tests `importSnapshotAndCensus()` when importing snapshots from the dominator
+ * tree view. The snapshot is expected to be loaded and its dominator tree
+ * should be computed.
+ */
+
+let { snapshotState, dominatorTreeState, viewState, } =
+ require("devtools/client/memory/constants");
+let { importSnapshotAndCensus } = require("devtools/client/memory/actions/io");
+let { changeViewAndRefresh } = require("devtools/client/memory/actions/view");
+
+function run_test() {
+ run_next_test();
+}
+
+add_task(function* () {
+ let front = new StubbedMemoryFront();
+ let heapWorker = new HeapAnalysesClient();
+ yield front.attach();
+ let store = Store();
+ let { subscribe, dispatch, getState } = store;
+
+ dispatch(changeViewAndRefresh(viewState.DOMINATOR_TREE, heapWorker));
+ equal(getState().view, viewState.DOMINATOR_TREE,
+ "We should now be in the DOMINATOR_TREE view");
+
+ let i = 0;
+ let expected = [
+ "IMPORTING",
+ "READING",
+ "READ",
+ "SAVING_CENSUS",
+ "SAVED_CENSUS",
+ "dominatorTree:COMPUTING",
+ "dominatorTree:FETCHING",
+ "dominatorTree:LOADED",
+ ];
+ let expectStates = () => {
+ let snapshot = getState().snapshots[0];
+ if (snapshot && hasExpectedState(snapshot, expected[i])) {
+ ok(true, `Found expected state ${expected[i]}`);
+ i++;
+ }
+ };
+
+ let unsubscribe = subscribe(expectStates);
+ const snapshotPath = yield front.saveHeapSnapshot();
+ dispatch(importSnapshotAndCensus(heapWorker, snapshotPath));
+
+ yield waitUntilState(store, () => i === expected.length);
+ unsubscribe();
+ equal(i, expected.length, "importSnapshotAndCensus() produces the correct " +
+ "sequence of states in a snapshot");
+ equal(getState().snapshots[0].dominatorTree.state, dominatorTreeState.LOADED,
+ "imported snapshot's dominator tree is in LOADED state");
+ ok(getState().snapshots[0].selected, "imported snapshot is selected");
+});
+
+/**
+ * Check that the provided snapshot is in the expected state. The expected state
+ * is a snapshotState by default. If the expected state is prefixed by
+ * dominatorTree, a dominatorTree is expected on the provided snapshot, in the
+ * corresponding state from dominatorTreeState.
+ */
+function hasExpectedState(snapshot, expectedState) {
+ let isDominatorState = expectedState.indexOf("dominatorTree:") === 0;
+ if (isDominatorState) {
+ let state = dominatorTreeState[expectedState.replace("dominatorTree:", "")];
+ return snapshot.dominatorTree && snapshot.dominatorTree.state === state;
+ }
+
+ let state = snapshotState[expectedState];
+ return snapshot.state === state;
+}
--- a/devtools/client/memory/test/unit/xpcshell.ini
+++ b/devtools/client/memory/test/unit/xpcshell.ini
@@ -16,16 +16,17 @@ skip-if = toolkit == 'android' || toolki
[test_action-clear-snapshots_04.js]
[test_action-clear-snapshots_05.js]
[test_action-clear-snapshots_06.js]
[test_action-export-snapshot.js]
[test_action-filter-01.js]
[test_action-filter-02.js]
[test_action-filter-03.js]
[test_action-import-snapshot-and-census.js]
+[test_action-import-snapshot-dominator-tree.js]
[test_action-select-snapshot.js]
[test_action-set-display.js]
[test_action-set-display-and-refresh-01.js]
[test_action-set-display-and-refresh-02.js]
[test_action-take-census.js]
[test_action-take-snapshot.js]
[test_action-take-snapshot-and-census.js]
[test_action-toggle-inverted.js]