Skip to content

Commit c1861e8

Browse files
and-oliDevtools-frontend LUCI CQ
authored andcommitted
[Perf] Move 3p dimming handling into TimelineFlameChartView
And call it when building the flamechart data from a trace. This way we ensure we keep dimmed 3p entries when toggling the custom tracks setting. Fixed: 394123714 Change-Id: I04a378f4f30b8247cba585d5e4ffe7c31dbaef52 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6235029 Reviewed-by: Jack Franklin <[email protected]> Commit-Queue: Andres Olivares <[email protected]>
1 parent 2cbf243 commit c1861e8

File tree

4 files changed

+49
-29
lines changed

4 files changed

+49
-29
lines changed

front_end/panels/timeline/TimelineFlameChartView.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,18 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
437437
return this.element;
438438
}
439439

440-
setActiveThirdPartyDimmingSetting(thirdPartyEvents: Trace.Types.Events.Event[]|null): void {
441-
this.#updateFlameChartDimmerWithEvents(this.#thirdPartyCheckboxDimmer, thirdPartyEvents);
440+
// Activates or disables dimming when setting is toggled.
441+
dimThirdPartiesIfRequired(): void {
442+
if (!this.#parsedTrace) {
443+
return;
444+
}
445+
const dim = Common.Settings.Settings.instance().createSetting('timeline-dim-third-parties', false).get();
446+
const thirdPartyEvents = this.#entityMapper?.thirdPartyEvents() ?? [];
447+
if (dim && thirdPartyEvents.length) {
448+
this.#updateFlameChartDimmerWithEvents(this.#thirdPartyCheckboxDimmer, thirdPartyEvents);
449+
} else {
450+
this.#updateFlameChartDimmerWithEvents(this.#thirdPartyCheckboxDimmer, null);
451+
}
442452
}
443453

444454
#registerFlameChartDimmer(opts: {inclusive: boolean, outline: boolean}): FlameChartDimmer {
@@ -1160,6 +1170,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
11601170
this.#updateFlameCharts();
11611171
this.resizeToPreferredHeights();
11621172
this.setMarkers(this.#parsedTrace);
1173+
this.dimThirdPartiesIfRequired();
11631174
}
11641175

11651176
setInsights(

front_end/panels/timeline/TimelinePanel.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import * as Common from '../../core/common/common.js';
6+
import * as Root from '../../core/root/root.js';
57
import * as SDK from '../../core/sdk/sdk.js';
68
import * as Bindings from '../../models/bindings/bindings.js';
79
import * as Trace from '../../models/trace/trace.js';
@@ -25,6 +27,9 @@ describeWithEnvironment('TimelinePanel', function() {
2527
SDK.TargetManager.TargetManager.instance(),
2628
Workspace.Workspace.WorkspaceImpl.instance(),
2729
);
30+
Root.Runtime.experiments.enableForTest(
31+
Root.Runtime.ExperimentName.TIMELINE_THIRD_PARTY_DEPENDENCIES,
32+
);
2833
Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance({
2934
forceNew: true,
3035
resourceMapping,
@@ -118,4 +123,22 @@ describeWithEnvironment('TimelinePanel', function() {
118123
const overlaysAfterEnablingSetting = timeline.getFlameChart().overlays().allOverlays();
119124
assert.deepEqual(overlaysBeforeDisablingSetting, overlaysAfterEnablingSetting);
120125
});
126+
127+
it('keeps entries set to be dimmed as so after toggling the custom tracks setting', async function() {
128+
const events = await TraceLoader.rawEvents(this, 'web-dev.json.gz') as Trace.Types.Events.Event[];
129+
await timeline.loadingComplete(events, null, null);
130+
const thirdPartyDimSetting = Common.Settings.Settings.instance().createSetting('timeline-dim-third-parties', false);
131+
132+
// Dim 3P entries.
133+
thirdPartyDimSetting.set(true);
134+
const dimIndicesBeforeToggle = timeline.getFlameChart().getMainFlameChart().getDimIndices();
135+
assert.exists(dimIndicesBeforeToggle);
136+
assert.isAbove(dimIndicesBeforeToggle.length, 0);
137+
138+
// Toggle the custom track setting and verify 3P entries remain dimmed.
139+
Timeline.TimelinePanel.TimelinePanel.extensionDataVisibilitySetting().set(true);
140+
const dimIndicesAfterToggle = timeline.getFlameChart().getMainFlameChart().getDimIndices();
141+
assert.exists(dimIndicesAfterToggle);
142+
assert.isAbove(dimIndicesAfterToggle.length, 0);
143+
});
121144
});

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
934934
this.#setModelForActiveTrace();
935935
this.#removeStatusPane();
936936
this.#showSidebarIfRequired();
937-
this.#dimThirdPartiesIfRequired(newMode.traceIndex);
937+
this.flameChart.dimThirdPartiesIfRequired();
938938
return;
939939
}
940940

@@ -1550,7 +1550,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
15501550
if (this.#viewMode.mode !== 'VIEWING_TRACE') {
15511551
return;
15521552
}
1553-
this.#dimThirdPartiesIfRequired(this.#viewMode.traceIndex);
1553+
this.flameChart.dimThirdPartiesIfRequired();
15541554
}
15551555

15561556
#extensionDataVisibilityChanged(): void {
@@ -2156,23 +2156,6 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
21562156
this.#restoreSidebarVisibilityOnTraceLoad = false;
21572157
}
21582158

2159-
// Activates or disables dimming when checkbox is clicked.
2160-
#dimThirdPartiesIfRequired(traceIndex: number): void {
2161-
const parsedTrace = this.#traceEngineModel.parsedTrace(traceIndex);
2162-
if (!parsedTrace) {
2163-
return;
2164-
}
2165-
2166-
const checkboxState = this.#dimThirdPartiesSetting?.getIfNotDisabled() ?? false;
2167-
const thirdPartyEvents = this.#entityMapper?.thirdPartyEvents() ?? [];
2168-
2169-
if (checkboxState && thirdPartyEvents.length) {
2170-
this.flameChart.setActiveThirdPartyDimmingSetting(thirdPartyEvents);
2171-
} else {
2172-
this.flameChart.setActiveThirdPartyDimmingSetting(null);
2173-
}
2174-
}
2175-
21762159
// Build a map mapping annotated entries to the colours that are used to display them in the FlameChart.
21772160
// We need this map to display the entries in the sidebar with the same colours.
21782161
private buildColorsAnnotationsMap(annotations: Trace.Types.File.Annotation[]): Map<Trace.Types.Events.Event, string> {

front_end/ui/legacy/components/perf_ui/FlameChart.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
315315
private totalTime?: number;
316316
private lastPopoverState: PopoverState;
317317

318-
private dimIndicies?: Uint8Array|null;
318+
private dimIndices?: Uint8Array|null;
319319
/** When true, all undimmed entries are outlined. When an array, only those indices are outlined (if not dimmed). */
320320
private dimShouldOutlineUndimmedEntries: boolean|Uint8Array = false;
321321

@@ -496,8 +496,8 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
496496
}
497497

498498
#shouldDimEvent(entryIndex: number): boolean {
499-
if (this.dimIndicies) {
500-
return this.dimIndicies[entryIndex] !== 0;
499+
if (this.dimIndices) {
500+
return this.dimIndices[entryIndex] !== 0;
501501
}
502502

503503
return false;
@@ -540,22 +540,22 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
540540
}
541541

542542
enableDimming(entryIndices: number[], inclusive: boolean, outline: boolean|number[]): void {
543-
this.dimIndicies = this.#createTypedIndexArray(entryIndices, inclusive);
543+
this.dimIndices = this.#createTypedIndexArray(entryIndices, inclusive);
544544
this.dimShouldOutlineUndimmedEntries =
545545
Array.isArray(outline) ? this.#createTypedIndexArray(outline, true) : outline;
546546

547547
this.draw();
548548
}
549549

550550
disableDimming(): void {
551-
this.dimIndicies = null;
551+
this.dimIndices = null;
552552
this.dimShouldOutlineUndimmedEntries = false;
553553

554554
this.draw();
555555
}
556556

557557
isDimming(): boolean {
558-
return Boolean(this.dimIndicies);
558+
return Boolean(this.dimIndices);
559559
}
560560

561561
#transformColor(entryIndex: number, color: string): string {
@@ -3364,7 +3364,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
33643364
this.rawTimelineData = null;
33653365
this.forceDecorationCache = null;
33663366
this.entryColorsCache = null;
3367-
this.dimIndicies = null;
3367+
this.dimIndices = null;
33683368
this.colorDimmingCache.clear();
33693369
this.rawTimelineDataLength = 0;
33703370
this.#groupTreeRoot = null;
@@ -3976,7 +3976,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
39763976
this.rawTimelineData = null;
39773977
this.rawTimelineDataLength = 0;
39783978
this.#groupTreeRoot = null;
3979-
this.dimIndicies = null;
3979+
this.dimIndices = null;
39803980
this.colorDimmingCache.clear();
39813981
this.highlightedMarkerIndex = -1;
39823982
this.highlightedEntryIndex = -1;
@@ -4015,6 +4015,9 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
40154015
boundarySpan(): Trace.Types.Timing.Milli {
40164016
return Trace.Types.Timing.Milli(this.maximumBoundary() - this.minimumBoundary());
40174017
}
4018+
getDimIndices(): Uint8Array<ArrayBufferLike>|null {
4019+
return this.dimIndices || null;
4020+
}
40184021
}
40194022

40204023
export const RulerHeight = 15;

0 commit comments

Comments
 (0)