<Swing Dev> Review Request: JDK-7012008 JDesktopPane - Wrong background color with Win7+WindowsLnf

[Semyon Sadetsky]

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 at 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160310/c59f0870/attachment.html>