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

[Anthony Petrov]

Thanks for the update. The fix looks good to me.

--
best regards,
Anthony

On 7/4/2014 5:49 PM, Alexey Ivanov wrote:
> Hi Anthony,
>
> Thank you for your review.
> You're right, there's a potential NPE at line 941 in WToolkit.java.
>
> I've added null check to avoid NPE.
>
>
> Could you please review the updated webrev?
> http://cr.openjdk.java.net/~aivanov/8046559/jdk9/webrev.01/
>
>
> Thank you,
> Alexey.
>
> On 04.07.2014 14:12, Anthony Petrov wrote:
>> Hi Alexey,
>>
>> src/windows/classes/sun/awt/windows/WToolkit.java
>>>  940         final Map<String, Object> props = getWProps();
>>>  941 updateXPStyleEnabled(props.get(XPSTYLE_THEME_ACTIVE));
>>
>>>  971     private synchronized Map<String, Object> getWProps() {
>>>  972         return (wprops != null) ? wprops.getProperties() : null;
>>
>> There is a potential NPE at line 941.
> Yes, you're
>>
>> --
>> best regards,
>> Anthony
>>
>> On 7/2/2014 5:28 PM, Alexey Ivanov wrote:
>>> Hi AWT, Swing teams,
>>>
>>> Could you please review the fix to JDK-8046559 for jdk9:
>>>      bug: https://bugs.openjdk.java.net/browse/JDK-8046559
>>>      webrev: http://cr.openjdk.java.net/~aivanov/8046559/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 accessed via XPStyle and ThemeReader. When the
>>> theme is switched to a classic one, theme handles become unavailable,
>>> and theme data cannot be accessed any more. The change in theme is
>>> posted to EDT; at the same time, the UI often needs to repaint before
>>> the theme change is completely handled in Swing. This leads to NPE and
>>> InternalError as the code tries to access the data that has become
>>> unavailable.
>>>
>>> The fix:
>>> Windows desktop properties are updated right on Windows Toolkit thread,
>>> and the value of "win.xpstyle.themeActive" is stored in
>>> ThemeReader.xpStyleEnabled field. PropertyChangeEvents for desktop
>>> properties are delivered either synchronously on EDT or asynchronously
>>> via DesktopPropertyChangeSupport, as it used to be.
>>>
>>> Getters in XPStyle class check for null values and return dummy defaults
>>> if ThemeReader returned null. SkinPainters also check whether theming is
>>> still available before asking ThemeReader to paint.
>>>
>>> Access to XPStyle.xp instance is blocked as soon as user switches
>>> themes. The object will be cleaned when the corresponding
>>> PropertyChangeEvent is handled by Swing.
>>>
>>>
>>> This new version of the fix addresses issues found with the initial
>>> submission of JDK-8039383 fix:
>>>    - JDK-8046239: Build failure in 9-client on all non-Windows platforms
>>>    - JDK-8046391: Hang displaying JFileChooser with Windows L&F
>>>
>>> See also
>>> http://mail.openjdk.java.net/pipermail/awt-dev/2014-June/007975.html
>>> and http://cr.openjdk.java.net/~aivanov/8039383/jdk9/webrev.03/
>>>
>>>
>>> Regression test:
>>> No regression test is provided due to its complexity. 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.
>>>
>>> I included regression test for hang in JFileChooser, JDK-8046391.
>>>
>>>
>>> Thank you,
>>> Alexey.
>