<AWT Dev> <Swing Dev> [9] Review request for JDK-8039383: NPE when changing Windows Theme

[Alexey Ivanov]

Hi AWT and Swing teams,

Could you please review the updated fix:
http://cr.openjdk.java.net/~dmarkov/8039383/jdk9/webrev.02/


What has changed since version .01?

The issue was still reproducible with the .01 version of the fix, 
however it was harder to reproduce.

So in version .02 I added checks for null where possible. If theme 
becomes unavailable, a dummy value will be used; this way applications 
will be more stable. As soon as the theme change events are handled, the 
entire UI will update properly.

Thank you,
Alexey.

On 24.04.2014 16:23, Alexey Ivanov wrote:
> Hi Anthony, AWT and Swing teams,
>
> Here's the updated fix:
> http://cr.openjdk.java.net/~dmarkov/8039383/jdk9/webrev.01/
>
> Description:
> In the new version of the fix, I use a new xpstyleEnabled field of 
> AtomicBoolean in WToolkit to track the current value of 
> "win.xpstyle.themeActive" desktop property. Its value is updated in 
> windowsSettingChange() and in updateProperties().
> XPStyle.getXP() checks its cached value in themeActive and the current 
> value in WToolkit; if the value changes, it schedules updateAllUIs and 
> invalidates the current style right away, so that components would not 
> access data from the theme that became unavailable.
> Two functions in ThemeReader also check this special field from 
> WToolkit which prevents throwing InternalError when paint is called 
> before theme change is fully handled.
>
> Before updateAllUIs is invoked, it's possible that application will 
> paint with some values cached from the previous theme. Usually it 
> happens before "Theme change" dialog disappears from the screen, at 
> least I noticed no UI glitches during normal theme change.
>
>
> Regression test:
> No regression test is provided due to its complexity. Additionally, 
> whether NullPointerException or InternalError are thrown depends on 
> the order of event handling, sometimes exceptions do not occur when 
> changing theme of visual styles enabled theme to a classic theme.
>
>
> Thank you,
> Alexey.
>
> On 18.04.2014 18:52, Anthony Petrov wrote:
>> Hi Alexey,
>>
>> No, unfortunately I don't have any suggestions right now.
>>
>> As for allowing executing user code on the toolkit thread, we can't 
>> accept such a fix. Sorry about that.
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 4/18/2014 6:48 PM, Alexey Ivanov wrote:
>>> Hi Anthony,
>>>
>>> Thank you for your review.
>>>
>>> Yes, user code can install a property change listener... It was my
>>> concern too, that's why I explicitly noted about this.
>>>
>>> Do you have any suggestion how this situation can be handled?
>>> Is it a general rule that all desktop property change listeners must be
>>> called on EDT?
>>>
>>>
>>> Thanks,
>>> Alexey.
>>>
>>> On 18.04.2014 16:02, Anthony Petrov wrote:
>>>> Hi Alexey,
>>>>
>>>>> With this change, property "win.xpstyle.themeActive" change is fired
>>>>> on the toolkit thread
>>>>
>>>> Is it possible to install a change listener for this property from
>>>> user code, and hence eventually allow executing some user code on the
>>>> toolkit thread with your fix?
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 4/18/2014 12:09 PM, Alexey Ivanov wrote:
>>>>> Hello Swing and AWT teams,
>>>>>
>>>>> Could you please review the fix for jdk9:
>>>>>      bug: https://bugs.openjdk.java.net/browse/JDK-8039383
>>>>>      webrev: 
>>>>> http://cr.openjdk.java.net/~dmarkov/8039383/jdk9/webrev.00/
>>>>>
>>>>> Problem description:
>>>>> When changing Windows themes from a theme with visual styles (Windows
>>>>> Aero or Windows Basic) to a classic one, NullPointerException 
>>>>> could be
>>>>> thrown from Swing code during component tree validation, or
>>>>> InternalError could be thrown during component painting.
>>>>>
>>>>> Root cause:
>>>>> Windows theme data are "cached" in XPStyle object. When the theme is
>>>>> switched to a classic one, HTHEME handle becomes unavailable and data
>>>>> cannot be accessed from the theme any more. The change in theme in
>>>>> posted to EDT via invokeLater. At the same time, the UI needs to 
>>>>> repaint
>>>>> itself as soon as Windows changed the theme, and paint code is often
>>>>> called before the theme change is handled in Java. This leads to 
>>>>> NPE and
>>>>> InternalError as the code tries to access the data that has become
>>>>> unavailable.
>>>>>
>>>>> The fix:
>>>>> Update "win.xpstyle.themeActive" desktop property and invalidate the
>>>>> cached XPStyle as soon as windowsSettingChange() is called from 
>>>>> native
>>>>> code. Thus when Swing code needs to access theme data, it will see no
>>>>> theme is available and will fallback to classic painting.
>>>>>
>>>>> Note: Before the fix, PropertyChangeEvents for desktop properties in
>>>>> Windows were fired on the Event Dispatch Thread. With this change,
>>>>> property "win.xpstyle.themeActive" change is fired on the toolkit
>>>>> thread; all other properties are changed on the EDT as before.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Alexey.
>>>
>