Skip to content

Commit 67670dd

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
Revert "RPP: remove "Disable JS Samples" checkbox"
This reverts commit 682af1f. Reason for revert: we got feedback that people will often rely on this functionality Original change's description: > RPP: remove "Disable JS Samples" checkbox > > It's rarely useful to disable samples, and from our metrics we know > barely anyone ever does. > > [email protected] > > Fixed: 414734883 > Change-Id: Ib328428e50f8f1571864e5f92a4ec69e252724d3 > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6494564 > Reviewed-by: Paul Irish <[email protected]> > Commit-Queue: Jack Franklin <[email protected]> > Auto-Submit: Jack Franklin <[email protected]> > Commit-Queue: Paul Irish <[email protected]> Change-Id: I8b3296b406631e5fe7bee915b28906e4e4c9e518 Bug: 436491188, 414734883 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6850129 Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Connor Clark <[email protected]>
1 parent ab79488 commit 67670dd

File tree

2 files changed

+31
-6
lines changed

2 files changed

+31
-6
lines changed

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@ const UIStrings = {
9696
*/
9797
dropTimelineFileOrUrlHere: 'Drop trace file or URL here',
9898
/**
99-
* @description Title of capture layers and pictures setting in timeline panel of the performance panel
99+
* @description Title of disable capture jsprofile setting in timeline panel of the performance panel
100+
*/
101+
disableJavascriptSamples: 'Disable JavaScript samples',
102+
/**
103+
*@description Title of capture layers and pictures setting in timeline panel of the performance panel
100104
*/
101105
enableAdvancedPaint: 'Enable advanced paint instrumentation (slow)',
102106
/**
@@ -138,6 +142,10 @@ const UIStrings = {
138142
/**
139143
* @description Text in Timeline Panel of the Performance panel
140144
*/
145+
disablesJavascriptSampling: 'Disables JavaScript sampling, reduces overhead when running against mobile devices',
146+
/**
147+
*@description Text in Timeline Panel of the Performance panel
148+
*/
141149
capturesAdvancedPaint: 'Captures advanced paint instrumentation, introduces significant performance overhead',
142150
/**
143151
* @description Text in Timeline Panel of the Performance panel
@@ -174,6 +182,10 @@ const UIStrings = {
174182
/**
175183
* @description Text in Timeline Panel of the Performance panel
176184
*/
185+
JavascriptSamplingIsDisabled: '- JavaScript sampling is disabled',
186+
/**
187+
*@description Text in Timeline Panel of the Performance panel
188+
*/
177189
stoppingTimeline: 'Stopping timeline…',
178190
/**
179191
* @description Text in Timeline Panel of the Performance panel
@@ -322,6 +334,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
322334
private readonly toggleRecordAction: UI.ActionRegistration.Action;
323335
private readonly recordReloadAction: UI.ActionRegistration.Action;
324336
readonly #historyManager: TimelineHistoryManager;
337+
private disableCaptureJSProfileSetting: Common.Settings.Setting<boolean>;
325338
private readonly captureLayersAndPicturesSetting: Common.Settings.Setting<boolean>;
326339
private readonly captureSelectorStatsSetting: Common.Settings.Setting<boolean>;
327340
readonly #thirdPartyTracksSetting: Common.Settings.Setting<boolean>;
@@ -453,6 +466,9 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
453466

454467
this.traceLoadStart = null;
455468

469+
this.disableCaptureJSProfileSetting = Common.Settings.Settings.instance().createSetting(
470+
'timeline-disable-js-sampling', false, Common.Settings.SettingStorageType.SESSION);
471+
this.disableCaptureJSProfileSetting.setTitle(i18nString(UIStrings.disableJavascriptSamples));
456472
this.captureLayersAndPicturesSetting = Common.Settings.Settings.instance().createSetting(
457473
'timeline-capture-layers-and-pictures', false, Common.Settings.SettingStorageType.SESSION);
458474
this.captureLayersAndPicturesSetting.setTitle(i18nString(UIStrings.enableAdvancedPaint));
@@ -1217,6 +1233,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
12171233
this);
12181234
SDK.CPUThrottlingManager.CPUThrottlingManager.instance().addEventListener(
12191235
SDK.CPUThrottlingManager.Events.RATE_CHANGED, this.updateShowSettingsToolbarButton, this);
1236+
this.disableCaptureJSProfileSetting.addChangeListener(this.updateShowSettingsToolbarButton, this);
12201237
this.captureLayersAndPicturesSetting.addChangeListener(this.updateShowSettingsToolbarButton, this);
12211238
this.captureSelectorStatsSetting.addChangeListener(this.updateShowSettingsToolbarButton, this);
12221239

@@ -1229,16 +1246,20 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
12291246
cpuThrottlingPane.append(this.cpuThrottlingSelect.control.element);
12301247

12311248
this.settingsPane.append(UI.SettingsUI.createSettingCheckbox(
1232-
this.captureLayersAndPicturesSetting.title(), this.captureLayersAndPicturesSetting,
1233-
i18nString(UIStrings.capturesAdvancedPaint)));
1249+
this.captureSelectorStatsSetting.title(), this.captureSelectorStatsSetting,
1250+
i18nString(UIStrings.capturesSelectorStats)));
12341251

12351252
const networkThrottlingPane = this.settingsPane.createChild('div');
12361253
networkThrottlingPane.append(i18nString(UIStrings.network));
12371254
networkThrottlingPane.append(this.createNetworkConditionsSelectToolbarItem().element);
12381255

12391256
this.settingsPane.append(UI.SettingsUI.createSettingCheckbox(
1240-
this.captureSelectorStatsSetting.title(), this.captureSelectorStatsSetting,
1241-
i18nString(UIStrings.capturesSelectorStats)));
1257+
this.captureLayersAndPicturesSetting.title(), this.captureLayersAndPicturesSetting,
1258+
i18nString(UIStrings.capturesAdvancedPaint)));
1259+
1260+
this.settingsPane.append(UI.SettingsUI.createSettingCheckbox(
1261+
this.disableCaptureJSProfileSetting.title(), this.disableCaptureJSProfileSetting,
1262+
i18nString(UIStrings.disablesJavascriptSampling)));
12421263

12431264
const thirdPartyCheckbox =
12441265
this.createSettingCheckbox(this.#thirdPartyTracksSetting, i18nString(UIStrings.showDataAddedByExtensions));
@@ -1627,6 +1648,9 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
16271648
if (this.captureSelectorStatsSetting.get()) {
16281649
messages.push(i18nString(UIStrings.SelectorStatsEnabled));
16291650
}
1651+
if (this.disableCaptureJSProfileSetting.get()) {
1652+
messages.push(i18nString(UIStrings.JavascriptSamplingIsDisabled));
1653+
}
16301654

16311655
this.showSettingsPaneButton.setChecked(messages.length > 0);
16321656
this.showSettingsPaneButton.element.style.setProperty('--dot-toggle-top', '16px');
@@ -1769,7 +1793,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
17691793
await this.#navigateToAboutBlank();
17701794
}
17711795
const recordingOptions = {
1772-
enableJSSampling: true,
1796+
enableJSSampling: !this.disableCaptureJSProfileSetting.get(),
17731797
capturePictures: this.captureLayersAndPicturesSetting.get(),
17741798
captureFilmStrip: this.showScreenshotsSetting.get(),
17751799
captureSelectorStats: this.captureSelectorStatsSetting.get(),

test/e2e/helpers/performance-helpers.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export async function openCaptureSettings(
7575
[
7676
veImpression('Toggle', 'timeline-capture-layers-and-pictures'),
7777
veImpression('Toggle', 'timeline-capture-selector-stats'),
78+
veImpression('Toggle', 'timeline-disable-js-sampling'),
7879
veImpression('DropDown', 'cpu-throttling'),
7980
veImpression('DropDown', 'active-network-condition-key'),
8081
veImpression('Toggle', 'timeline-show-extension-data'),

0 commit comments

Comments
 (0)