Skip to content

Commit d0889db

Browse files
ergunshDevtools-frontend LUCI CQ
authored andcommitted
[Memory] Release all animations before memory operations
This is a workaround to remove user facing issue of having detached elements in the profiles because of the animations panel. What happens is, we keep the animations for replay in the animations panel. However, when a DOM node is removed; we still keep the animation related to the DOM node in the animations panel and this causes the node to be retained and show up in the memory profiles as detached elements. Fixed: 400635410 Change-Id: I686799c1069978fbb78250d88ae6459a5ee26572 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6406316 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Ergün Erdoğmuş <[email protected]>
1 parent 2dfe194 commit d0889db

File tree

9 files changed

+142
-6
lines changed

9 files changed

+142
-6
lines changed

front_end/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ group("unittests") {
191191
"panels/network:unittests",
192192
"panels/network/components:unittests",
193193
"panels/performance_monitor:unittests",
194+
"panels/profiler:unittests",
194195
"panels/protocol_monitor:unittests",
195196
"panels/recorder:unittests",
196197
"panels/recorder/components:unittests",

front_end/legacy_test_runner/heap_profiler_test_runner/heap_profiler_test_runner.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ HeapProfilerTestRunner.startSamplingHeapProfiler = async function() {
777777
resolve =>
778778
UI.Context.Context.instance().addFlavorChangeListener(SDK.HeapProfilerModel.HeapProfilerModel, resolve));
779779
}
780-
Profiler.HeapProfileView.SamplingHeapProfileType.instance.startRecordingProfile();
780+
await Profiler.HeapProfileView.SamplingHeapProfileType.instance.startRecordingProfile();
781781
};
782782

783783
HeapProfilerTestRunner.stopSamplingHeapProfiler = function() {

front_end/panels/profiler/BUILD.gn

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,18 @@ devtools_entrypoint("meta") {
9696

9797
visibility = [ "../../entrypoints/*" ]
9898
}
99+
100+
ts_library("unittests") {
101+
testonly = true
102+
103+
sources = [
104+
"HeapDetachedElementsView.test.ts",
105+
"HeapProfileView.test.ts",
106+
"HeapSnapshotView.test.ts",
107+
]
108+
109+
deps = [
110+
":bundle",
111+
"../../testing",
112+
]
113+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2025 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import * as SDK from '../../core/sdk/sdk.js';
6+
import {createTarget} from '../../testing/EnvironmentHelpers.js';
7+
import {expectCall} from '../../testing/ExpectStubCall.js';
8+
import {describeWithMockConnection} from '../../testing/MockConnection.js';
9+
import * as UI from '../../ui/legacy/legacy.js';
10+
11+
describeWithMockConnection('DetachedElementsProfileType', () => {
12+
describe('buttonClicked', () => {
13+
let releaseAllAnimationsStub: sinon.SinonStub;
14+
let getDetachedDOMNodesStub: sinon.SinonStub;
15+
beforeEach(() => {
16+
const target = createTarget();
17+
const heapProfilerModel = target.model(SDK.HeapProfilerModel.HeapProfilerModel);
18+
UI.Context.Context.instance().setFlavor(SDK.HeapProfilerModel.HeapProfilerModel, heapProfilerModel);
19+
20+
releaseAllAnimationsStub =
21+
sinon.stub(SDK.AnimationModel.AnimationModel.prototype, 'releaseAllAnimations').resolves();
22+
getDetachedDOMNodesStub = sinon.stub(SDK.DOMModel.DOMModel.prototype, 'getDetachedDOMNodes').resolves([]);
23+
});
24+
25+
it('releases all animations before `getDetachedDOMNodes` call', async () => {
26+
// We need dynamic import here because statically importing the module requires locale vars to be initialized vars.
27+
const Profiler = await import('./profiler.js');
28+
Profiler.ProfileTypeRegistry.instance.detachedElementProfileType.buttonClicked();
29+
30+
assert.isTrue(releaseAllAnimationsStub.calledOnce, 'Expected release all animations to be called');
31+
await expectCall(getDetachedDOMNodesStub);
32+
});
33+
});
34+
});

front_end/panels/profiler/HeapDetachedElementsView.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,15 @@ export class DetachedElementsProfileType extends
118118
const heapProfilerModel = UI.Context.Context.instance().flavor(SDK.HeapProfilerModel.HeapProfilerModel);
119119
const target = heapProfilerModel?.target();
120120
const domModel = target?.model(SDK.DOMModel.DOMModel);
121-
122121
if (!heapProfilerModel || !target || !domModel) {
123122
return;
124123
}
124+
125+
const animationModel = target?.model(SDK.AnimationModel.AnimationModel);
126+
if (animationModel) {
127+
// TODO(b/406904348): Remove this once we correctly release animations on the backend.
128+
await animationModel.releaseAllAnimations();
129+
}
125130
const data = await domModel.getDetachedDOMNodes();
126131

127132
const profile: DetachedElementsProfileHeader = new DetachedElementsProfileHeader(heapProfilerModel, this, data);
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2025 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import * as SDK from '../../core/sdk/sdk.js';
6+
import {createTarget} from '../../testing/EnvironmentHelpers.js';
7+
import {expectCall} from '../../testing/ExpectStubCall.js';
8+
import {describeWithMockConnection} from '../../testing/MockConnection.js';
9+
import * as UI from '../../ui/legacy/legacy.js';
10+
11+
describeWithMockConnection('SamplingHeapProfileTypeBase', () => {
12+
describe('buttonClicked', () => {
13+
let releaseAllAnimationsStub: sinon.SinonStub;
14+
beforeEach(() => {
15+
const target = createTarget();
16+
const heapProfilerModel = target.model(SDK.HeapProfilerModel.HeapProfilerModel);
17+
UI.Context.Context.instance().setFlavor(SDK.HeapProfilerModel.HeapProfilerModel, heapProfilerModel);
18+
19+
releaseAllAnimationsStub =
20+
sinon.stub(SDK.AnimationModel.AnimationModel.prototype, 'releaseAllAnimations').resolves();
21+
});
22+
23+
it('releases all animations before `startSampling` call', async () => {
24+
// We need dynamic import here because statically importing the module requires locale vars to be initialized vars.
25+
const Profiler = await import('./profiler.js');
26+
const startSamplingStub =
27+
sinon.stub(Profiler.ProfileTypeRegistry.instance.samplingHeapProfileType, 'startSampling');
28+
Profiler.ProfileTypeRegistry.instance.samplingHeapProfileType.buttonClicked();
29+
30+
assert.isTrue(releaseAllAnimationsStub.calledOnce, 'Expected release all animations to be called');
31+
await expectCall(startSamplingStub);
32+
});
33+
});
34+
});

front_end/panels/profiler/HeapProfileView.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,12 @@ export class SamplingHeapProfileTypeBase extends
263263
if (this.recording) {
264264
void this.stopRecordingProfile();
265265
} else {
266-
this.startRecordingProfile();
266+
void this.startRecordingProfile();
267267
}
268268
return this.recording;
269269
}
270270

271-
startRecordingProfile(): void {
271+
async startRecordingProfile(): Promise<void> {
272272
const heapProfilerModel = UI.Context.Context.instance().flavor(SDK.HeapProfilerModel.HeapProfilerModel);
273273
if (this.profileBeingRecorded() || !heapProfilerModel) {
274274
return;
@@ -282,6 +282,12 @@ export class SamplingHeapProfileTypeBase extends
282282
UI.InspectorView.InspectorView.instance().setPanelWarnings('heap-profiler', warnings);
283283

284284
this.recording = true;
285+
const target = heapProfilerModel.target();
286+
const animationModel = target.model(SDK.AnimationModel.AnimationModel);
287+
if (animationModel) {
288+
// TODO(b/406904348): Remove this once we correctly release animations on the backend.
289+
await animationModel.releaseAllAnimations();
290+
}
285291
this.startSampling();
286292
}
287293

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2025 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import * as SDK from '../../core/sdk/sdk.js';
6+
import {createTarget} from '../../testing/EnvironmentHelpers.js';
7+
import {expectCall} from '../../testing/ExpectStubCall.js';
8+
import {describeWithMockConnection} from '../../testing/MockConnection.js';
9+
import * as UI from '../../ui/legacy/legacy.js';
10+
11+
describeWithMockConnection('TrackingHeapSnapshotProfileType', () => {
12+
describe('buttonClicked', () => {
13+
let releaseAllAnimationsStub: sinon.SinonStub;
14+
let startTrackingHeapObjectsStub: sinon.SinonStub;
15+
beforeEach(() => {
16+
const target = createTarget();
17+
const heapProfilerModel = target.model(SDK.HeapProfilerModel.HeapProfilerModel);
18+
UI.Context.Context.instance().setFlavor(SDK.HeapProfilerModel.HeapProfilerModel, heapProfilerModel);
19+
20+
releaseAllAnimationsStub =
21+
sinon.stub(SDK.AnimationModel.AnimationModel.prototype, 'releaseAllAnimations').resolves();
22+
startTrackingHeapObjectsStub =
23+
sinon.stub(SDK.HeapProfilerModel.HeapProfilerModel.prototype, 'startTrackingHeapObjects').resolves();
24+
});
25+
26+
it('releases all animations before `startTrackingHeapObjects` call', async () => {
27+
// We need dynamic import here because statically importing the module requires locale vars to be initialized vars.
28+
const Profiler = await import('./profiler.js');
29+
Profiler.ProfileTypeRegistry.instance.trackingHeapSnapshotProfileType.buttonClicked();
30+
31+
assert.isTrue(releaseAllAnimationsStub.calledOnce, 'Expected release all animations to be called');
32+
await expectCall(startTrackingHeapObjectsStub);
33+
});
34+
});
35+
});

front_end/panels/profiler/HeapSnapshotView.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,14 +1526,20 @@ export class TrackingHeapSnapshotProfileType extends
15261526
return this.toggleRecording();
15271527
}
15281528

1529-
startRecordingProfile(): void {
1529+
async startRecordingProfile(): Promise<void> {
15301530
if (this.profileBeingRecorded()) {
15311531
return;
15321532
}
15331533
const heapProfilerModel = this.addNewProfile();
15341534
if (!heapProfilerModel) {
15351535
return;
15361536
}
1537+
1538+
const animationModel = heapProfilerModel.target().model(SDK.AnimationModel.AnimationModel);
1539+
if (animationModel) {
1540+
// TODO(b/406904348): Remove this once we correctly release animations on the backend.
1541+
await animationModel.releaseAllAnimations();
1542+
}
15371543
void heapProfilerModel.startTrackingHeapObjects(this.recordAllocationStacksSettingInternal.get());
15381544
}
15391545

@@ -1593,7 +1599,7 @@ export class TrackingHeapSnapshotProfileType extends
15931599
if (this.recording) {
15941600
void this.stopRecordingProfile();
15951601
} else {
1596-
this.startRecordingProfile();
1602+
void this.startRecordingProfile();
15971603
}
15981604
return this.recording;
15991605
}

0 commit comments

Comments
 (0)