Prevent submitting URL Metric if viewport size changed

[westonruter]

This is cherry-picked from a75b94f in #1697.

Only proceed with submitting the URL Metric if the viewport stayed the same size from when the page first loaded until it was closed. Changing the viewport size (e.g. due to resizing a window or changing the orientation of a device) will result in unexpected metrics being collected.

[westonruter]

From @felixarntz in #1697 (comment):

Couldn't this be moved much further up in the logic, given we'll know whether the viewport has changed far earlier in this function? This could avoid doing quite a bit of "useless" processing as far as I can tell.

[felixarntz]

Awaiting an answer to this question, otherwise LGTM :)

[westonruter]

No, it cannot be moved up. Note the immediately preceding code:

performance/plugins/optimization-detective/detect.js

Lines 497 to 511 in a5bd367

	// Wait for the page to be hidden.
	await new Promise( ( resolve ) => {
	win.addEventListener( 'pagehide', resolve, { once: true } );
	win.addEventListener( 'pageswap', resolve, { once: true } );
	doc.addEventListener(
	'visibilitychange',
	() => {
	if ( document.visibilityState === 'hidden' ) {
	// TODO: This will fire even when switching tabs.
	resolve();
	}
	},
	{ once: true }
	);
	} );

Recall that the URL Metric is now sent when someone leaves the page, not as soon as the page is fully loaded. So data is collected up until the moment someone leaves the page, and if at this moment we see that the page is not the same dimensions, then we abort sending the URL Metric.

That said, I realize that a resize event listener would be more robust, as it could be that someone resizes the window once the page has loaded, and then before leaving they resize it back to the original size. That wouldn't be detected as it is here. I've implemented resize event in e222c85.

[westonruter]

@felixarntz Does this make sense?

[felixarntz]

@westonruter Makes sense. I wonder though, would there be other tasks here that could be deferred to when leaving the page so that they don't run unnecessarily? For example some of the LCP element logic that runs before then as far as I can see?

[westonruter]

Yes, although I think if we wait until this point to call onLCP() I'd worry about increasing the amount of time needing to be spent at this pagehide phase and risk the browser killing the tasks before it gets a chance to send the beacon. But I think that is something that we can investigate further in a follow-up.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.