Tidy up URL generation for asset aggregates

This issue is a followup to 🐛 [regression] Since #1014086 generated CSS assets have absolute URLs without varying by url.site cache context Fixed that was tagged, 'Needs tests'. I removed the tag over there because it was explained in #3354204-8: [regression] Since #1014086 generated CSS assets have absolute URLs without varying by url.site cache context → that tests would be added in the followup, that is, this issue.

Tests are actually in 🐛 Add additional test coverage for CssOptimizer Needs work .

Here's a patch for what the result would look like, but I don't think this is an improvement currently.

Two issues:
1. not sure how to do constructor bc with property promotion.

2. I think we need to postpone this on an issue to add urldecode to fileUrlGenerator somehow.

So needs work, but should be postponed on that issue, however I'm not even sure what the file generator issue would look like.

I've only quick glanced this based on a ping in another issue so If I make an error my apologies in advance.

To me the call to urldecode() and the comment that its a call to a streamWrapper with the query string already present indicates fault in how the FileUrlGenerator is being used. T

If the path has a query string on it that is not part of its on-disk filename that would be a problem in my opinion a streamWrapepr path of assets://foo.css?bar is not the same file as assets://foo.css and should not be treated as such.

FileUrlGeneratorInterface::generateString()

   * @param string $uri
   *   The URI to a file for which we need an external URL, or the path to a
   *   shipped file.
   *

Core currently uses $uri frequently to refer to public://foo/bar.txt or /foo/bar.txt as it relates to files, which has its own idiosyncrasies. In all these cases its pointing to a full path, in some cases this might be http/https links which are urlencoded() as that is actually a part of its filename to the system, in other cases, such as local disk its the raw filename where characters that would be special for URL's are actual literal filename characters.

Realistically its impossible for the fileUrlGenerator to know when a request its receiving is urlencoded, or when the file actually contains those characters as part of the literal filename. This means the FileUrlGenerator needs to demand that file paths to it be always URL encoded, or never URL encoded it can't reliably have it both ways.

We already have one bug with the FileUrlGenerator trying to urldecode() paths it receives from streamWrapeprs causing faults, see #3314663: File links for paths with reserved characters ( '+' & '#' '?') generated wrong for external streamWrappers. → , deferring this to FileUrlGenerator is likely to cause another path where such an issue occurs.

fwiw #2 is a better patch to look at for the change than #6 which is missing a hunk, but yeah I am starting to think this issue will not actually 'tidy up' but is more moving mess from one place to another.