Hello Petr,
Thank you for your review.
Regards,
Alexey.
On 04.07.2014 15:14, Petr Pchelko wrote:
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.
Sorry, disregard this comment. You've updated the code so that now it could
deadlock in standalone too.
The fix looks good.
With best regards. Petr.
On 04 июля 2014 г., at 14:28, Petr Pchelko <petr.pche...@oracle.com> wrote:
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.pet...@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.
