RFR: 8267554: Support loading stylesheets from data-URIs [v5]

[Kevin Rushforth]

On Wed, 23 Jun 2021 13:09:51 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> This PR extends data URI support to allow stylesheets to be loaded from data URIs.
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleaned up paragraph tags

I spent some time analyzing the changes to `StyleManager::loadStylesheetUnPrivileged`, especially around the use of the local `parse` variable, since you no longer modify it during the processing in a couple places. I think everything is fine, but I'd like @aghaisas to also look at this when he reviews it. If @dsgrieve has any comments, they would be welcome.

The following is the current behavior for the case where `fname` ends with `.css` or `.bss`:

| `fname` | `.css` exists | `.bss` exists | Result |
| ------- | ------------- | ------------- | ------ |
| _SOMENAME_.bss | YES | NO | fail |
| _SOMENAME_.bss | NO | YES | Use _SOMENAME_.bss instead |
| _SOMENAME_.bss | YES | YES | Use _SOMENAME_.bss |
| _SOMENAME_.css | YES | NO | Use _SOMENAME_.css |
| _SOMENAME_.css | NO | YES | Use _SOMENAME_.bss (NOTE: if `-Dbinary.css=false` then fail) |
| _SOMENAME_.css | YES | YES | Use _SOMENAME_.bss (NOTE: if `-Dbinary.css=false` then _SOMENAME_.css) |

In case where `fname` ends with neither `.css` nor `.css`, it is assumed to be a CSS file and parsed as such.

As near as I can tell, both from reading the code and from running tests, this is still working as expected. So there should be no regression in behavior.

The code to process a data URI looks good.

As for the tests, can you add at least one test of calling the public `setUserAgentStylesheet` method using a data URI? Maybe as part of `StyleManagerTest`? Also, I left a couple suggestions inline.

modules/javafx.graphics/src/main/java/javafx/css/Stylesheet.java line 316:

> 314:             final int bssVersion = dataInputStream.readShort();
> 315:             if (bssVersion > Stylesheet.BINARY_CSS_VERSION) {
> 316:                 throw new IOException("Wrong binary CSS version: "

If the `url` string is non-null shouldn't it still be part of the exception message?

modules/javafx.graphics/src/test/java/test/javafx/css/StylesheetTest.java line 658:

> 656:         try {
> 657:             inputFile = File.createTempFile("convertCssTextToBinary", ".css");
> 658:             outputFile = File.createTempFile("convertCssTextToBinary", ".bss");

Instead of writing a pair of temp files, maybe you can call `CssParser.parse` to create a `Stylesheet` and then `Stylesheet.writeBinary` to write to an in memory `DataOutputStream`? Or alternatively, create the `.bss file` and add it as a resource under `src/test/resources/...`?

-------------

PR: https://git.openjdk.java.net/jfx/pull/536