I'm kind of curious why the code (before your changes) wasn't framed in this sort of way:

if ( false !== strpos( $agent, 'googleweblight' ) ) {
	$set_cache = true;
}

/**
 * Allow plugins to enable nocache headers. Enabled for Google weblight.
 *
 * @param bool $enable_nocache_headers Flag indicating whether to add nocache headers. Default: false.
 */
$set_cache = (bool) apply_filters( 'woocommerce_enable_nocache_headers', $set_cache );

If only because the way the filter's docblock reads to me is that $set_cache will already be true if the request was made by Google Weblight...but really what's happening in the current code is, we force it to true if the request was made by Google Weblight—regardless of what comes through the filter (and I'm just noting this to give context for my next few points).

We could also alternatively introduce a filter to disable the cache if they so choose.

Couldn't we do this now? If for back compat reasons we don't want to change the way the logic works for Weblight, we could still do something like I outlined above but focused on Chrome and the cart page. That way it would be easy for merchants to unwind this change if necessary, at no extra expense in terms of adding or triggering new hooks.

I'm also wondering if we should update the docblock, because we're adding this new exception for Chrome but we're not documenting it there (whereas it's documented for Weblight).

Yeah not sure why it was like that from before but I just reorganized it to a way which was more readable to me. Before it was kind of confusing not to mention the added conditional for Chrome and cart.

So just to be clear, do we want this to take effect on the checkout page?

• For me, is_cart() is only triggered on the cart page and this condition is not met on the checkout page. I'm not sure why we see different things there.
• If we want this to take effect on checkout pages, should we be explicit about it and use the is_checkout() function?

This is because we already have a bug in the logic somewhere. If you look at this https://github.com/woocommerce/woocommerce/blob/5.3.0/includes/wc-conditional-functions.php#L98 you will see it also checks for WOOCOMMERCE_CART constant and if that is true, is_cart() will return true. Coincidentally that constant is set in checkout page as well. Hence this issue. But yeah we could explicitly also add is_checkout() as well.

Yep, I think that makes sense—again for me, locally, is_cart() does not return true on the checkout page. So it would seem like that's a bug, as you say, and possibly a bug that needs specific conditions to be triggered (so either way, shouldn't be relied on)?

Yeah I am not too sure myself. It might of been something wonky with cache but the way I found this is when I ran xdebug and stepped through. I've since removed that comment. So another round please @barryhughes