RFR: 8278620: properties installed by javax.swing.LookAndFeel installColors and installColorsAndFont are not uninstalled [v3]

[Alexey Ivanov]

On Fri, 24 Feb 2023 04:58:12 GMT, Phil Race <prr at openjdk.org> wrote:

> > > And if a default font prevents the FontUIResource from being installed, how does it get installed the in the first place ?
> > > Perhaps that first time the component has no peer and it really is null ?
> > 
> > 
> > The font is null when it's created. Calling `getFont` causes it to go up the hierarchy, if it has a parent. The UI looks to be initialized (and thus a font is set) before `getFont` is called.
> 
> That is what I guessed. Accidental or deliberate ? I'd have to spend time to know.

This is the code of `Component.getFont`:
https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/share/classes/java/awt/Component.java#L1912-L1927

Until the UI is shown, there's no heavyweight peer, so the default font doesn't get set.

Even more, a `UI` instance is set from a component constructor:

https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/share/classes/javax/swing/JLabel.java#L176-L182

https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/share/classes/javax/swing/JPanel.java#L85-L90

Thus, a font is (always) set on a `JComponent` when the constructor is called, the default L&F font would be an instance of `UIResource`.

However, if the font is reset to `null`, the default font (of heavyweight) components / peers comes into play.

The default font gets set in `getGraphics`:

https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/windows/classes/sun/awt/windows/WComponentPeer.java#L640-L643

https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/unix/classes/sun/awt/X11/XWindow.java#L380-L383

> > > FontUIResource is something devised by Swing, for Swing.
> > > Making AWT components depend on it for the convenience of Swing is backwards.
> > 
> > D'oh.

I agree. **AWT should not depend on Swing**.

> > Sounds like the path would be to undo my last commit and just put a note in the code.
> 
> But then there'd either be the correct font not installed or something else bad ?
> How would a note in the code help ?
> 
> Maybe we are going about this all wrong ?
> Maybe uninstall isn't needed ?
> What are the rules (set by Swing?) for what a L&F should do when installing a UI ? If it is "if there is a FontUIResource, then feel free to replace it with yours" then may be everything in this PR (at least about fonts) is un-needed ?

With where the testing led us, I'm inclined to think that the omission of nullifying the font in `uninstallUI` wasn't accidental but rather a deliberate decision.

Yes, a `Font` object gets “leaked” after `UI` is uninstalled from a `JComponent`. In majority of cases `uninstallUI` is followed by `installUI` for another Look-and-Feel. (Without a UI delegate, any `JComponent` is unusable.) As such, the `Font` object gets replaced with a newer one if the previous one is `UIResource` or remains intact if it's not `UIResource`, which likely means it was explicitly set by an app developer.

The common rule for Swing: if a property is `UIResource`, replace it with a new default; otherwise preserve the property, don't change it. From this point of view, all the properties of a Swing component could remain untouched in `uninstallUI`. At the same time, many properties are reset to `null`.

There are issues where font doesn't get reset even though it should. The latest example that comes to my mind is [JDK-6753661](https://bugs.openjdk.org/browse/JDK-6753661) and #12180. Were there others?

This current bug [JDK-8278620](https://bugs.openjdk.org/browse/JDK-8278620) was submitted as the result of work on [JDK-8277817](https://bugs.openjdk.org/browse/JDK-8277817) and #6603 where an object had been serialized, if I remember correctly, but the serialized form cannot be read because a class is removed in a later version of JDK. (The `java/awt/dnd/BadSerializationTest/BadSerializationTest.java` test seems to do the wrong thing: it reads a serialized version of a Swing object which was saved by a previous version of JDK, which isn't supported and which was [mentioned in the comments](https://github.com/openjdk/jdk/pull/6603#issuecomment-991295599).)

The description of JDK-8278620 states `BasicListUI` correctly uninstalls all its properties, *including the font_. Why is it not affected by the problem we're seeing here where the font gets reset to a non-`ResourceUI` font from the heavyweight peer of the frame?

> This is code relied upon by tens of thousands of applications written over a period of 25 years. A change like this is really risky, when you consider that there are probably 10 times more apps than there are Swing regresssion tests .. and only a few of the regression (and other) tests cover this kind of scenario.
> 
> Testing on every platform of every test we have is a bare minimum.
> 
> In the end my point is that unless (and until) we see some application complain, these proactive changes are a bad trade-off.

Perhaps, font property should be *left untouched* just like it is now. However, resetting the color properties — foreground and background — doesn't seem to cause any issues. As such, the scope of this PR could be reduced to adding the `LookAndFeel.uninstallColors` method and using it in UI classes.

As a matter of fact, I ran the client tests with the latest changeset. The set of failing tests [remains the same](https://github.com/openjdk/jdk/pull/10565#issuecomment-1354064460):

* `javax/swing/GraphicsConfigNotifier/StalePreferredSize.java` (on Windows only)
* one closed test;
* one JCK test.

Only the `sanity/client/SwingSet/src/SwingSet2DemoTest.java` test is gone from the list of failures.

So, this hasn't helped.

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

PR: https://git.openjdk.org/jdk/pull/10565