Looks good.
--Semyon
On 3/10/2016 9:45 AM, Prem Balakrishnan wrote:
Hi Semyon and Alexey,
Thankyou for the Review.
I have updated the test as per review comments.
http://cr.openjdk.java.net/~arapte/prem/7012008/webrev.01/
<http://cr.openjdk.java.net/%7Earapte/prem/7012008/webrev.01/>
Regards,
Prem
*From:*Alexey Ivanov
*Sent:* Wednesday, March 09, 2016 3:58 PM
*To:* Prem Balakrishnan; Rajeev Chamyal; Sergey Bylokhov; Semyon
Sadetsky; swing-dev@openjdk.java.net
*Subject:* Re: <Swing Dev> Review Request: JDK-7012008 JDesktopPane -
Wrong background color with Win7+WindowsLnf
Hi Prem,
The fix itself looks good.
Yet I have some questions to the test.
Could you please place the comment with @test tag and other before the
class declaration rather than before imports?
Could you place asterisk in front of each line in this comment block?
It will look consistent with Java style doc comments and other tests.
Do you really need to create UI?
Wouldn't it be enough to create JDesktopPane and check its background
color? I believe it will significantly simplify the test.
I think converting Color to ColorUIResource is unnecessary here.
ColorUIResource is Color decorated with UIResource interface:
Color.equals method does not take this distinction into account.
In main(), you can create and initialize the LaF array in one go.
In executeTest(), you should dispose mainFrame on EDT rather than on
main thread. I'd recommend using finally block in run() to dispose
mainFrame: it will handle both success and failure.
Additionally, you access variables from different threads. The way the
code is written now does not guarantee memory consistency between the
different threads, thus the test could fail or pass sporadically.
And I agree with Semyon, the test should fail on other platforms, you
should just skip the test.
Regards,
Alexey
On 09.03.2016 9:59, Prem Balakrishnan wrote:
Hi*,*
Please review fix for JDK9,
*Bug:*https://bugs.openjdk.java.net/browse/JDK-7012008**
*Webrev:*http://cr.openjdk.java.net/~arapte/prem/7012008/webrev.00/
<http://cr.openjdk.java.net/%7Earapte/prem/7012008/webrev.00/>
*Issue:*
JDesktopPane - Wrong background color with Win7+WindowsLnf
*Cause:*
Desktop.background property set to "win.desktop.backgroundColor"
*Fix:*
Changed Desktop.background property to "win.mdi.backgroundColor"
Regards,
Prem
