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

[Petr Pchelko]

Hello, Alexey.

Your test does not test the JFileChooser hang, because the bug is reproducible only in plugin mode where TK thread doesn't 
have an AppContext and we update the ThemeReader on the Toolkit thread. Your test runs in standalone mode
where completely different codepath is used. So I suggest either remove the test or update it to emulate multi-appcontext 
environment. Please see test/javax/swing/JComponent/8043610/bug8043610.java for an example on how to emulate this.

Otherwise the fix looks good assuming the JPRT job runs fine.

With best regards. Petr.

On 04 июля 2014 г., at 14:12, Anthony Petrov <anthony.petrov at oracle.com> 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.
> 
> --
> 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.