Skip to content

Commit fd8de19

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Recommened which throttling settings to use
* Network: recommened the closest preset based on CruX data. If no feild data, recommened nothing * CPU: always recommend 4x (for now) The Network portion currently is not implemented for the toolbar dropdown, as it requires some refactor to how CruX data is managed to query it outside LiveMetricsView. Bug: 311438203 Change-Id: I027de20bbda94c84dcd9084a26a1d9e724f17705 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6039744 Reviewed-by: Adam Raine <[email protected]> Commit-Queue: Connor Clark <[email protected]>
1 parent c6e874e commit fd8de19

File tree

9 files changed

+150
-25
lines changed

9 files changed

+150
-25
lines changed

front_end/panels/mobile_throttling/ThrottlingManager.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ const UIStrings = {
8282
*@description Tooltip text for an input box that overrides navigator.hardwareConcurrency on the page
8383
*/
8484
hardwareConcurrencySettingLabel: 'Override the value reported by navigator.hardwareConcurrency',
85+
/**
86+
* @description Text label for a selection box showing that a specific option is recommended.
87+
* @example {Fast 4G} PH1
88+
* @example {4x slowdown} PH1
89+
*/
90+
recommendedThrottling: '{PH1} - recommended',
8591
};
8692
const str_ = i18n.i18n.registerUIStrings('panels/mobile_throttling/ThrottlingManager.ts', UIStrings);
8793
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
@@ -135,7 +141,9 @@ export class ThrottlingManager {
135141
return throttlingManagerInstance;
136142
}
137143

138-
decorateSelectWithNetworkThrottling(selectElement: HTMLSelectElement): NetworkThrottlingSelector {
144+
decorateSelectWithNetworkThrottling(
145+
selectElement: HTMLSelectElement,
146+
recommendedConditions: SDK.NetworkManager.Conditions|null = null): NetworkThrottlingSelector {
139147
let options: (SDK.NetworkManager.Conditions|null)[] = [];
140148
const selector = new NetworkThrottlingSelector(populate, select, this.customNetworkConditionsSetting);
141149
selectElement.setAttribute(
@@ -156,7 +164,11 @@ export class ThrottlingManager {
156164
groupElement.label = group.title;
157165
for (const conditions of group.items) {
158166
// The title is usually an i18nLazyString except for custom values that are stored in the local storage in the form of a string.
159-
const title = typeof conditions.title === 'function' ? conditions.title() : conditions.title;
167+
let title = typeof conditions.title === 'function' ? conditions.title() : conditions.title;
168+
if (conditions === recommendedConditions) {
169+
title = i18nString(UIStrings.recommendedThrottling, {PH1: title});
170+
}
171+
160172
const option = new Option(title, title);
161173
UI.ARIAUtils.setLabel(option, i18nString(UIStrings.sS, {PH1: group.title, PH2: title}));
162174
const jslogContext = i === groups.length - 1 ?
@@ -296,7 +308,7 @@ export class ThrottlingManager {
296308
this.updatePanelIcon();
297309
}
298310

299-
createCPUThrottlingSelector(): UI.Toolbar.ToolbarComboBox {
311+
createCPUThrottlingSelector(recommendedRate: number|null = null): UI.Toolbar.ToolbarComboBox {
300312
const control = new UI.Toolbar.ToolbarComboBox(
301313
event => this.setCPUThrottlingRate(this.cpuThrottlingRates[(event.target as HTMLSelectElement).selectedIndex]),
302314
i18nString(UIStrings.cpuThrottling), '', 'cpu-throttling');
@@ -305,7 +317,11 @@ export class ThrottlingManager {
305317

306318
for (let i = 0; i < this.cpuThrottlingRates.length; ++i) {
307319
const rate = this.cpuThrottlingRates[i];
308-
const title = rate === 1 ? i18nString(UIStrings.noThrottling) : i18nString(UIStrings.dSlowdown, {PH1: rate});
320+
let title = rate === 1 ? i18nString(UIStrings.noThrottling) : i18nString(UIStrings.dSlowdown, {PH1: rate});
321+
if (rate === recommendedRate) {
322+
title = i18nString(UIStrings.recommendedThrottling, {PH1: title});
323+
}
324+
309325
const value = rate === 1 ? 'cpu-no-throttling' : `cpu-throttled-${rate}`;
310326
const option = control.createOption(title, value);
311327
control.addOption(option);

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,10 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
11951195

11961196
const cpuThrottlingToolbar = new UI.Toolbar.Toolbar('', throttlingPane.element);
11971197
cpuThrottlingToolbar.appendText(i18nString(UIStrings.cpu));
1198-
this.cpuThrottlingSelect = MobileThrottling.ThrottlingManager.throttlingManager().createCPUThrottlingSelector();
1198+
// TODO(crbug.com/311438203): need to get recommended like we do in LiveMetricsView
1199+
const recommendedRate = 4;
1200+
this.cpuThrottlingSelect =
1201+
MobileThrottling.ThrottlingManager.throttlingManager().createCPUThrottlingSelector(recommendedRate);
11991202
cpuThrottlingToolbar.appendToolbarItem(this.cpuThrottlingSelect);
12001203

12011204
const networkThrottlingToolbar = new UI.Toolbar.Toolbar('', throttlingPane.element);
@@ -1221,8 +1224,10 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
12211224
private createNetworkConditionsSelect(): UI.Toolbar.ToolbarComboBox {
12221225
const toolbarItem = new UI.Toolbar.ToolbarComboBox(null, i18nString(UIStrings.networkConditions));
12231226
toolbarItem.setMaxWidth(140);
1227+
// TODO(crbug.com/311438203): need to get recommended like we do in LiveMetricsView
1228+
const recommendedConditions = null;
12241229
MobileThrottling.ThrottlingManager.throttlingManager().decorateSelectWithNetworkThrottling(
1225-
toolbarItem.selectElement());
1230+
toolbarItem.selectElement(), recommendedConditions);
12261231
return toolbarItem;
12271232
}
12281233

front_end/panels/timeline/components/CPUThrottlingSelector.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import '../../../ui/components/menus/menus.js';
66

77
import * as i18n from '../../../core/i18n/i18n.js';
88
import * as SDK from '../../../core/sdk/sdk.js';
9+
import * as Buttons from '../../../ui/components/buttons/buttons.js';
910
import * as ComponentHelpers from '../../../ui/components/helpers/helpers.js';
1011
import type * as Menus from '../../../ui/components/menus/menus.js';
1112
import * as LitHtml from '../../../ui/lit-html/lit-html.js';
@@ -36,6 +37,15 @@ const UIStrings = {
3637
* @example {2} PH1
3738
*/
3839
dSlowdown: '{PH1}× slowdown',
40+
/**
41+
* @description Text label for a selection box showing that a specific option is recommended.
42+
* @example {4x slowdown} PH1
43+
*/
44+
recommendedThrottling: '{PH1} - recommended',
45+
/**
46+
* @description Text for why user should change a throttling setting.
47+
*/
48+
recommendedThrottlingReason: 'Consider changing setting to simulate real user environments',
3949
};
4050

4151
const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/CPUThrottlingSelector.ts', UIStrings);
@@ -45,13 +55,19 @@ export class CPUThrottlingSelector extends HTMLElement {
4555
readonly #shadow = this.attachShadow({mode: 'open'});
4656

4757
#currentRate: number;
58+
#recommendedRate: number|null = null;
4859

4960
constructor() {
5061
super();
5162
this.#currentRate = SDK.CPUThrottlingManager.CPUThrottlingManager.instance().cpuThrottlingRate();
5263
this.#render();
5364
}
5465

66+
set recommendedRate(recommendedRate: number|null) {
67+
this.#recommendedRate = recommendedRate;
68+
void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
69+
}
70+
5571
connectedCallback(): void {
5672
this.#shadow.adoptedStyleSheets = [cpuThrottlingSelectorStyles];
5773
SDK.CPUThrottlingManager.CPUThrottlingManager.instance().addEventListener(
@@ -75,6 +91,15 @@ export class CPUThrottlingSelector extends HTMLElement {
7591
}
7692

7793
#render = (): void => {
94+
let recommendedInfoEl;
95+
if (this.#recommendedRate && this.#currentRate === 1) {
96+
recommendedInfoEl = html`<devtools-button
97+
title=${i18nString(UIStrings.recommendedThrottlingReason)}
98+
.iconName=${'info'}
99+
.variant=${Buttons.Button.Variant.ICON}
100+
></devtools-button>`;
101+
}
102+
78103
const selectionTitle = this.#currentRate === 1 ? i18nString(UIStrings.noThrottling) :
79104
i18nString(UIStrings.dSlowdown, {PH1: this.#currentRate});
80105

@@ -92,7 +117,11 @@ export class CPUThrottlingSelector extends HTMLElement {
92117
title=${i18nString(UIStrings.cpuThrottling, {PH1: selectionTitle})}
93118
>
94119
${MobileThrottling.ThrottlingPresets.ThrottlingPresets.cpuThrottlingPresets.map(rate => {
95-
const title = rate === 1 ? i18nString(UIStrings.noThrottling) : i18nString(UIStrings.dSlowdown, {PH1: rate});
120+
let title = rate === 1 ? i18nString(UIStrings.noThrottling) : i18nString(UIStrings.dSlowdown, {PH1: rate});
121+
if (rate === this.#recommendedRate) {
122+
title = i18nString(UIStrings.recommendedThrottling, {PH1: title});
123+
}
124+
96125
const jslogContext = rate === 1 ? 'cpu-no-throttling' : `cpu-throttled-${rate}`;
97126
return html`
98127
<devtools-menu-item
@@ -105,6 +134,7 @@ export class CPUThrottlingSelector extends HTMLElement {
105134
`;
106135
})}
107136
</devtools-select-menu>
137+
${recommendedInfoEl}
108138
`;
109139
// clang-format on
110140
LitHtml.render(output, this.#shadow, {host: this});

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,10 @@ describeWithMockConnection('LiveMetricsView', () => {
945945
assert.lengthOf(envRecs, 2);
946946
assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
947947
assert.match(envRecs[1].textContent!, /Slow 4G/);
948+
949+
const recNotice = view.shadowRoot!.querySelector('.environment-option devtools-network-throttling-selector')
950+
?.shadowRoot!.querySelector('devtools-button');
951+
assert.exists(recNotice);
948952
});
949953

950954
it('should hide if no RTT data', async () => {
@@ -958,6 +962,10 @@ describeWithMockConnection('LiveMetricsView', () => {
958962
const envRecs = getEnvironmentRecs(view);
959963
assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
960964
assert.strictEqual(envRecs[1].textContent, 'Not enough data');
965+
966+
const recNotice = view.shadowRoot!.querySelector('.environment-option devtools-network-throttling-selector')
967+
?.shadowRoot!.querySelector('devtools-button');
968+
assert.notExists(recNotice);
961969
});
962970

963971
it('should suggest no throttling for very low latency', async () => {
@@ -974,6 +982,10 @@ describeWithMockConnection('LiveMetricsView', () => {
974982
const envRecs = getEnvironmentRecs(view);
975983
assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
976984
assert.match(envRecs[1].textContent!, /too fast to simulate with throttling/);
985+
986+
const recNotice = view.shadowRoot!.querySelector('.environment-option devtools-network-throttling-selector')
987+
?.shadowRoot!.querySelector('devtools-button');
988+
assert.notExists(recNotice);
977989
});
978990

979991
it('should ignore presets that are generally too far off', async () => {
@@ -990,6 +1002,10 @@ describeWithMockConnection('LiveMetricsView', () => {
9901002
const envRecs = getEnvironmentRecs(view);
9911003
assert.strictEqual(envRecs[0].textContent, '30% mobile, 60% desktop');
9921004
assert.strictEqual(envRecs[1].textContent, 'Not enough data');
1005+
1006+
const recNotice = view.shadowRoot!.querySelector('.environment-option devtools-network-throttling-selector')
1007+
?.shadowRoot!.querySelector('devtools-button');
1008+
assert.notExists(recNotice);
9931009
});
9941010
});
9951011

front_end/panels/timeline/components/LiveMetricsView.ts

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,12 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
557557
// clang-format on
558558
}
559559

560-
#getNetworkRec(): string|null {
560+
#getCpuRec(): number {
561+
// TODO(cjamcl): will soon be set to recommended calibrated throttling - go/cpq:adaptive-throttling
562+
return 4;
563+
}
564+
565+
#getNetworkRecTitle(): string|null {
561566
const response = this.#getFieldMetricData('round_trip_time');
562567
if (!response?.percentiles) {
563568
return null;
@@ -572,6 +577,30 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
572577
return i18nString(UIStrings.tryDisablingThrottling);
573578
}
574579

580+
const conditions = this.#getNetworkRec();
581+
if (!conditions) {
582+
return null;
583+
}
584+
585+
const title = typeof conditions.title === 'function' ? conditions.title() : conditions.title;
586+
return i18nString(UIStrings.tryUsingThrottling, {PH1: title});
587+
}
588+
589+
#getNetworkRec(): SDK.NetworkManager.Conditions|null {
590+
const response = this.#getFieldMetricData('round_trip_time');
591+
if (!response?.percentiles) {
592+
return null;
593+
}
594+
595+
const rtt = Number(response.percentiles.p75);
596+
if (!Number.isFinite(rtt)) {
597+
return null;
598+
}
599+
600+
if (rtt < RTT_MINIMUM) {
601+
return null;
602+
}
603+
575604
let closestPreset: SDK.NetworkManager.Conditions|null = null;
576605
let smallestDiff = Infinity;
577606
for (const preset of MobileThrottling.ThrottlingPresets.ThrottlingPresets.networkPresets) {
@@ -593,13 +622,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
593622
smallestDiff = diff;
594623
}
595624

596-
if (!closestPreset) {
597-
return null;
598-
}
599-
600-
const title = typeof closestPreset.title === 'function' ? closestPreset.title() : closestPreset.title;
601-
602-
return i18nString(UIStrings.tryUsingThrottling, {PH1: title});
625+
return closestPreset;
603626
}
604627

605628
#getDeviceRec(): string|null {
@@ -624,7 +647,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
624647

625648
const networkRecEl = document.createElement('span');
626649
networkRecEl.classList.add('environment-rec');
627-
networkRecEl.textContent = this.#getNetworkRec() || i18nString(UIStrings.notEnoughData);
650+
networkRecEl.textContent = this.#getNetworkRecTitle() || i18nString(UIStrings.notEnoughData);
628651

629652
// clang-format off
630653
return html`
@@ -637,10 +660,10 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
637660
</ul>
638661
` : nothing}
639662
<div class="environment-option">
640-
<devtools-cpu-throttling-selector></devtools-cpu-throttling-selector>
663+
<devtools-cpu-throttling-selector .recommendedRate=${this.#getCpuRec()}></devtools-cpu-throttling-selector>
641664
</div>
642665
<div class="environment-option">
643-
<devtools-network-throttling-selector></devtools-network-throttling-selector>
666+
<devtools-network-throttling-selector .recommendedConditions=${this.#getNetworkRec()}></devtools-network-throttling-selector>
644667
</div>
645668
<div class="environment-option">
646669
<setting-checkbox

front_end/panels/timeline/components/NetworkThrottlingSelector.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as Common from '../../../core/common/common.js';
88
import * as i18n from '../../../core/i18n/i18n.js';
99
import * as Platform from '../../../core/platform/platform.js';
1010
import * as SDK from '../../../core/sdk/sdk.js';
11+
import * as Buttons from '../../../ui/components/buttons/buttons.js';
1112
import * as ComponentHelpers from '../../../ui/components/helpers/helpers.js';
1213
import type * as Menus from '../../../ui/components/menus/menus.js';
1314
import * as LitHtml from '../../../ui/lit-html/lit-html.js';
@@ -29,6 +30,15 @@ const UIStrings = {
2930
* @example {No throttling} PH1
3031
*/
3132
networkThrottling: 'Network throttling: {PH1}',
33+
/**
34+
* @description Text label for a selection box showing that a specific option is recommended.
35+
* @example {Fast 4G} PH1
36+
*/
37+
recommendedThrottling: '{PH1} - recommended',
38+
/**
39+
* @description Text for why user should change a throttling setting.
40+
*/
41+
recommendedThrottlingReason: 'Consider changing setting to simulate real user environments',
3242
/**
3343
* @description Text label for a menu group that disables network throttling.
3444
*/
@@ -63,6 +73,7 @@ export class NetworkThrottlingSelector extends HTMLElement {
6373
#customNetworkConditionsSetting: Common.Settings.Setting<SDK.NetworkManager.Conditions[]>;
6474
#groups: ConditionsGroup[] = [];
6575
#currentConditions: SDK.NetworkManager.Conditions;
76+
#recommendedConditions: SDK.NetworkManager.Conditions|null = null;
6677

6778
constructor() {
6879
super();
@@ -73,6 +84,11 @@ export class NetworkThrottlingSelector extends HTMLElement {
7384
this.#render();
7485
}
7586

87+
set recommendedConditions(recommendedConditions: SDK.NetworkManager.Conditions|null) {
88+
this.#recommendedConditions = recommendedConditions;
89+
void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
90+
}
91+
7692
connectedCallback(): void {
7793
this.#shadow.adoptedStyleSheets = [networkThrottlingSelectorStyles];
7894
SDK.NetworkManager.MultitargetNetworkManager.instance().addEventListener(
@@ -155,6 +171,15 @@ export class NetworkThrottlingSelector extends HTMLElement {
155171
const selectionTitle = this.#getConditionsTitle(this.#currentConditions);
156172
const selectedConditionsKey = this.#keyForNetworkConditions(this.#currentConditions);
157173

174+
let recommendedInfoEl;
175+
if (this.#recommendedConditions && this.#currentConditions === SDK.NetworkManager.NoThrottlingConditions) {
176+
recommendedInfoEl = html`<devtools-button
177+
title=${i18nString(UIStrings.recommendedThrottlingReason)}
178+
.iconName=${'info'}
179+
.variant=${Buttons.Button.Variant.ICON}
180+
></devtools-button>`;
181+
}
182+
158183
// clang-format off
159184
const output = html`
160185
<devtools-select-menu
@@ -172,12 +197,15 @@ export class NetworkThrottlingSelector extends HTMLElement {
172197
return html`
173198
<devtools-menu-group .name=${group.name}>
174199
${group.items.map(conditions => {
200+
let title = this.#getConditionsTitle(conditions);
201+
if (conditions === this.#recommendedConditions) {
202+
title = i18nString(UIStrings.recommendedThrottling, {PH1: title});
203+
}
204+
175205
const key = this.#keyForNetworkConditions(conditions);
176-
const title = this.#getConditionsTitle(conditions);
177206
const jslogContext = group.jslogContext || Platform.StringUtilities.toKebabCase(conditions.i18nTitleKey || title);
178207
return html`
179208
<devtools-menu-item
180-
title=${title}
181209
.value=${key}
182210
.selected=${selectedConditionsKey === key}
183211
jslog=${VisualLogging.item(jslogContext).track({click: true})}
@@ -199,6 +227,7 @@ export class NetworkThrottlingSelector extends HTMLElement {
199227
`;
200228
})}
201229
</devtools-select-menu>
230+
${recommendedInfoEl}
202231
`;
203232
// clang-format on
204233
LitHtml.render(output, this.#shadow, {host: this});

front_end/panels/timeline/components/cpuThrottlingSelector.css

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@
55
*/
66

77
:host {
8-
display: inline-block;
8+
display: flex;
9+
align-items: center;
910
max-width: 100%;
10-
min-width: 50px;
11+
height: 20px;
1112
}
1213

1314
devtools-select-menu {
15+
min-width: 160px;
1416
max-width: 100%;
17+
height: 20px;
1518
}

0 commit comments

Comments
 (0)