Skip to content

Commit 970e566

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: remove annotations experiment
Keeps the feature, but removes the experiment flag (which had been enabled by default for all users). Fixed: 377234926 Change-Id: Ib89e7d93b63f3841804aeb382a5af425358b512e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6055037 Reviewed-by: Nancy Li <[email protected]> Auto-Submit: Jack Franklin <[email protected]> Commit-Queue: Nancy Li <[email protected]>
1 parent dbb9e9b commit 970e566

File tree

12 files changed

+68
-154
lines changed

12 files changed

+68
-154
lines changed

front_end/core/host/UserMetrics.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1013,7 +1013,6 @@ export enum DevtoolsExperiments {
10131013
'timeline-enhanced-traces' = 90,
10141014
'timeline-compiled-sources' = 91,
10151015
'timeline-debug-mode' = 93,
1016-
'perf-panel-annotations' = 94,
10171016
'timeline-rpp-sidebar' = 95,
10181017
'timeline-observations' = 96,
10191018
'timeline-server-timings' = 98,

front_end/core/root/Runtime.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,6 @@ export const enum ExperimentName {
293293
NETWORK_PANEL_FILTER_BAR_REDESIGN = 'network-panel-filter-bar-redesign',
294294
AUTOFILL_VIEW = 'autofill-view',
295295
TIMELINE_SHOW_POST_MESSAGE_EVENTS = 'timeline-show-postmessage-events',
296-
TIMELINE_ANNOTATIONS = 'perf-panel-annotations',
297296
TIMELINE_INSIGHTS = 'timeline-rpp-sidebar',
298297
TIMELINE_DEBUG_MODE = 'timeline-debug-mode',
299298
TIMELINE_OBSERVATIONS = 'timeline-observations',

front_end/entrypoints/main/MainImpl.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -379,11 +379,6 @@ export class MainImpl {
379379
'Performance panel: show postMessage dispatch and handling flows',
380380
);
381381

382-
Root.Runtime.experiments.register(
383-
Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS,
384-
'Performance panel: enable annotations',
385-
);
386-
387382
Root.Runtime.experiments.register(
388383
Root.Runtime.ExperimentName.TIMELINE_INSIGHTS,
389384
'Performance panel: enable performance insights',
@@ -423,7 +418,6 @@ export class MainImpl {
423418
Root.Runtime.ExperimentName.AUTOFILL_VIEW,
424419
Root.Runtime.ExperimentName.TIMELINE_INSIGHTS,
425420
Root.Runtime.ExperimentName.TIMELINE_OBSERVATIONS,
426-
Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS,
427421
Root.Runtime.ExperimentName.NETWORK_PANEL_FILTER_BAR_REDESIGN,
428422
Root.Runtime.ExperimentName.FLOATING_ENTRY_POINTS_FOR_AI_ASSISTANCE,
429423
...(Root.Runtime.Runtime.queryParam('isChromeForTesting') ? ['protocol-monitor'] : []),

front_end/panels/timeline/TimelineFlameChartView.ts

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -312,23 +312,22 @@ export class TimelineFlameChartView extends
312312
this.detailsSplitWidget.setSidebarWidget(this.detailsView);
313313
this.detailsSplitWidget.show(this.element);
314314

315+
// Event listeners for annotations.
315316
this.onMainAddEntryLabelAnnotation = this.onAddEntryLabelAnnotation.bind(this, this.mainDataProvider);
316317
this.onNetworkAddEntryLabelAnnotation = this.onAddEntryLabelAnnotation.bind(this, this.networkDataProvider);
317318
this.#onMainEntriesLinkAnnotationCreated = event =>
318319
this.onEntriesLinkAnnotationCreate(this.mainDataProvider, event.data.entryFromIndex);
319320
this.#onNetworkEntriesLinkAnnotationCreated = event =>
320321
this.onEntriesLinkAnnotationCreate(this.networkDataProvider, event.data.entryFromIndex);
321-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS)) {
322-
this.mainFlameChart.addEventListener(
323-
PerfUI.FlameChart.Events.ENTRY_LABEL_ANNOTATION_ADDED, this.onMainAddEntryLabelAnnotation, this);
324-
this.networkFlameChart.addEventListener(
325-
PerfUI.FlameChart.Events.ENTRY_LABEL_ANNOTATION_ADDED, this.onNetworkAddEntryLabelAnnotation, this);
322+
this.mainFlameChart.addEventListener(
323+
PerfUI.FlameChart.Events.ENTRY_LABEL_ANNOTATION_ADDED, this.onMainAddEntryLabelAnnotation, this);
324+
this.networkFlameChart.addEventListener(
325+
PerfUI.FlameChart.Events.ENTRY_LABEL_ANNOTATION_ADDED, this.onNetworkAddEntryLabelAnnotation, this);
326326

327-
this.mainFlameChart.addEventListener(
328-
PerfUI.FlameChart.Events.ENTRIES_LINK_ANNOTATION_CREATED, this.#onMainEntriesLinkAnnotationCreated, this);
329-
this.networkFlameChart.addEventListener(
330-
PerfUI.FlameChart.Events.ENTRIES_LINK_ANNOTATION_CREATED, this.#onNetworkEntriesLinkAnnotationCreated, this);
331-
}
327+
this.mainFlameChart.addEventListener(
328+
PerfUI.FlameChart.Events.ENTRIES_LINK_ANNOTATION_CREATED, this.#onMainEntriesLinkAnnotationCreated, this);
329+
this.networkFlameChart.addEventListener(
330+
PerfUI.FlameChart.Events.ENTRIES_LINK_ANNOTATION_CREATED, this.#onNetworkEntriesLinkAnnotationCreated, this);
332331

333332
this.detailsView.addEventListener(TimelineTreeView.Events.TREE_ROW_HOVERED, node => {
334333
if (!Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_DIM_UNRELATED_EVENTS)) {
@@ -910,30 +909,30 @@ export class TimelineFlameChartView extends
910909
this.delegate.select(selectionFromRangeMilliSeconds(
911910
Trace.Types.Timing.MilliSeconds(startTime), Trace.Types.Timing.MilliSeconds(endTime)));
912911

913-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS)) {
914-
const bounds = Trace.Helpers.Timing.traceWindowFromMilliSeconds(
915-
Trace.Types.Timing.MilliSeconds(startTime),
916-
Trace.Types.Timing.MilliSeconds(endTime),
917-
);
912+
// We need to check if the user is updating the range because they are
913+
// creating a time range annotation.
914+
const bounds = Trace.Helpers.Timing.traceWindowFromMilliSeconds(
915+
Trace.Types.Timing.MilliSeconds(startTime),
916+
Trace.Types.Timing.MilliSeconds(endTime),
917+
);
918918

919-
// If the current time range annotation exists, the range selection
920-
// for it is in progress and we need to update its bounds.
921-
//
922-
// When the range selection is finished, the current range is set to null.
923-
// If the current selection is null, create a new time range annotations.
924-
if (this.#timeRangeSelectionAnnotation) {
925-
this.#timeRangeSelectionAnnotation.bounds = bounds;
926-
ModificationsManager.activeManager()?.updateAnnotation(this.#timeRangeSelectionAnnotation);
927-
} else {
928-
this.#timeRangeSelectionAnnotation = {
929-
type: 'TIME_RANGE',
930-
label: '',
931-
bounds,
932-
};
933-
// Before creating a new range, make sure to delete the empty ranges.
934-
ModificationsManager.activeManager()?.deleteEmptyRangeAnnotations();
935-
ModificationsManager.activeManager()?.createAnnotation(this.#timeRangeSelectionAnnotation);
936-
}
919+
// If the current time range annotation exists, the range selection
920+
// for it is in progress and we need to update its bounds.
921+
//
922+
// When the range selection is finished, the current range is set to null.
923+
// If the current selection is null, create a new time range annotations.
924+
if (this.#timeRangeSelectionAnnotation) {
925+
this.#timeRangeSelectionAnnotation.bounds = bounds;
926+
ModificationsManager.activeManager()?.updateAnnotation(this.#timeRangeSelectionAnnotation);
927+
} else {
928+
this.#timeRangeSelectionAnnotation = {
929+
type: 'TIME_RANGE',
930+
label: '',
931+
bounds,
932+
};
933+
// Before creating a new range, make sure to delete the empty ranges.
934+
ModificationsManager.activeManager()?.deleteEmptyRangeAnnotations();
935+
ModificationsManager.activeManager()?.createAnnotation(this.#timeRangeSelectionAnnotation);
937936
}
938937
}
939938

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
679679
}
680680

681681
#setActiveInsight(insight: TimelineComponents.Sidebar.ActiveInsight|null): void {
682-
if (insight && this.#panelSidebarEnabled()) {
682+
if (insight) {
683683
this.#splitWidget.showBoth();
684684
}
685685
this.#sideBar.setActiveInsight(insight);
@@ -974,20 +974,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
974974
return checkboxItem;
975975
}
976976

977-
/**
978-
* Users don't explicitly opt in to the sidebar, but if they opt into either
979-
* Insights or Annotations, we will show the sidebar.
980-
*/
981-
#panelSidebarEnabled(): boolean {
982-
const sidebarEnabled = Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS) ||
983-
Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS);
984-
return sidebarEnabled;
985-
}
986977
#addSidebarIconToToolbar(): void {
987-
if (!this.#panelSidebarEnabled()) {
988-
return;
989-
}
990-
991978
if (this.panelToolbar.hasItem(this.#sidebarToggleButton)) {
992979
return;
993980
}
@@ -1030,18 +1017,9 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
10301017
this.selectFileToLoad();
10311018
});
10321019

1033-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS)) {
1034-
this.saveButton = new UI.Toolbar.ToolbarMenuButton(
1035-
this.populateDownloadMenu.bind(this), true, true, 'timeline.save-to-file-more-options', 'download');
1036-
this.saveButton.setTitle(i18nString(UIStrings.saveProfile));
1037-
} else {
1038-
this.saveButton = new UI.Toolbar.ToolbarButton(
1039-
i18nString(UIStrings.saveProfile), 'download', undefined, 'timeline.save-to-file');
1040-
this.saveButton.addEventListener(UI.Toolbar.ToolbarButton.Events.CLICK, _event => {
1041-
Host.userMetrics.actionTaken(Host.UserMetrics.Action.PerfPanelTraceExported);
1042-
void this.saveToFile();
1043-
});
1044-
}
1020+
this.saveButton = new UI.Toolbar.ToolbarMenuButton(
1021+
this.populateDownloadMenu.bind(this), true, true, 'timeline.save-to-file-more-options', 'download');
1022+
this.saveButton.setTitle(i18nString(UIStrings.saveProfile));
10451023

10461024
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ENHANCED_TRACES)) {
10471025
this.saveButton.element.addEventListener('contextmenu', event => {
@@ -1278,9 +1256,8 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
12781256
}
12791257
const traceEvents = this.#traceEngineModel.rawTraceEvents(this.#viewMode.traceIndex);
12801258
const metadata = this.#traceEngineModel.metadata(this.#viewMode.traceIndex);
1281-
// Save modifications into the metadata if modifications experiment is on
1282-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS) && metadata &&
1283-
addModifications) {
1259+
1260+
if (metadata && addModifications) {
12841261
metadata.modifications = ModificationsManager.activeManager()?.toJSON();
12851262
} else if (metadata) {
12861263
delete metadata.modifications;
@@ -1990,7 +1967,6 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
19901967
* We automatically show the sidebar in only 2 scenarios:
19911968
* 1. The user has never seen it before, so we show it once to aid discovery
19921969
* 2. The user had it open, and we hid it (for example, during recording), so now we need to bring it back.
1993-
* We also check that the experiments are enabled, else we reveal an entirely empty sidebar.
19941970
*/
19951971
#showSidebarIfRequired(): void {
19961972
if (Root.Runtime.Runtime.queryParam('disable-auto-performance-sidebar-reveal') !== null) {
@@ -1999,9 +1975,8 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
19991975
}
20001976
const needToRestore = this.#restoreSidebarVisibilityOnTraceLoad;
20011977
const userHasSeenSidebar = this.#sideBar.userHasOpenedSidebarOnce();
2002-
const experimentsEnabled = this.#panelSidebarEnabled();
20031978

2004-
if (experimentsEnabled && (!userHasSeenSidebar || needToRestore)) {
1979+
if (!userHasSeenSidebar || needToRestore) {
20051980
this.#splitWidget.showBoth();
20061981
}
20071982
this.#restoreSidebarVisibilityOnTraceLoad = false;

front_end/panels/timeline/components/Sidebar.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ describeWithEnvironment('Sidebar', () => {
2828

2929
beforeEach(() => {
3030
enableFeatureForTest(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS);
31-
enableFeatureForTest(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS);
3231
});
3332

3433
it('renders with two tabs for insights & annotations', async function() {

front_end/panels/timeline/components/Sidebar.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,9 @@ export class SidebarWidget extends UI.Widget.VBox {
7777
SidebarTabs.INSIGHTS, 'Insights', this.#insightsView, undefined, undefined, false, false, 0,
7878
'timeline.insights-tab');
7979
}
80-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS)) {
81-
this.#tabbedPane.appendTab(
82-
SidebarTabs.ANNOTATIONS, 'Annotations', this.#annotationsView, undefined, undefined, false, false, 1,
83-
'timeline.annotations-tab');
84-
}
80+
this.#tabbedPane.appendTab(
81+
SidebarTabs.ANNOTATIONS, 'Annotations', this.#annotationsView, undefined, undefined, false, false, 1,
82+
'timeline.annotations-tab');
8583

8684
// Default the selected tab to Insights. In wasShown() we will change this
8785
// if this is a trace that has no insights.

front_end/testing/EnvironmentHelpers.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ const REGISTERED_EXPERIMENTS = [
123123
'font-editor',
124124
Root.Runtime.ExperimentName.NETWORK_PANEL_FILTER_BAR_REDESIGN,
125125
Root.Runtime.ExperimentName.AUTOFILL_VIEW,
126-
Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS,
127126
Root.Runtime.ExperimentName.TIMELINE_INSIGHTS,
128127
Root.Runtime.ExperimentName.TIMELINE_DEBUG_MODE,
129128
Root.Runtime.ExperimentName.TIMELINE_OBSERVATIONS,

front_end/ui/components/docs/performance_panel/basic.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,8 @@ const traceUrl = params.get('loadTimelineFromURL');
102102
const nodeMode = params.get('isNode');
103103
const isNodeMode = nodeMode === 'true' ? true : false;
104104

105-
// These are both enabled by default in Chrome M131 and will be removed in M132.
105+
// This experiment is enabled by default in Chrome M131.
106106
Root.Runtime.experiments.setEnabled(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS, true);
107-
Root.Runtime.experiments.setEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS, true);
108107

109108
const timeline = Timeline.TimelinePanel.TimelinePanel.instance({forceNew: true, isNode: isNodeMode});
110109
const container = document.getElementById('container');

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

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
// found in the LICENSE file.
44

55
import * as Common from '../../../../core/common/common.js';
6-
import * as i18n from '../../../../core/i18n/i18n.js';
76
import * as Platform from '../../../../core/platform/platform.js';
8-
import * as Root from '../../../../core/root/root.js';
97
import * as Coordinator from '../../../components/render_coordinator/render_coordinator.js';
108
import * as UI from '../../legacy.js';
119

@@ -33,7 +31,6 @@ export class ChartViewport extends UI.Widget.VBox {
3331
private vScrollElement: HTMLElement;
3432
private vScrollContent: HTMLElement;
3533
private readonly selectionOverlay: HTMLElement;
36-
private selectedTimeSpanLabel: HTMLElement;
3734
private cursorElement: HTMLElement;
3835
private isDraggingInternal!: boolean;
3936
private totalHeight!: number;
@@ -59,8 +56,6 @@ export class ChartViewport extends UI.Widget.VBox {
5956

6057
#config: Config;
6158

62-
#usingNewOverlayForTimeRange = Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ANNOTATIONS);
63-
6459
constructor(delegate: ChartViewportDelegate, config: Config) {
6560
super();
6661
this.#config = config;
@@ -89,17 +84,11 @@ export class ChartViewport extends UI.Widget.VBox {
8984
this.vScrollElement.addEventListener('scroll', this.onScroll.bind(this), false);
9085

9186
this.selectionOverlay = this.contentElement.createChild('div', 'chart-viewport-selection-overlay hidden');
92-
this.selectedTimeSpanLabel = this.selectionOverlay.createChild('div', 'time-span');
9387

9488
this.cursorElement = this.contentElement.createChild('div', 'chart-cursor-element hidden');
95-
if (this.#usingNewOverlayForTimeRange) {
96-
this.cursorElement.classList.add('using-new-overlays');
97-
}
9889

9990
this.reset();
100-
10191
this.rangeSelectionStart = null;
102-
10392
this.rangeSelectionEnd = null;
10493
}
10594

@@ -112,7 +101,6 @@ export class ChartViewport extends UI.Widget.VBox {
112101
this.rangeSelectionEnabled = false;
113102
this.rangeSelectionStart = null;
114103
this.rangeSelectionEnd = null;
115-
this.updateRangeSelectionOverlay();
116104
}
117105

118106
isDragging(): boolean {
@@ -268,13 +256,6 @@ export class ChartViewport extends UI.Widget.VBox {
268256
this.isDraggingInternal = true;
269257
this.selectionOffsetShiftX = event.offsetX - event.pageX;
270258
this.selectionStartX = event.offsetX;
271-
if (!this.#usingNewOverlayForTimeRange) {
272-
const style = this.selectionOverlay.style;
273-
style.left = this.selectionStartX + 'px';
274-
style.width = '1px';
275-
this.selectedTimeSpanLabel.textContent = '';
276-
this.selectionOverlay.classList.remove('hidden');
277-
}
278259
return true;
279260
}
280261

@@ -300,7 +281,6 @@ export class ChartViewport extends UI.Widget.VBox {
300281
}
301282
this.rangeSelectionStart = Math.min(startTime, endTime);
302283
this.rangeSelectionEnd = Math.max(startTime, endTime);
303-
this.updateRangeSelectionOverlay();
304284
this.delegate.updateRangeSelection(this.rangeSelectionStart, this.rangeSelectionEnd);
305285
}
306286

@@ -321,25 +301,6 @@ export class ChartViewport extends UI.Widget.VBox {
321301
this.setRangeSelection(start, end);
322302
}
323303

324-
private updateRangeSelectionOverlay(): void {
325-
if (this.#usingNewOverlayForTimeRange) {
326-
return;
327-
}
328-
329-
const rangeSelectionStart = this.rangeSelectionStart || 0;
330-
const rangeSelectionEnd = this.rangeSelectionEnd || 0;
331-
const margin = 100;
332-
const left =
333-
Platform.NumberUtilities.clamp(this.timeToPosition(rangeSelectionStart), -margin, this.offsetWidth + margin);
334-
const right =
335-
Platform.NumberUtilities.clamp(this.timeToPosition(rangeSelectionEnd), -margin, this.offsetWidth + margin);
336-
const style = this.selectionOverlay.style;
337-
style.left = left + 'px';
338-
style.width = (right - left) + 'px';
339-
const timeSpan = rangeSelectionEnd - rangeSelectionStart;
340-
this.selectedTimeSpanLabel.textContent = i18n.TimeUtilities.preciseMillisToString(timeSpan, 2);
341-
}
342-
343304
private onScroll(): void {
344305
this.scrollTop = this.vScrollElement.scrollTop;
345306
this.scheduleUpdate();
@@ -472,7 +433,6 @@ export class ChartViewport extends UI.Widget.VBox {
472433
}
473434

474435
override update(): void {
475-
this.updateRangeSelectionOverlay();
476436
this.delegate.update();
477437
}
478438

0 commit comments

Comments
 (0)