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

[Hannes Wallnöfer]

On Thu, 20 May 2021 15:22:08 GMT, liach <github.com+7806504+liach at openjdk.org> wrote:

>> This is a simple cleanup to replace the sentinel `HtmlTree.EMPTY` text constant with an instance that achieves the same by overriding `isValid()`. I think this is the nicer solution, and it allows us to remove the special case identity check in `HtmlTree.add(Content)`.
>
> 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`.

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

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