Fix profiling screenshots data structure to map rootID to commitIndex
Propagate `rootID` throughout the code for `captureScreenshot`. Rename private profiling maps of `store` to make relations more clear. Fix missing cleanup for screenshots data in `set importedProfilingData` of `store`.
Ivan Babak committed
May 1, 2019 at 01:07 UTC
c6c71ef8f900341bfd2778c8dd60a45f34ec0273
6 files changed
+69
-39
shells/browser/shared/src/background.js
+2
-1
@@ -122,7 +122,7 @@ chrome.runtime.onMessage.addListener((request, sender) => {
122
}
123
124
if (request.captureScreenshot) {
125
- const { commitIndex } = request;
125
+ const { commitIndex, rootID } = request;
126
try {
127
chrome.tabs.captureVisibleTab(undefined, undefined, dataURL => {
128
// TODO For some reason, sending a response using the third param (sendResponse) doesn't work,
@@ -134,6 +134,7 @@ chrome.runtime.onMessage.addListener((request, sender) => {
134
payload: {
135
commitIndex,
136
dataURL,
137
+ rootID,
138
},
139
});
140
}
shells/browser/shared/src/main.js
+2
-1
@@ -77,11 +77,12 @@ function createPanelIfReactLoaded() {
77
filename,
78
});
79
});
80
- bridge.addListener('captureScreenshot', ({ commitIndex }) => {
80
+ bridge.addListener('captureScreenshot', ({ commitIndex, rootID }) => {
81
chrome.runtime.sendMessage(
82
{
83
captureScreenshot: true,
84
commitIndex,
85
+ rootID,
86
},
87
response => bridge.send('screenshotCaptured', response)
88
);
shells/dev/src/backend.js
+2
-1
@@ -20,11 +20,12 @@ const bridge = new Bridge({
20
},
21
});
22
23
-bridge.addListener('captureScreenshot', ({ commitIndex }) => {
23
+bridge.addListener('captureScreenshot', ({ commitIndex, rootID }) => {
24
html2canvas(document.body, { logging: false }).then(canvas => {
25
bridge.send('screenshotCaptured', {
26
commitIndex,
27
dataURL: canvas.toDataURL(),
28
+ rootID,
29
});
30
});
31
});
src/backend/agent.js
+8
-2
@@ -125,8 +125,14 @@ export default class Agent extends EventEmitter {
125
}
126
}
127
128
- captureScreenshot = ({ commitIndex }: { commitIndex: number }) => {
129
- this._bridge.send('captureScreenshot', { commitIndex });
128
+ captureScreenshot = ({
129
+ commitIndex,
130
+ rootID,
131
+ }: {
132
+ commitIndex: number,
133
+ rootID: number,
134
+ }) => {
135
+ this._bridge.send('captureScreenshot', { commitIndex, rootID });
136
};
137
138
getIDForNode(node: Object): number | null {
src/devtools/store.js
+50
-32
@@ -91,16 +91,20 @@ export default class Store extends EventEmitter {
91
// Map of root (id) to a list of tree mutation that occur during profiling.
92
// Once profiling is finished, these mutations can be used, along with the initial tree snapshots,
93
// to reconstruct the state of each root for each commit.
94
- _profilingOperations: Map<number, Array<Uint32Array>> = new Map();
94
+ _profilingOperationsByRootID: Map<number, Array<Uint32Array>> = new Map();
95
96
+ // Map of root (id) to a Map of screenshots by commit ID.
97
// Stores screenshots for each commit (when profiling).
97
- _profilingScreenshots: Map<number, string> = new Map();
98
+ _profilingScreenshotsByRootID: Map<number, Map<number, string>> = new Map();
99
100
// Snapshot of the state of the main Store (including all roots) when profiling started.
101
// Once profiling is finished, this snapshot can be used along with "operations" messages emitted during profiling,
102
// to reconstruct the state of each root for each commit.
103
// It's okay to use a single root to store this information because node IDs are unique across all roots.
103
- _profilingSnapshot: Map<number, ProfilingSnapshotNode> = new Map();
104
+ _profilingSnapshotsByElementID: Map<
105
+ number,
106
+ ProfilingSnapshotNode
107
+ > = new Map();
108
109
// Incremented each time the store is mutated.
110
// This enables a passive effect to detect a mutation between render and commit phase.
@@ -187,14 +191,14 @@ export default class Store extends EventEmitter {
191
if (this._ownersMap.size !== 0) {
192
throw new Error('Expected _ownersMap to be empty.');
193
}
190
- if (this._profilingOperations.size !== 0) {
191
- throw new Error('Expected _profilingOperations to be empty.');
194
+ if (this._profilingOperationsByRootID.size !== 0) {
195
+ throw new Error('Expected _profilingOperationsByRootID to be empty.');
196
}
193
- if (this._profilingScreenshots.size !== 0) {
194
- throw new Error('Expected _profilingScreenshots to be empty.');
197
+ if (this._profilingScreenshotsByRootID.size !== 0) {
198
+ throw new Error('Expected _profilingScreenshotsByRootID to be empty.');
199
}
196
- if (this._profilingSnapshot.size !== 0) {
197
- throw new Error('Expected _profilingSnapshot to be empty.');
200
+ if (this._profilingSnapshotsByElementID.size !== 0) {
201
+ throw new Error('Expected _profilingSnapshotsByElementID to be empty.');
202
}
203
if (this._rootIDToCapabilities.size !== 0) {
204
throw new Error('Expected _rootIDToCapabilities to be empty.');
@@ -239,7 +243,8 @@ export default class Store extends EventEmitter {
243
// Profiling data has been recorded for at least one root.
244
get hasProfilingData(): boolean {
245
return (
242
- this._importedProfilingData !== null || this._profilingOperations.size > 0
246
+ this._importedProfilingData !== null ||
247
+ this._profilingOperationsByRootID.size > 0
248
);
249
}
250
@@ -248,8 +253,9 @@ export default class Store extends EventEmitter {
253
}
254
set importedProfilingData(value: ImportedProfilingData | null): void {
255
this._importedProfilingData = value;
251
- this._profilingOperations = new Map();
252
- this._profilingSnapshot = new Map();
256
+ this._profilingOperationsByRootID = new Map();
257
+ this._profilingScreenshotsByRootID = new Map();
258
+ this._profilingSnapshotsByElementID = new Map();
259
this._profilingCache.invalidate();
260
261
this.emit('importedProfilingData');
@@ -268,15 +274,15 @@ export default class Store extends EventEmitter {
274
}
275
276
get profilingOperations(): Map<number, Array<Uint32Array>> {
271
- return this._profilingOperations;
277
+ return this._profilingOperationsByRootID;
278
}
279
274
- get profilingScreenshots(): Map<number, string> {
275
- return this._profilingScreenshots;
280
+ get profilingScreenshots(): Map<number, Map<number, string>> {
281
+ return this._profilingScreenshotsByRootID;
282
}
283
284
get profilingSnapshot(): Map<number, ProfilingSnapshotNode> {
279
- return this._profilingSnapshot;
285
+ return this._profilingSnapshotsByElementID;
286
}
287
288
get revision(): number {
@@ -305,9 +311,9 @@ export default class Store extends EventEmitter {
311
312
clearProfilingData(): void {
313
this._importedProfilingData = null;
308
- this._profilingOperations = new Map();
309
- this._profilingScreenshots = new Map();
310
- this._profilingSnapshot = new Map();
314
+ this._profilingOperationsByRootID = new Map();
315
+ this._profilingScreenshotsByRootID = new Map();
316
+ this._profilingSnapshotsByElementID = new Map();
317
318
// Invalidate suspense cache if profiling data is being (re-)recorded.
319
// Note that we clear now because any existing data is "stale".
@@ -639,17 +645,17 @@ export default class Store extends EventEmitter {
645
}
646
647
_captureScreenshot = throttle(
642
- memoize((commitIndex: number) => {
643
- this._bridge.send('captureScreenshot', { commitIndex });
648
+ memoize((rootID: number, commitIndex: number) => {
649
+ this._bridge.send('captureScreenshot', { commitIndex, rootID });
650
}),
651
THROTTLE_CAPTURE_SCREENSHOT_DURATION
652
);
653
648
- _takeProfilingSnapshotRecursive = (id: number) => {
649
- const element = this.getElementByID(id);
654
+ _takeProfilingSnapshotRecursive = (elementID: number) => {
655
+ const element = this.getElementByID(elementID);
656
if (element !== null) {
651
- this._profilingSnapshot.set(id, {
652
- id,
657
+ this._profilingSnapshotsByElementID.set(elementID, {
658
+ id: elementID,
659
children: element.children.slice(0),
660
displayName: element.displayName,
661
key: element.key,
@@ -703,17 +709,17 @@ export default class Store extends EventEmitter {
709
const rootID = operations[1];
710
711
if (this._isProfiling) {
706
- let profilingOperations = this._profilingOperations.get(rootID);
712
+ let profilingOperations = this._profilingOperationsByRootID.get(rootID);
713
if (profilingOperations == null) {
714
profilingOperations = [operations];
709
- this._profilingOperations.set(rootID, profilingOperations);
715
+ this._profilingOperationsByRootID.set(rootID, profilingOperations);
716
} else {
717
profilingOperations.push(operations);
718
}
719
720
if (this._captureScreenshots) {
721
const commitIndex = profilingOperations.length - 1;
716
- this._captureScreenshot(commitIndex);
722
+ this._captureScreenshot(rootID, commitIndex);
723
}
724
}
725
@@ -990,9 +996,9 @@ export default class Store extends EventEmitter {
996
onProfilingStatus = (isProfiling: boolean) => {
997
if (isProfiling) {
998
this._importedProfilingData = null;
993
- this._profilingOperations = new Map();
994
- this._profilingScreenshots = new Map();
995
- this._profilingSnapshot = new Map();
999
+ this._profilingOperationsByRootID = new Map();
1000
+ this._profilingScreenshotsByRootID = new Map();
1001
+ this._profilingSnapshotsByElementID = new Map();
1002
this.roots.forEach(this._takeProfilingSnapshotRecursive);
1003
}
1004
@@ -1011,11 +1017,23 @@ export default class Store extends EventEmitter {
1017
onScreenshotCaptured = ({
1018
commitIndex,
1019
dataURL,
1020
+ rootID,
1021
}: {|
1022
commitIndex: number,
1023
dataURL: string,
1024
+ rootID: number,
1025
|}) => {
1018
- this._profilingScreenshots.set(commitIndex, dataURL);
1026
+ let profilingScreenshotsForRootByCommitIndex = this._profilingScreenshotsByRootID.get(
1027
+ rootID
1028
+ );
1029
+ if (!profilingScreenshotsForRootByCommitIndex) {
1030
+ profilingScreenshotsForRootByCommitIndex = new Map();
1031
+ this._profilingScreenshotsByRootID.set(
1032
+ rootID,
1033
+ profilingScreenshotsForRootByCommitIndex
1034
+ );
1035
+ }
1036
+ profilingScreenshotsForRootByCommitIndex.set(commitIndex, dataURL);
1037
};
1038
1039
onBridgeShutdown = () => {
src/devtools/views/Profiler/SidebarCommitInfo.js
+5
-2
@@ -24,10 +24,13 @@ export default function SidebarCommitInfo(_: Props) {
24
profilingScreenshots,
25
} = useContext(StoreContext);
26
27
+ const screenshotsByCommitIndex =
28
+ rootID !== null ? profilingScreenshots.get(rootID) : null;
29
const screenshot =
28
- selectedCommitIndex !== null
29
- ? profilingScreenshots.get(selectedCommitIndex)
30
+ screenshotsByCommitIndex != null && selectedCommitIndex !== null
31
+ ? screenshotsByCommitIndex.get(selectedCommitIndex)
32
: null;
33
+
34
const [
35
isScreenshotModalVisible,
36
setIsScreenshotModalVisible,