RFR: JDK-8267394: Do not rely on object identity for empty valid Content instance

[Jonathan Gibbons]

On Mon, 31 May 2021 08:47:45 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java line 174:
>> 
>>> 172:             ((ContentBuilder) content).contents.forEach(this::add);
>>> 173:         }
>>> 174:         else if (content.isValid()) {
>> 
>> Should the content builder have a similar validity check to ensure if it's not empty, its contents are always valid? otherwise it's quite hard to define if the content builder is valid or not as it can be considered either and always need special case in client code. In comparison, the html tree's contents are always valid no matter if the outer tags are valid or not.
>
> I think the current behaviour of `ContentBuilder` to accept all content and check validity on demand is the better solution. At least in theory it's possible that invalid content (e.g. an empty HtmlTree) is added to a ContentBuilder which later becomes valid by adding valid content to it. On the other hand, one could argue that `ContentBuilder` and `HtmlTree` should behave the same, which would speak for checking validity when adding to `ContentBuilder`.

Can we take the bigger step and make it unnecessary to have/use `isValid`?  We should never knowingly create nodes that will be invalid by whatever metric we choose to define validity.  If there's a risk of creating nodes that will end up being invalid, we should do the work ahead of time to not create the nodes in the first place.

Bottom line, the original reasons for `isValid` were, at best, flawed. Is this the time to fix that?

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

PR: https://git.openjdk.java.net/jdk/pull/4130