Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: add emulatedFormFactor setting #6098

Merged
merged 2 commits into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lighthouse-cli/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function getFlags(manualArgv) {
.group(
[
'save-assets', 'list-all-audits', 'list-trace-categories', 'additional-trace-categories',
'config-path', 'preset', 'chrome-flags', 'port', 'hostname',
'config-path', 'preset', 'chrome-flags', 'port', 'hostname', 'emulated-form-factor',
'max-wait-for-load', 'enable-error-reporting', 'gather-mode', 'audit-mode',
'only-audits', 'only-categories', 'skip-audits',
],
Expand All @@ -70,7 +70,8 @@ function getFlags(manualArgv) {
'blocked-url-patterns': 'Block any network requests to the specified URL patterns',
'disable-storage-reset':
'Disable clearing the browser cache and other storage APIs before a run',
'disable-device-emulation': 'Disable Nexus 5X emulation',
'disable-device-emulation': 'Disable all device form factor emulation. Deprecated: use --emulated-form-factor=none instead',
'emulated-form-factor': 'Controls the emulated device form factor (mobile vs. desktop) if not disabled',
'throttling-method': 'Controls throttling method',
'throttling.rttMs': 'Controls simulated network RTT (TCP layer)',
'throttling.throughputKbps': 'Controls simulated network download throughput',
Expand Down Expand Up @@ -120,6 +121,7 @@ function getFlags(manualArgv) {
'list-trace-categories', 'view', 'verbose', 'quiet', 'help',
])
.choices('output', printer.getValidOutputOptions())
.choices('emulated-form-factor', ['mobile', 'desktop', 'none'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's set the default to mobile here in the CLI too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

.choices('throttling-method', ['devtools', 'provided', 'simulate'])
.choices('preset', ['full', 'perf', 'mixed-content'])
// force as an array
Expand All @@ -134,6 +136,7 @@ function getFlags(manualArgv) {
// default values
.default('chrome-flags', '')
.default('output', ['html'])
.default('emulated-form-factor', 'mobile')
.default('port', 0)
.default('hostname', 'localhost')
.default('enable-error-reporting', undefined) // Undefined so prompted by default
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const defaultSettings = {
gatherMode: false,
disableStorageReset: false,
disableDeviceEmulation: false,
emulatedFormFactor: 'mobile',

// the following settings have no defaults but we still want ensure that `key in settings`
// in config will work in a typechecked way
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/config/lr-desktop-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const config = {
extends: 'lighthouse:default',
settings: {
maxWaitForLoad: 35 * 1000,
disableDeviceEmulation: true,
emulatedFormFactor: 'desktop',
throttling: {
// Using a "broadband" connection type
// Corresponds to "Dense 4G 25th percentile" in https://docs.google.com/document/d/1Ft1Bnq9-t4jK5egLSOc28IL4TvR-Tt0se_1faTA4KTY/edit#heading=h.bb7nfy2x9e5v
Expand Down
7 changes: 6 additions & 1 deletion lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -991,8 +991,13 @@ class Driver {
* @return {Promise<void>}
*/
async beginEmulation(settings) {
// TODO(phulce): remove this flag on next breaking change
if (!settings.disableDeviceEmulation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add TODO somewhere to remove this flag in a breaking version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 👍

await emulation.enableNexus5X(this);
if (settings.emulatedFormFactor === 'mobile') {
await emulation.enableNexus5X(this);
} else if (settings.emulatedFormFactor === 'desktop') {
await emulation.enableDesktop(this);
}
}

await this.setThrottling(settings, {useThrottling: true});
Expand Down
33 changes: 32 additions & 1 deletion lighthouse-core/lib/emulation.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,25 @@ const NEXUS5X_EMULATION_METRICS = {
},
};

/**
* Desktop metrics adapted from emulated_devices/module.json
* @type {LH.Crdp.Emulation.SetDeviceMetricsOverrideRequest}
*/
const DESKTOP_EMULATION_METRICS = {
mobile: false,
width: 1366,
height: 768,
deviceScaleFactor: 1,
};

const NEXUS5X_USERAGENT = {
userAgent: 'Mozilla/5.0 (Linux; Android 6.0.1; Nexus 5 Build/MRA58N) AppleWebKit/537.36' +
'(KHTML, like Gecko) Chrome/66.0.3359.30 Mobile Safari/537.36',
'(KHTML, like Gecko) Chrome/71.0.3559.0 Mobile Safari/537.36',
};

const DESKTOP_USERAGENT = {
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 ' +
'(KHTML, like Gecko) Chrome/71.0.3559.0 Safari/537.36',
};

const OFFLINE_METRICS = {
Expand Down Expand Up @@ -65,6 +81,20 @@ async function enableNexus5X(driver) {
]);
}

/**
* @param {Driver} driver
* @return {Promise<void>}
*/
async function enableDesktop(driver) {
await Promise.all([
driver.sendCommand('Emulation.setDeviceMetricsOverride', DESKTOP_EMULATION_METRICS),
// Network.enable must be called for UA overriding to work
driver.sendCommand('Network.enable'),
driver.sendCommand('Network.setUserAgentOverride', DESKTOP_USERAGENT),
driver.sendCommand('Emulation.setTouchEmulationEnabled', {enabled: false}),
]);
}

/**
* @param {Driver} driver
* @param {Required<LH.ThrottlingSettings>} throttlingSettings
Expand Down Expand Up @@ -121,6 +151,7 @@ function disableCPUThrottling(driver) {

module.exports = {
enableNexus5X,
enableDesktop,
enableNetworkThrottling,
clearAllNetworkEmulation,
enableCPUThrottling,
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/lib/sentry.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ function init(opts) {
const context = Object.assign({
url: opts.url,
deviceEmulation: !opts.flags.disableDeviceEmulation,
emulatedFormFactor: opts.flags.emulatedFormFactor,
throttlingMethod: opts.flags.throttlingMethod,
}, opts.flags.throttling);
Sentry.mergeContext({extra: Object.assign({}, opts.environmentData.extra, context)});
Expand Down
7 changes: 6 additions & 1 deletion lighthouse-core/report/html/renderer/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,12 @@ class Util {
summary = 'Unknown';
}

const deviceEmulation = settings.disableDeviceEmulation ? 'No emulation' : 'Emulated Nexus 5X';
let deviceEmulation = 'No emulation';
if (!settings.disableDeviceEmulation) {
if (settings.emulatedFormFactor === 'mobile') deviceEmulation = 'Emulated Nexus 5X';
if (settings.emulatedFormFactor === 'desktop') deviceEmulation = 'Emulated Desktop';
}

return {
deviceEmulation,
cpuThrottling,
Expand Down
23 changes: 22 additions & 1 deletion lighthouse-core/test/gather/gather-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('GatherRunner', function() {
);

return GatherRunner.setupDriver(driver, {
settings: {},
settings: {emulatedFormFactor: 'mobile'},
}).then(_ => {
assert.ok(tests.calledDeviceEmulation, 'did not call device emulation');
assert.deepEqual(tests.calledNetworkEmulation, {
Expand Down Expand Up @@ -215,6 +215,25 @@ describe('GatherRunner', function() {
});
});

it('uses correct emulation form factor', async () => {
let emulationParams;
const driver = getMockedEmulationDriver(
params => emulationParams = params,
() => true,
() => true
);

await GatherRunner.setupDriver(driver, {settings: {emulatedFormFactor: 'mobile'}});
expect(emulationParams).toMatchObject({mobile: true});

await GatherRunner.setupDriver(driver, {settings: {emulatedFormFactor: 'desktop'}});
expect(emulationParams).toMatchObject({mobile: false});

emulationParams = undefined;
await GatherRunner.setupDriver(driver, {settings: {emulatedFormFactor: 'none'}});
expect(emulationParams).toBe(undefined);
});

it('stops throttling when not devtools', () => {
const tests = {
calledDeviceEmulation: false,
Expand All @@ -233,6 +252,7 @@ describe('GatherRunner', function() {

return GatherRunner.setupDriver(driver, {
settings: {
emulatedFormFactor: 'mobile',
throttlingMethod: 'provided',
},
}).then(_ => {
Expand Down Expand Up @@ -262,6 +282,7 @@ describe('GatherRunner', function() {

return GatherRunner.setupDriver(driver, {
settings: {
emulatedFormFactor: 'mobile',
throttlingMethod: 'devtools',
throttling: {
requestLatencyMs: 100,
Expand Down
5 changes: 4 additions & 1 deletion lighthouse-core/test/report/html/renderer/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ describe('util helpers', () => {
it('builds device emulation string', () => {
const get = opts => Util.getEmulationDescriptions(opts).deviceEmulation;
assert.equal(get({disableDeviceEmulation: true}), 'No emulation');
assert.equal(get({disableDeviceEmulation: false}), 'Emulated Nexus 5X');
assert.equal(get({disableDeviceEmulation: false}), 'No emulation');
assert.equal(get({emulatedFormFactor: 'none'}), 'No emulation');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this include the disableDeviceEmulation: false -> 'No emulation' case as well (since no emulatedFormFactor)? Admittedly Config won't let you get into this situation due to the emulatedFormFactor default, but it feels weird to leave out when testing just this file :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done :)

assert.equal(get({emulatedFormFactor: 'mobile'}), 'Emulated Nexus 5X');
assert.equal(get({emulatedFormFactor: 'desktop'}), 'Emulated Desktop');
});

it('builds throttling strings when provided', () => {
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -2779,6 +2779,7 @@
"gatherMode": false,
"disableStorageReset": false,
"disableDeviceEmulation": false,
"emulatedFormFactor": "mobile",
"locale": "en-US",
"blockedUrlPatterns": null,
"additionalTraceCategories": null,
Expand Down
3 changes: 2 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Configuration:
--hostname The hostname to use for the debugging protocol. [default: "localhost"]
--max-wait-for-load The timeout (in milliseconds) to wait before the page is considered done loading and the run should continue.
WARNING: Very high values can lead to large traces and instability [default: 45000]
--emulated-form-factor Controls the emulated device form factor (mobile vs. desktop) if not disabled [choices: "mobile", "desktop", "none"] [default: "mobile"]
--enable-error-reporting Enables error reporting, overriding any saved preference. --no-enable-error-reporting will do the opposite. More:
https://git.io/vFFTO
--gather-mode, -G Collect artifacts from a connected browser and save to disk. If audit-mode is not also enabled, the run will quit
Expand All @@ -83,7 +84,7 @@ Options:
--version Show version number [boolean]
--blocked-url-patterns Block any network requests to the specified URL patterns [array]
--disable-storage-reset Disable clearing the browser cache and other storage APIs before a run [boolean]
--disable-device-emulation Disable Nexus 5X emulation [boolean]
--disable-device-emulation Disable all device form factor emulation. Deprecated: use --emulated-form-factor=none instead [boolean]
--throttling-method Controls throttling method [choices: "devtools", "provided", "simulate"]
--throttling.rttMs Controls simulated network RTT (TCP layer)
--throttling.throughputKbps Controls simulated network download throughput
Expand Down
1 change: 1 addition & 0 deletions typings/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ declare global {
gatherMode?: boolean | string;
disableStorageReset?: boolean;
disableDeviceEmulation?: boolean;
emulatedFormFactor?: 'mobile'|'desktop'|'none';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes me want to add a 'none' to throttlingMethod that's just an alias of 'provided' 😆

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes me want to add a 'none' to throttlingMethod that's just an alias of 'provided'

yes pls :D

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to as it would be confusing if this one has a none but throttling does not.
Maybe add an alias provided here too? 🙄

throttlingMethod?: 'devtools'|'simulate'|'provided';
throttling?: ThrottlingSettings;
onlyAudits?: string[] | null;
Expand Down