WIP: audit responsive images

[patrickhulce]

enables more device emulation
adds use-responsive-images audit to dobetterweb

addresses #978

[patrickhulce]

@ebidel needs a bit more cleanup but wanted to grab first impression before doing so

[patrickhulce]

I added extra empty passes at first, but for some sites this adds a really long time. Not sure if these emulation changes will break anything

[brendankenny]

yeah, I think we're going to have to work through some of the implications here.

We've talked about having flags be settable both on the command line but also in the config file, which I think will be a good thing, but we'll have to keep them in sync, e.g. what does it mean when you have --disable-device-emulation=true but emulation: iphone in your config, why does one allow you to select the device but the other doesn't, etc.

We also need to dedupe some of the emulation terminology as we did for the CLI flags but didn't do internally since driver.beginEmulation still handles everything.

That's all stuff we need to figure out/do anyways, so that's great, but specific to this PR, I'm not entirely sure of the reason for the audit vs #876 (and maybe #718). Why not just check if the images are way too big for the page and suggest alternatives?

[patrickhulce]

We've talked about having flags be settable both on the command line but also in the config file, which I think will be a good thing, but we'll have to keep them in sync, e.g. what does it mean when you have --disable-device-emulation=true but emulation: iphone in your config, why does one allow you to select the device but the other doesn't, etc.

I still see these as two distinct abilities atm, in the config you can specify which device you wish to be emulating, and through the CLI you can disable emulation entirely which will effectively ignore those devices (whether or not that's a good thing is another question ;) )

We also need to dedupe some of the emulation terminology as we did for the CLI flags but didn't do internally since driver.beginEmulation still handles everything.

Yeah I see this as stage 1, where stage 2 has the emulate device bit control corresponding network and CPU throttling as well

That's all stuff we need to figure out/do anyways, so that's great, but specific to this PR, I'm not entirely sure of the reason for the audit vs #876 (and maybe #718). Why not just check if the images are way too big for the page and suggest alternatives?

Yeah the descriptions all seemed to overlap a lot to me, but I currently see this as v1 to actually analyzing the images for size maybe compression/quality etc as v2. And client hints/save data is just a "do you support it" check

[brendankenny]

Yeah the descriptions all seemed to overlap a lot to me, but I currently see this as v1 to actually analyzing the images for size maybe compression/quality etc as v2. And client hints/save data is just a "do you support it" check

My real question was why compare two image loads vs just "is this image appropriate for this device, size on screen, etc". The only benefit seems to be we could warn that that desktop could be loading bigger images

[brendankenny]

(also worth noting that when running lighthouse on a real device, no emulation obviously still means running on a mobile device)

[patrickhulce]

My real question was why compare two image loads vs just "is this image appropriate for this device, size on screen, etc". The only benefit seems to be we could warn that that desktop could be loading bigger images

Short answer: because that seemed more complicated to intermingle with a PR focused on the emulation angle and would come later

There seemed to be many different underlying concerns amidst those 3 issues and this represents one angle of attack (which I'm fine scraping if it seems useless)

(also worth noting that when running lighthouse on a real device, no emulation obviously still means running on a mobile device)

Fair I'll switch to back to an emulation profile

[brendankenny]

Short answer: because that seemed more complicated to intermingle with a PR focused on the emulation angle and would come later

ah, I missed that the real point of this PR is the increased emulation flexibility and not (necessarily) the responsive image audit :)

[brendankenny]

buuut, that said, I'm thinking we actually shouldn't move forward with this level of granularity in emulation switching unless we have a really compelling reason for it.

It will be difficult to write gatherers that are indifferent to the emulation state they're in. A good example would be the DBW gatherers that run in the fourth pass in this audit. Since the callsite gatherers don't do static analysis, only reporting when problematic functions were actually executed, the specific code paths that end up running become really important.

If the fourth pass were to run with no mobile emulation, for example, the report may end up confusing on performance problems since the perf metrics would be reported for the mobile version of the site but things like document.write calls would be reported for the desktop version of the site. This is especially problematic if the developer only wanted to test the mobile version of the site in the first place.

So my initial thoughts, at least, are that the assumption should be that all passes run at the same level of emulation, and then we could maybe (somehow) make it easy for a gatherer to make requests or take actions with different emulation properties solely within that gatherer's methods, restoring everything before leaving it.

[patrickhulce]

alright