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

[Alexey Ivanov]

Hi Anthony,

Thank you for your review.
And thanks for pointing out no user code is allowed to run on the 
toolkit thread.

--
Regards,
Alexey.

On 25.04.2014 16:03, Anthony Petrov wrote:
> Hi Alexey,
>
> I'm not an expert in this particular code, but as long as the 
> suggested fix doesn't allow user code to run on the toolkit thread, 
> I'm fine with the fix.
>
> -- 
> best regards,
> Anthony
>
> On 4/24/2014 4:23 PM, 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.
>>>>
>>