core(emulation): refactor emulation settings & CLI flags #11779

paulirish · 2020-12-07T21:53:26Z

overview

You must always set formFactor. It doesn't control emulation, but it determines how Lighthouse should interpret the run in regards to scoring performance metrics and skipping mobile-only tests in desktop.

You can choose how screenEmulation is applied. It can accept an object of {width: number, height: number, deviceScaleRatio: number, mobile: boolean} to apply that screen emulation or false if Lighthouse should avoid applying screen emulation. It's typically set to false if either emulation is applied outside of Lighthouse, or it's being run on a mobile device. The mobile boolean applies overlay scrollbars and a few other mobile-specific screen emulation characteristics.

You can choose how to handle userAgent emulation. The emulatedUserAgent property accepts either a string to apply the provided userAgent or false if no UA spoofing should be applied. Typically false is used if UA spoofing is apply outside of Lighthouse or on a mobile device. You can also redundantly apply userAgent emulation with no risk.

changes

Removed: The emulatedFormFactor property (which determined how emulation is applied).
Removed: The TestedAsMobileDevice artifact is removed. Instead of being inferred, the formFactor property is used.
The internalDisableDeviceScreenEmulation property is removed. It's equivalent to the new screenEmulation: false.
Added: The formFactor property.
Added: The screenEmulation property.
Added: The emulatedUserAgent property.
(throttling and throttlingMethod remain unchanged)

To reviewers: i'm not sure there's a good way to break this up. Most of the test changes are rote and boring, except for emulation-test.

TODO

the code already detects a few mismatches and warns.. but if there's a mismatch of settings vs detected environment (hostuseragent is desktop, no emulation being applied, but formFactor === mobile) ... we should also warn/error.
add providedDeviceString, providedNetworkThrottlingString, providedCPUThrottlingString HTML Report renderer incorrect labels for custom throttling & device #7053

attention

…creenEmulation is dropped from devtools

docs/emulation.md

Co-authored-by: Brendan Kenny <[email protected]>

connorjclark · 2020-12-15T23:18:58Z

types/externs.d.ts

-    export type ScreenEmulationSettings = Partial<DeviceMetricsOverrideParams & {disabled: boolean}>;
+    export type ScreenEmulationSettings = {
+      /** Overriding width value in pixels (minimum 0, maximum 10000000). 0 disables the override. */
+      width: number,


we use ; up in here

Co-authored-by: Patrick Hulce <[email protected]> Co-authored-by: Brendan Kenny <[email protected]>

brendankenny

ok, a bit more stuff :)

brendankenny · 2020-12-15T22:08:26Z

lighthouse-cli/cli-flags.js

+          coerce: coerceScreenEmulation,
+        },
+        'emulatedUserAgent': {
+          type: 'string',


note that doing it this way means --no-emulated-user-agent works (it gives false), but

--emulated-user-agent gives ''

--emulated-user-agent false gives (the string) 'false'.

better to use coerce: coerceOptionalStringBoolean, like audit-mode and gather-mode?

ya good call.

brendankenny · 2020-12-15T22:10:57Z

lighthouse-cli/cli-flags.js

      .example(
-          'lighthouse <url> --emulated-form-factor=none --throttling-method=provided',
-          'Disable device emulation and all throttling')
+          'lighthouse <url> --screenEmulation.disabled --throttling-method=provided --no-emulatedUserAgent',


This definitely looks weird interspersing camelCase and kebab-case. Agreed that --screen-emulation.disabled also looks weird, but for non-dot ones --no-emulatedUserAgent is kind of confusing...

brendankenny · 2020-12-15T22:20:21Z

lighthouse-core/gather/gatherers/full-page-screenshot.js

    const height = Math.min(metrics.contentSize.height, MAX_SCREENSHOT_HEIGHT);

    await driver.sendCommand('Emulation.setDeviceMetricsOverride', {
-      mobile: passContext.baseArtifacts.TestedAsMobileDevice,


maybe worth a comment here why it's not set to the emulationSettings.mobile? (I'm not totally sure I know, actually)

It's a good point. This should be screenEmulation.mobile. In practice it doesnt matter because we already exited if there's a mismatch but I like keeping this correct!

brendankenny · 2020-12-15T22:21:48Z

lighthouse-core/gather/gatherers/full-page-screenshot.js

    // like now and reset after gatherer is done.
-    const lighthouseControlsEmulation = passContext.settings.emulatedFormFactor !== 'none' &&
-      !passContext.settings.internalDisableDeviceScreenEmulation;
+    const lighthouseControlsEmulation = passContext.settings.screenEmulation.disabled !== true;


Suggested change

const lighthouseControlsEmulation = passContext.settings.screenEmulation.disabled !== true;

const lighthouseControlsEmulation = passContext.settings.screenEmulation.disabled;

?
for me, at least, !== true twists the brain more than just setting it directly :)

agree with simplifying it, but gotta add a bang to your suggestion.

brendankenny · 2020-12-15T22:49:23Z

lighthouse-core/lib/emulation.js

-    await driver.sendCommand('Emulation.setDeviceMetricsOverride', params.metrics);
-    await driver.sendCommand('Emulation.setTouchEmulationEnabled', {enabled: params.touchEnabled});
+  // As a result, we don't double-apply viewport emulation (devtools sets `screenEmulation` to `false`).
+  // UA emulation, however, is lost in the protocol handover from devtools frontend to the lighthouse_worker. So it's always applied.


these comments might be worth moving to devtools-entry since this is now pretty generalized in here

brendankenny · 2020-12-15T23:09:01Z

lighthouse-core/test/results/artifacts/artifacts.json

-    "emulatedFormFactor": "mobile",
-    "internalDisableDeviceScreenEmulation": false,
+    "formFactor": "mobile",
+    "emulatedUserAgent": "Mozilla/5.0 (Linux; Android 7.0; Moto G (4)) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4143.7 Mobile Safari/537.36 Chrome-Lighthouse",


I guess the UA string count in the LHR is getting a bit ridiculous but not really a clean way to cut back :)

brendankenny · 2020-12-15T23:11:53Z

readme.md

  --additional-trace-categories  Additional categories to capture with the trace (comma-delimited).  [string]
  --config-path                  The path to the config JSON.
-                                 An example config file: lighthouse-core/config/lr-desktop-config.js  [string]
+                                              An example config file: lighthouse-core/config/lr-desktop-config.js  [string]


ha, I did manually edit these ones. I guess yargs does something fancy with the whitespace on these too?

brendankenny · 2020-12-15T23:12:42Z

lighthouse-cli/cli-flags.js

+          describe: 'Determines how performance metrics are scored and if mobile-only audits are skipped. For desktop, --preset=desktop instead.',
+        },
+        'screenEmulation': {
+          describe: 'Sets screen emulation parameters. See also --preset. Use --screenEmulation.disabled to disable. Otherwise set these 4 parameters individually: --screenEmulation.mobile --screenEmulation.width=360 --screenEmulation.height=640 --screenEmulation.deviceScaleFactor=2',


purposefully don't want to break these up like throttling?

correct. I don't wanna grow --help even more.

brendankenny · 2020-12-15T23:24:37Z

lighthouse-core/config/config.js

+    // formFactor doesn't control emulation. So we don't want a mismatch:
+    //   Bad mismatch A: user wants mobile emulation but scoring is configured for desktop
+    //   Bad mismtach B: user wants everything desktop and set formFactor, but accidentally not screenEmulation
+    if (settings.screenEmulation.mobile !== (settings.formFactor === 'mobile')) {


does this mean if you're testing on a real phone and you want to be graded like that, you need to do

formFactor: 'mobile', screenEmulation: { mobile: true, disabled: true, // ... }

?

I had the same thought but it's nested within a check on disabled so it should be fine just with disabled, right?

I had the same thought but it's nested within a check on disabled so it should be fine just with disabled, right?

Oops, yes, good catch. Maybe also worth calling out in the comment then?

does this mean if you're testing on a real phone and you want to be graded like that, you need to do

aside from the defaults you get for free, yes.

lighthouse/docs/emulation.md

Line 19 in 31a7b55

If you're using Lighthouse on a mobile device, you want to set `--screenEmulation.disabled` and `--throttling.cpuSlowdownMultiplier=1`. (`--formFactor=mobile` is the default already).

brendankenny · 2020-12-15T23:30:45Z

lighthouse-core/config/config.js

    // Locale is special and comes only from flags/settings/lookupLocale.
    settingsWithFlags.locale = locale;

+    if (settingsWithFlags.emulatedUserAgent === true) {


you know, locale was special, now everyone wants their setting to have a bespoke initialization :P

I don't know if there's a better way to do this, but leave a comment?

brendankenny

Looks great to me! Way to land a huge improvement in a timeline that didn't seem possible :D

An extra check I wanted to add on top of #11779 It was described back in #10910 > the TestedAsMobileDevice logic correctly handles real-mobile-device, but doesn't handle external UA emulation. We could detect there's external UA emulation being applied and throw if we don't see configuration that matches. Or with the proposal implemented, we may just log a warning to let them know we see i We can only check this sort of mismatch after we've gathered the host UA, so it can't be done earlier.

paulirish added 15 commits December 2, 2020 17:19

batch rename emulatedFormFactor to formFactor

2663a9c

throw if eFF or internalDDSE are set

565ee83

mostly remove internalDisableDeviceScreenEmulation

bd19dd8

remove TestedAsMobileDevice

fac4b46

add disableScreenEmulation

40e2617

add screenEmulation and emulatedUserAgent to config

1e97bf8

lint. plenty of TS errors awaiting me. :)

8f38c6a

move disableScreenEmulation to screenEmulation===false

2716b92

add emulatedUserAgent === false

f413348

artifacts + samplejson

ce850ef

tests. fixing up to match. emulation-test got the most significant

c7c2d9c

attention

add desktop preset

4a61dbf

add docs

36b99a3

cli flags

2f378ff

assert valid flags and settings

ce9b4f8

paulirish requested a review from a team as a code owner December 7, 2020 21:53

paulirish requested review from adamraine and removed request for a team December 7, 2020 21:53

google-cla bot added the cla: yes label Dec 7, 2020

devtools-bot assigned adamraine Dec 7, 2020

devtools-bot added the waiting4reviewer label Dec 7, 2020

paulirish added 7.0 breaking labels Dec 7, 2020

paulirish force-pushed the emulation-flags-refactor branch from 8895bd5 to ce9b4f8 Compare December 7, 2020 23:19

vercel bot deployed to Preview December 7, 2020 23:19 View deployment

toe Pick

3e5fe75

vercel bot deployed to Preview December 8, 2020 00:58 View deployment

paulirish added 2 commits December 7, 2020 17:12

undefined default for a boolean yargs

bab1c15

devtools-entry tweak. webtests will fail until internalDisableDeviceS…

f3c6d35

…creenEmulation is dropped from devtools

vercel bot deployed to Preview December 8, 2020 01:29 View deployment

patrickhulce reviewed Dec 15, 2020

View reviewed changes

docs/emulation.md Outdated Show resolved Hide resolved

Apply suggestions from code review

8b27252

Co-authored-by: Brendan Kenny <[email protected]>

vercel bot deployed to Preview December 15, 2020 23:17 View deployment

connorjclark reviewed Dec 15, 2020

View reviewed changes

Apply suggestions from code review

3caee82

Co-authored-by: Patrick Hulce <[email protected]> Co-authored-by: Brendan Kenny <[email protected]>

vercel bot deployed to Preview December 15, 2020 23:20 View deployment

screenshot smoketest fix

f31a89d

vercel bot deployed to Preview December 15, 2020 23:22 View deployment

semicolon-gate

05917d1

vercel bot deployed to Preview December 15, 2020 23:24 View deployment

drop todo on formfactor

31a7b55

vercel bot deployed to Preview December 15, 2020 23:28 View deployment

brendankenny reviewed Dec 15, 2020

View reviewed changes

paulirish added 2 commits December 15, 2020 16:16

feedback from brendan. emuUA cast still outstanding

09fb558

readme whitespace yay

77d98c7

vercel bot deployed to Preview December 16, 2020 00:20 View deployment

bespoke emuUA initialization comment

7a0e1b9

vercel bot deployed to Preview December 16, 2020 00:25 View deployment

emulatedUserAgent tests, authored by Brendan Kenny

c4a19c6

vercel bot deployed to Preview December 16, 2020 00:28 View deployment

paulirish added the land-when-ci-is-green label Dec 16, 2020

brendankenny approved these changes Dec 16, 2020

View reviewed changes

tests arent typed

58179b0

vercel bot deployed to Preview December 16, 2020 01:12 View deployment

devtools-bot merged commit a6738e0 into master Dec 16, 2020

devtools-bot deleted the emulation-flags-refactor branch December 16, 2020 01:24

devtools-bot removed the land-when-ci-is-green label Dec 16, 2020

paulirish mentioned this pull request Jan 12, 2021

core(proto): clarify deprecated state of EmulatedFormFactor enum #11946

Merged

adamraine mentioned this pull request Jan 25, 2021

misc: fix "fast" npm script #11997

Merged

	const lighthouseControlsEmulation = passContext.settings.screenEmulation.disabled !== true;
	const lighthouseControlsEmulation = passContext.settings.screenEmulation.disabled;

core(emulation): refactor emulation settings & CLI flags #11779

core(emulation): refactor emulation settings & CLI flags #11779

Uh oh!

Conversation

paulirish commented Dec 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

overview

changes

TODO

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brendankenny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulirish Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brendankenny left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

paulirish commented Dec 7, 2020 •

edited

Loading

paulirish Dec 16, 2020 •

edited

Loading