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

[Anthony Petrov]

You're welcome!

--
best regards,
Anthony

On 5/6/2014 12:19 PM, Alexey Ivanov wrote:
> 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.
>>>>>
>>>
>