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

[Alexey Ivanov]

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.