Re: CFV: New Swing Group Member: Semyon Sadetsky

2016-03-22 Thread Artem Ananiev 


Vote: yes

Artem

On 3/21/16 9:01 AM, Alexander Scherbatiy wrote:


I hereby nominate Semyon Sadetsky (OpenJDK user name: ssadetsky) to
Membership in the Swing Group.

Semyon is active member of Swing group and contributed a lot of fixes
which include Swing TimerQueue race condition improvement, UndoManager
deadlock resolving, proposing and implementation a public API for
internal Swing functionality which is part of Swing modularization
support for JDK 9, and others (see Semyon’s list OpenJDK changesets [1])

Semyon proposed a big change for the Swing part of the JEP 283 “Enable
GTK 3 on Linux” [2] implementation [3].

Semyon not only provides fixes for Swing issues but also regularly
reviews fixes suggested by other members.

Votes are due by Apr 5, 2016.

Only current Members of the Swing  Group [4] are eligible
to vote on this nomination.

For Lazy Consensus voting instructions, see [5].

Thanks,
Alexandr.

[1]
http://hg.openjdk.java.net/jdk9/dev/jdk/log?revcount=1000&rev=author%28%22ssadetsky%22%29

[2] http://openjdk.java.net/jeps/283
[3] http://mail.openjdk.java.net/pipermail/swing-dev/2016-March/005440.html
[4] http://openjdk.java.net/census#swing
[5] http://openjdk.java.net/groups/#member-vote

Re: Review Request of 8151282: [TEST_BUG] javax/swing/JInternalFrame/8146321/JInternalFrameIconTest.java fails with GTK LnF

2016-03-22 Thread Sergey Bylokhov 

Looks fine.

On 22.03.16 10:38, Avik Niyogi wrote:

Hi All,
Please find webrev with new code fix:
http://cr.openjdk.java.net/~aniyogi/8151282/webrev.02/

With Regards,
Avik Niyogi


On 21-Mar-2016, at 8:07 pm, Sergey Bylokhov
mailto:sergey.bylok...@oracle.com>> wrote:

Hi, Avik.
How this test can fail? I do not see any raised exceptions.

On 21.03.16 9:46, Avik Niyogi wrote:

Hi Sergey,
Please review the following code change.
With Regards,
Avik Niyogi

On 19-Mar-2016, at 2:15 am, Alexander Scherbatiy
mailto:alexandr.scherba...@oracle.com>
> wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 14/03/16 16:25, Avik Niyogi wrote:

Hi All,
Please review code changes made as per inputs provided.

http://cr.openjdk.java.net/~aniyogi/8151282/webrev.01/

With Regards,
Avik Niyogi

On 14-Mar-2016, at 10:53 am, Avik Niyogi
<avik.niy...@oracle.com
> wrote:

Hi Sergey,
Seems like it is a delay issue. Will submit test case with a
waitForIdle() fix.

With Regards,
Avik Niyogi

On 10-Mar-2016, at 7:22 pm, Sergey Bylokhov
mailto:sergey.bylok...@oracle.com>>
wrote:

Hi, Avik.
A few questions.
- Why webrev contains the new file?
- You marked the test as a mac specific but it is iterates over a
bunch of l&fs. It seems it should not be OS specific, but should
check some specific l&fs (which support such icons): Metal, Nimbus,
Aqua, Windows(???). So gtk/motif should be skipped.

On 08.03.16 8:10, Avik Niyogi wrote:

Hi All,

Kindly review the bug fix for JDK 9.

*Bug:*

https://bugs.openjdk.java.net/browse/JDK-8151282

*Webrev:*

_http://cr.openjdk.java.net/~aniyogi/8151282/webrev.00/_
_
_
*Issue:*
The test case failed for GTK LAF.

*Cause:*
The test case was meant to be Mac specific and was missing a jtreg
parameter

*Fix:*
Minor change to test case with @requires tag added to set the fix.

With Regards,
Avik Niyogi



--
Best regards, Sergey.











--
Best regards, Sergey.





--
Best regards, Sergey.

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

2016-03-22 Thread Sergey Bylokhov 
Looks fine to me. But I am not an expert here. And I wonder why the  
tag is so specific, can we get the similar issue if some other tags will 
be used instead?


On 22.03.16 11:35, Rajeev Chamyal wrote:

Hello All,

Gentle reminder.
Please review the fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8150225
Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal
Sent: 09 March 2016 15:58
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hello Sergey,

I have run JCK tests for HTMLWriter and AbstractWriter with this fix and all 
passed.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov
Sent: 09 March 2016 15:54
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hi, Rajeev.
Please confirm that there are no new issues in the jck after this fix.

On 09.03.16 12:18, Rajeev Chamyal wrote:

Hello All,

Please review the following fix for Jdk9:

Bug : https://bugs.openjdk.java.net/browse/JDK-8150225

Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/


Issue : JCK conformance test failed due to fix for bug JDK-7104635

Fix: Reverted the fix for JDK-7104635 and added a new method in
HTMLWriter.java to check if P tag is within Pre tag.

Decrement indentation is skipped if P tag is with a Pre tag.

Regards,

Rajeev Chamyal




--
Best regards, Sergey.




--
Best regards, Sergey.

Re: [9] Review Request for 8078514: Nightly: api/javax_swing/DefaultRowSorter/index_ModelStructChanged failure

2016-03-22 Thread Alexandr Scherbatiy 

The fix looks good to me.

Thanks,
Alexandr.

On 12/8/2015 11:04 PM, Semyon Sadetsky wrote:



On 12/8/2015 10:56 PM, Sergey Bylokhov wrote:

On 02/11/15 16:51, Semyon Sadetsky wrote:

On 5/26/2015 1:38 PM, Alexander Scherbatiy wrote:

On 5/21/2015 5:34 PM, Semyon Sadetsky wrote:

Hello,

I have decided to remake the fix.
The reason for that is sun.swing.FilePane class. One of its inner
classes extends DefaultRowSorter and relays on lazy initialization of
the DefaultRowSorter in unsorted state. After removing the lazy init
the FilePane crashes with AOB exception.


   It looks like FilePane tries to call some operations like
DefaultRowSorter.convertRowIndexToView(int) on non updated
DefaultRowSorter.
   Is it possible to fix it in FilePane?

I fixed the FilePane. Please review the updated webrev:
http://cr.openjdk.java.net/~ssadetsky/8078514/webrev.02/


Can you please clarify the change from v01->v02 related to 
modelRowCount. One fix version uses Math.max, latest version skip it. 
but getViewRowCount() still use Math.max.
There are DefaultRowSorter usage in the FilePane that relays on the 
current logic, but the current logic doesn't work for JTable sorting. 
(Remind you that the initial problem was 6894632).
The first solution worked in both cases, but it had caused Alexanders 
doubts in offline discussion.
The second version fixes the FilePane and doesn't use the ambiguous 
Math.max.

For me both are working solutions.




  Thanks,
  Alexandr.


This can be fixed, but I think it will be too big change for the
issue and users can be already using the DefaultRowSorter in the
similar way.
Here is the new approach:
http://cr.openjdk.java.net/~ssadetsky/8078514/webrev.01/
It looks a little bit risky ,but all related tests have been passed.

--Semyon

On 5/19/2015 2:03 PM, Alexander Scherbatiy wrote:

On 5/15/2015 5:49 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8078514
webrev: http://cr.openjdk.java.net/~ssadetsky/8078514/webrev.00/


  DefaultRowSorter
221 allChanged();
222 modelRowCount = getModelWrapper().getRowCount();

- This can be moved to a private method that will be used both in
the public modelStructureChanged() and setModelWrapper() methods.

532 public void sort()
- Could the rawFilter be null in case viewToModel != null an
!isUnsorted() ?
- isUnsorted() method is called twice. Is it possible to store its
value to a variable and use it?

Thanks,
Alexandr.



The 6894632 fix violated a contract between the table and its row
sorter: the sorter should receive TableChanged events even if table
is not sorted actually.
Another way to fix 6894632 is to initialize sorter internal
structures instantly. The last is applied in the fix.

--Semyon

Re: [9] Review Request: 8144166 [macosx] Test java/awt/Component/CompEventOnHiddenComponent/CompEventOnHiddenComponent.java fails

2016-03-22 Thread Sergey Bylokhov 

On 22.03.16 9:11, Semyon Sadetsky wrote:

On 3/11/2016 12:40 PM, Sergey Bylokhov wrote:

On 11.03.16 9:30, Semyon Sadetsky wrote:

Hi Sergey,

Could you explain why do you suppose that the icon cannot be requested
more then once between the frame minimize and restore?


Every time when the frame is minimized the new
AquaInternalFrameDockIconUI is created. Every
AquaInternalFrameDockIconUI have its own ScaledImageLabel(which is
JLabel). At the beginning this label is initialized by the icon from
the cache, and this label is used until the frame is restored(cache is
not used).

What if user requests the dock icon to paint it somewhere else while the
frame is hidden? Before the fix the user got the same icon image from
the cache, but after the fix he gets another image because it is being
re-created on each paint.


After the fix the new image recreated only when the frame is minimized, 
after that the same icon will be used for every repaint action: Because 
we set this image as an icon for ScaledImageLabel and check it to null 
on each repaint:

189 public void paint(final Graphics g) {
190 if (getIcon() == null) updateIcon();


So it becomes impossible to paint the same
icon that is shown in the dock currently.






 > The bug is in the last step. When the frame will be minimized in the
second time, it can have another content. But minimized icon
 > will show the miniature from the previous minimization...
Doesn't it contradict with :
 >* If the frame will be resized/show the cache will be reset.
?

--Semyon

On 3/10/2016 11:35 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

Problem description:
 - CompEventOnHiddenComponent test was written not according the
specification but according an assumption(performance related) - "the
component should not post move/resize events if no listeners were
registered". The test creates a JInternalFrame and adds
ComponentListener to it. No events should come to the listeners,
because the frame is not resized or moved by the test(except initial
layout). Initial move/resize events are skipped usually, because
during initialization there are no any listeners. But this is not the
case in Aqua. AquaInternalFrameDockIconUI adds a listeners to the
JInternalFrame during initialization -> move/resize events are posted
-> the test adds own listeners -> events dispatched -> the test
listeners called -> boom.

According above description the bug could be closed as not a defect.
But I found another related problem in the AquaInternalFrameDockIconUI.

 - These component listeners in AquaInternalFrameDockIconUI are used
to maintain the cache of image for the dock(the icon which is used
when the internal frame is minimized). The logic is next:
   * Before the frame will be minimized it draws to the special
ImageIcon which will be used as an icon for the dock.
   * After the frame is minimized and dock is showing the saved image
will be used(and placed to the cache)
   * If the frame will be resized/show the cache will be reset.
   * When in the next time the same frame will be minimized the cached
image will be used.

The bug is in the last step. When the frame will be minimized in the
second time, it can have another content. But minimized icon will show
the miniature from the previous minimization. Note this is the only
step where the cache is used.

Solution:
As a solution all cache related stuff were dropped. The new test was
added.


Bug: https://bugs.openjdk.java.net/browse/JDK-8144166
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8144166/webrev.01











--
Best regards, Sergey.

Re: Review request for JDK-8075084 JOptionPane.showMessageDialog causes JScrollBar to move

2016-03-22 Thread Rajeev Chamyal 
Hello All,

Please review the re-worked fix.
Bug: https://bugs.openjdk.java.net/browse/JDK-8075084
Webrev : http://cr.openjdk.java.net/~rchamyal/8075084/webrev.03/

In the updated fix a global awt event listener has been added to 
BasicScrollBarUI to take actions on mouse events.
The awt event listener determines the state of arrow buttons and source of 
mouse events and based on these it stops the timer.

Regards,
Rajeev Chamyal


 -Original Message-
From: Alexander Scherbatiy 
Sent: 13 January 2016 21:48
To: Rajeev Chamyal
Cc: Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  Review request for JDK-8075084 
JOptionPane.showMessageDialog causes JScrollBar to move

On 1/12/2016 5:28 PM, Rajeev Chamyal wrote:
> Hello All,
>
> Gentle reminder to review the fix.
> http://cr.openjdk.java.net/~rchamyal/8075084/webrev.02/

My impression was that the scroll timer is started before an adjustment 
listener is executed. In this case the timer can track the open modal dialog 
and be stopped.
It looks like the real situation is opposite and the scroll timer should 
track the closed modal dialog which is not reliable because it is possible to 
press and hold  a scroll thumb on another scrollbar and it will detect the 
closed modal dialog too.

If I am correct for the first version of the fix the mouse release and exit 
events can be still missed if a modal dialog is shown outside the scroll bar.

I do not have a good idea how the mouse exit event can be caught in this 
case when a modal dialog is shown.
May be it possible to add a counter of mouse release events to the toolkit 
(or read it by AWTEventListener).
If number of mouse release events are different before the scroll bar  
adjustment listener is executed and after that it means that mouse was released 
on a modal dialog or missed by some other reason.

   Thanks,
   Alexandr.

>
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Rajeev Chamyal
> Sent: 27 December 2015 20:32
> To: Sergey Bylokhov
> Cc: swing-dev@openjdk.java.net
> Subject: Re:  Review request for JDK-8075084 
> JOptionPane.showMessageDialog causes JScrollBar to move
>
> Hello Sergey,
>
> The first webrev version is implemented on similar lines as you suggested but 
> it had issues as pointed my Alexandr in his review below.
> If the mouse pointer after clicking on modal dialog close lies outside the 
> parent window then parent window is not getting any mouse events and 
> BasicScrollBarUI::scrollByUnit method is again getting called recursively, 
> Because of which the scroll bar pointer keeps on moving till the end of 
> scrollbar.
> With this new implementation these issue are not seen.
>
> Regards,
> Rajeev Chamyal
>
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: 25 December 2015 21:52
> To: Rajeev Chamyal; Alexander Scherbatiy
> Cc: Prasanta Sadhukhan; swing-dev@openjdk.java.net
> Subject: Re: Review request for JDK-8075084 
> JOptionPane.showMessageDialog causes JScrollBar to move
>
> Probably this bug can be fixed in a different way. Is it possible to check 
> the state of the scroll bar in the timer? And if in some iteration the button 
> became unpressed then stops itself(timer).
>
> On 23/12/15 12:29, Rajeev Chamyal wrote:
>> Hello Alexandr,
>>
>> The modal dialog can be application modal, document modal and toolkit modal.
>>1) Application-modal dialog box blocks all windows from the same 
>> application, except windows from its child hierarchy
>>2) Document-modal dialog box blocks all windows from the same 
>> document, except windows from its child hierarchy.
>>3) Toolkit-modal dialog box blocks all windows that run in the 
>> same toolkit, except windows from its child hierarchy
>>
>> The current issue is reproducible with all modal dialog types. I have 
>> updated the condition in code to check for modal dialogs.
>>
>> http://cr.openjdk.java.net/~rchamyal/8075084/webrev.02/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Alexander Scherbatiy
>> Sent: 22 December 2015 05:13
>> To: Rajeev Chamyal
>> Cc: Sergey Bylokhov; Prasanta Sadhukhan; swing-dev@openjdk.java.net
>> Subject: Re: Review request for JDK-8075084 
>> JOptionPane.showMessageDialog causes JScrollBar to move
>>
>> On 21/12/15 12:21, Rajeev Chamyal wrote:
>>> Hello Alexandr,
>>>
>>> I have updated the fix. Please review it.
>>> http://cr.openjdk.java.net/~rchamyal/8075084/webrev.01/
>>   When a modal dialog is shown does it block all windows or is it 
>> possible that a modal dialog blocks some windows and does not block others?
>>
>> Thanks,
>> Alexandr.
>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -Original Message-
>>> From: Alexander Scherbatiy
>>> Sent: 10 December 2015 16:59
>>> To: Rajeev Chamyal
>>> Cc: Sergey Bylokhov; Prasanta Sadhukhan; swing-dev@openjdk.java.net
>>> Subject: Re: Review request for JDK-8075084 
>>> JOptionPane.showMessageDialog causes JScrollBar to move

Re: Review Request for 6439354 : Win L&F: TitledBorder colors are not from desktop

2016-03-22 Thread Prem Balakrishnan 
Hi Sergey,

Updated test as per the review comments.

Webrev: http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.01/ 

Regards,
Prem


-Original Message-
From: Sergey Bylokhov 
Sent: Monday, March 21, 2016 7:31 PM
To: Prem Balakrishnan; Semyon Sadetsky; Rajeev Chamyal; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re: Review Request for 6439354 : Win L&F: TitledBorder colors are not 
from desktop

Hi, Prem.
In the test the Swing components accessed on non-EDT thread. Probably the test 
can be simplified further?

On 21.03.16 11:20, Prem Balakrishnan wrote:
> Hi*,*
>
> Please review fix for JDK 9,
>
> *Bug: *https://bugs.openjdk.java.net/browse/JDK-6439354 **
>
> *Webrev: *http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.00/
>
> *Issue:*
>
> Win L&F: TitledBorder colors are not from desktop
>
> *Cause:*
>
> "TitledBorder.border" is set to EtchedBorder with default 
> highlight/shadow colors.
>
> *Fix:*
>
> "TitledBorder.border" is set to  EtchedBorder with Desktop
>   highlight/shadow colors.
>
> **
>
> *Test: *Manual Test (since windows theme to be set)**
>
> **
>
> Regards,
> Prem
>


--
Best regards, Sergey.

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

2016-03-22 Thread Rajeev Chamyal 
Hello All,

Gentle reminder.
Please review the fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8150225 
Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/ 

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal 
Sent: 09 March 2016 15:58
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hello Sergey,

I have run JCK tests for HTMLWriter and AbstractWriter with this fix and all 
passed.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov
Sent: 09 March 2016 15:54
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hi, Rajeev.
Please confirm that there are no new issues in the jck after this fix.

On 09.03.16 12:18, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the following fix for Jdk9:
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225
>
> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/
> 
>
> Issue : JCK conformance test failed due to fix for bug JDK-7104635
>
> Fix: Reverted the fix for JDK-7104635 and added a new method in 
> HTMLWriter.java to check if P tag is within Pre tag.
>
> Decrement indentation is skipped if P tag is with a Pre tag.
>
> Regards,
>
> Rajeev Chamyal
>


--
Best regards, Sergey.

Re: Review Request of 8151282: [TEST_BUG] javax/swing/JInternalFrame/8146321/JInternalFrameIconTest.java fails with GTK LnF

2016-03-22 Thread Avik Niyogi 
Hi All,
Please find webrev with new code fix:
http://cr.openjdk.java.net/~aniyogi/8151282/webrev.02/ 


With Regards,
Avik Niyogi

> On 21-Mar-2016, at 8:07 pm, Sergey Bylokhov  
> wrote:
> 
> Hi, Avik.
> How this test can fail? I do not see any raised exceptions.
> 
> On 21.03.16 9:46, Avik Niyogi wrote:
>> Hi Sergey,
>> Please review the following code change.
>> With Regards,
>> Avik Niyogi
>>> On 19-Mar-2016, at 2:15 am, Alexander Scherbatiy
>>> mailto:alexandr.scherba...@oracle.com>
>>> >> >> wrote:
>>> 
>>> The fix looks good to me.
>>> 
>>> Thanks,
>>> Alexandr.
>>> 
>>> On 14/03/16 16:25, Avik Niyogi wrote:
 Hi All,
 Please review code changes made as per inputs provided.
 
 http://cr.openjdk.java.net/~aniyogi/8151282/webrev.01/ 
 
 >
 With Regards,
 Avik Niyogi
> On 14-Mar-2016, at 10:53 am, Avik Niyogi
> < >avik.niy...@oracle.com 
> > wrote:
> 
> Hi Sergey,
> Seems like it is a delay issue. Will submit test case with a
> waitForIdle() fix.
> 
> With Regards,
> Avik Niyogi
>> On 10-Mar-2016, at 7:22 pm, Sergey Bylokhov
>> mailto:sergey.bylok...@oracle.com> 
>> >> 
>> wrote:
>> 
>> Hi, Avik.
>> A few questions.
>> - Why webrev contains the new file?
>> - You marked the test as a mac specific but it is iterates over a
>> bunch of l&fs. It seems it should not be OS specific, but should
>> check some specific l&fs (which support such icons): Metal, Nimbus,
>> Aqua, Windows(???). So gtk/motif should be skipped.
>> 
>> On 08.03.16 8:10, Avik Niyogi wrote:
>>> Hi All,
>>> 
>>> Kindly review the bug fix for JDK 9.
>>> 
>>> *Bug:*
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8151282
>>> 
>>> *Webrev:*
>>> 
>>> _http://cr.openjdk.java.net/~aniyogi/8151282/webrev.00/_
>>> _
>>> _
>>> *Issue:*
>>> The test case failed for GTK LAF.
>>> 
>>> *Cause:*
>>> The test case was meant to be Mac specific and was missing a jtreg
>>> parameter
>>> 
>>> *Fix:*
>>> Minor change to test case with @requires tag added to set the fix.
>>> 
>>> With Regards,
>>> Avik Niyogi
>> 
>> 
>> --
>> Best regards, Sergey.
> 
 
>>> 
>> 
> 
> 
> -- 
> Best regards, Sergey.