Re: [9] Review request for 8143914: Provide Mac-specific fullscreen support

[Alexander Zvegintsev]

Please see the updated webrev:

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/02/


Thanks,
Alexander.

On 10/11/16 5:00 PM, Alexander Zvegintsev wrote:

Hi Sergey,

Please see the updated webrev:

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/01/

As I can observe other windows  such as native dialogs doesn't have 
fullscreen button.



On 10/2/16 5:52 PM, Sergey Bylokhov wrote:

Hi, Alexander.
In the fix you use eawt events which are generated by the same peer. 
Why not use windowWill/DidEnterFullScreen + 
windowWill/DidExitFullScreen?
Why the fix is for the Frames only, there are some limitations for 
other windows, dialogs etc?


On 06.09.16 17:32, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/00/

for the issue

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


This fix adds the green FullScreen button to a resizable frames by 
default.


Previous maximize behavior is still accessible by holding Alt while
clicking on the green button.

setResizable is fixed because of two reasons:

Window will not be able to leave fullscreen if we remove resizable from
style bits(and even if we set it again)

Window will not have the green button(maximize/zoom or fullscreen) if
the window was initially not resizable.

Re: [9] Review request for 8166594: Taskbar.setWindowProgressValue() spec does not specify expected visual behavior of setWindowProgressValue()

[Alexander Zvegintsev]

Updated the "behavior is undefined" section:

http://cr.openjdk.java.net/~azvegint/jdk/9/8166594/01/

Thanks,
Alexander.

On 10/6/16 6:14 PM, Semyon Sadetsky wrote:

On 10/6/2016 3:38 PM, Alexander Zvegintsev wrote:


Hi Semyon,

Yes it is, Microsoft defined some set of rules for such case[0].

However it looks redundant and too implementation-specific(which can 
be changed) for me.
I didn't mean to specify the behavior for Windows platform. It seems 
that any desktop platform may have its own rules for this case. 
"Unspecified  behavior" just sounds a bit like a warning to me. Maybe 
it worth to write that this behavior is a platform specific, or don't 
mention the scenario at all since those rules cannot be changed by JDK 
anyway. But it is just a recommendation, the decision is up to you.


Reformatted for 80 chars in place.

Thanks. Looks good then.

--Semyon



[0] 
https://msdn.microsoft.com/en-us/library/windows/desktop/dd391698(v=vs.85).aspx#How_the_Taskbar_Button_Chooses_the_Progress_Indicator_for_a_Group



On 10/6/16 9:23 AM, Semyon Sadetsky wrote:

Hi Alexander,

416  * Note that the behavior is undefined when multiple windows 
is grouped in the task area.


Isn't the above a some kind of simplification?

Could you reformat changed lines to make them following the 80 chars 
maximum?


--Semyon


On 06.10.2016 04:56, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8166594/00/

for the issue

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


It also fixes two issues to conform the documentation:

setting value below 0 doesn't disable progress indication

switching from indeterminate to normal state has no effect

Re: [9] Review request for 8166897: Some font overlap in the Optionpane dialog.

[Alexander Zvegintsev]

+1

Thanks,
Alexander.

On 10/13/16 12:12 AM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8166897

webrev: http://cr.openjdk.java.net/~ssadetsky/8166897/webrev.00/

The setResizable() method of XDecoratedPeer class clears the frame 
insets to allow them to receive the updated size. In some situations 
clearing insets during establishing the frame dimensions may break the 
new frame dimensions algorithm for Unity and the frame content 
positioning becomes wrong.


Since in this new algorithm the insets may be arbitrary updated by WM 
it doesn't necessary to clear insets to get them the new size. The 
proposed solution is to avoid to force frame insets to zero when the 
frame is made resizable or non-resizable by user.


--Semyon

Re: [9] Review Request: 8143077 Deprecate InputEvent._MASK in favor of InputEvent._DOWN_MASK

[Alexander Zvegintsev]

Looks good.


On 10/7/16 4:21 PM, Sergey Bylokhov wrote:

On 07.10.16 10:06, Semyon Sadetsky wrote:

Hi Sergey,

After applying the patch I found 72 usages of the Event class. Why they
are not replaced?


By the same reason why InputEvent.getModifiers() was not replaced by 
InputEvent.getModifiersEx():


>>> We can replace old constants by the new one in the whole jdk, 
but I

>>> updated only the code in InputEvent to make change smaller and
>>> safer. So at least the new code in jdk and the code in 
applications

>>> will start to use the new constants.



On 10/2/2016 4:53 PM, Sergey Bylokhov wrote:

Thanks for the comments.
The new version:
http://cr.openjdk.java.net/~serb/8143077/webrev.01
The specification of Event class and InputEvent.getModifiers() are
updated.

On 30.09.16 19:08, Jonathan Bluett-Duncan wrote:

Hi Sergey,

I'm not a reviewer, but after reading the deprecation messages in
Event.java

* @deprecated It is recommended that {@code AWTEvent} class and 
its

subclasses
* be used instead.


I get the impression they would read better without the redundant
"class" in the middle, like so.

* @deprecated It is recommended that {@code AWTEvent} and its
subclasses
* be used instead.


Kind regards,
Jonathan


On 30 September 2016 at 16:45, Sergey Bylokhov
> 
wrote:


Hello.

Please review the fix for jdk9.

This is request to deprecate the obsolete flags inside InputEvent.
This constants were marked as obsolete in jdk1.4 and were replaced
by the new one. In jdk1.4 the documentation were update with 
notion

that the new constants should be used. And this bug is about
official deprecation of them.

We can replace old constants by the new one in the whole jdk, 
but I

updated only the code in InputEvent to make change smaller and
safer. So at least the new code in jdk and the code in 
applications

will start to use the new constants.

The changes in jconsole are necessary to fix deprecation warning.

jprt build passed, no new issues were found by jck/jtreg tests.


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

Webrev can be found at:
http://cr.openjdk.java.net/~serb/8143077/webrev.00


--
Best regards, Sergey.












--
Thanks,
Alexander.

[9] Review request for 8167565: [macosx] Maximization of a dialog hides it

[Alexander Zvegintsev]

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8167565/00/

for the issue

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

We trying to zoom to a zero size window(standardFrame has zero size). 
standardFrame initialization happens on setMaximizedBounds call.
Dialog doesn't initialize it(Frames has setMaximizedBounds() call in 
Frame.addNotify).
I prefer to fix it in the native level, if you have other thoughts 
please share it.


--
Thanks,
Alexander.

Re: [9] Review request for 8143914: Provide Mac-specific fullscreen support

[Alexander Zvegintsev]

Actually I can't reproduce such behavior.


On 9/30/16 5:12 PM, Semyon Sadetsky wrote:

The fix looks good.

Do you know why JDialog maximize button doesn't work? When I click on 
it the maximize button the dialog disappears.


--Semyon


On 9/6/2016 5:32 PM, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/00/

for the issue

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


This fix adds the green FullScreen button to a resizable frames by 
default.


Previous maximize behavior is still accessible by holding Alt while 
clicking on the green button.


setResizable is fixed because of two reasons:

Window will not be able to leave fullscreen if we remove resizable 
from style bits(and even if we set it again)


Window will not have the green button(maximize/zoom or fullscreen) if 
the window was initially not resizable.







--
Thanks,
Alexander.

Re: RFR: JDK-8165232 XKeycodeToKeysym is deprecated and should be replaced with XkbKeycodeToKeysym

[Alexander Zvegintsev]

+1

P.S. It wasn't hard to find, but the webrev is updated at the first 
revision http://cr.openjdk.java.net/~alanbur/JDK-8165232


--
Thanks,
Alexander.

On 06.10.2016 13:18, Alan Burlison wrote:

On 04/10/2016 19:34, Alan Burlison wrote:


key_syms is not freed.


So there is, thanks for the catch.


Done, webrev updated, jprt -tset hostpot rerun & clean.

Re: [9] Review request for 8166594: Taskbar.setWindowProgressValue() spec does not specify expected visual behavior of setWindowProgressValue()

[Alexander Zvegintsev]

Hi Semyon,

Yes it is, Microsoft defined some set of rules for such case[0].

However it looks redundant and too implementation-specific(which can be 
changed) for me.


Reformatted for 80 chars in place.


[0] 
https://msdn.microsoft.com/en-us/library/windows/desktop/dd391698(v=vs.85).aspx#How_the_Taskbar_Button_Chooses_the_Progress_Indicator_for_a_Group



On 10/6/16 9:23 AM, Semyon Sadetsky wrote:

Hi Alexander,

416  * Note that the behavior is undefined when multiple windows 
is grouped in the task area.


Isn't the above a some kind of simplification?

Could you reformat changed lines to make them following the 80 chars 
maximum?


--Semyon


On 06.10.2016 04:56, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8166594/00/

for the issue

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


It also fixes two issues to conform the documentation:

setting value below 0 doesn't disable progress indication

switching from indeterminate to normal state has no effect





--
Thanks,
Alexander.

Re: [9] Review request for 8166942: Usage of SplashScreen Functions hangs up FX Application

[Alexander Zvegintsev]

You are right, it it undefined behavior.


On 10/6/16 10:02 AM, Semyon Sadetsky wrote:

Hi Alexander,

Is it safe to lock the mutex on one thread and unlock it on another?

--Semyon


On 06.10.2016 06:08, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8166942/00/

for the issue

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

SplashEventLoop() acquires the lock[0] and then trying to call 
SplashRedrawWindow() which is trying execute some code on the main 
thread[1]. Thus if we call SplashLock() from the main thread between 
[0] and SplashRedrawWindow() calls then we are in deadlock. Main 
thread is waiting for mutex release, other thread waiting for finish 
of code execution on the main thread. It is our case [2].


The proposed solution is to acquire the lock on the main thread.

[0] 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/5518ac2f2ead/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m#l359
[1] 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/5518ac2f2ead/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m#l262
[2] 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/5518ac2f2ead/src/java.desktop/share/native/libsplashscreen/java_awt_SplashScreen.c#l57






--
Thanks,
Alexander.

[9] Review request for 8166942: Usage of SplashScreen Functions hangs up FX Application

[Alexander Zvegintsev]

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8166942/00/

for the issue

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

SplashEventLoop() acquires the lock[0] and then trying to call 
SplashRedrawWindow() which is trying execute some code on the main 
thread[1]. Thus if we call SplashLock() from the main thread between [0] 
and SplashRedrawWindow() calls then we are in deadlock. Main thread is 
waiting for mutex release, other thread waiting for finish of code 
execution on the main thread. It is our case [2].


The proposed solution is to acquire the lock on the main thread.

[0] 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/5518ac2f2ead/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m#l359
[1] 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/5518ac2f2ead/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m#l262
[2] 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/5518ac2f2ead/src/java.desktop/share/native/libsplashscreen/java_awt_SplashScreen.c#l57


--
Thanks,
Alexander.

[9] Review request for 8166594: Taskbar.setWindowProgressValue() spec does not specify expected visual behavior of setWindowProgressValue()

[Alexander Zvegintsev]

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8166594/00/

for the issue

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


It also fixes two issues to conform the documentation:

setting value below 0 doesn't disable progress indication

switching from indeterminate to normal state has no effect

--
Thanks,
Alexander.

Re: RFR: JDK-8165232 XKeycodeToKeysym is deprecated and should be replaced with XkbKeycodeToKeysym

[Alexander Zvegintsev]

Hello,

as I can see there is a memory leak in case when the index is greater 
than or equal to num_syms:


837 KeySym *key_syms = XGetKeyboardMapping(display, keycode, 1, _syms);
838 if (index >= num_syms) {
839 return NoSymbol;
840 }

key_syms is not freed.

--
Thanks,
Alexander.


On 10/4/16 1:06 PM, Alan Burlison wrote:

On 03/10/2016 22:08, Phil Race wrote:


Do we have a 2nd reviewer yet ? I have not seen it.


Gah you are right, I thought I'd seen a 2nd. Any takers?

Re: [9] Review Request for 8141528: Test closed/java/awt/FullScreen/DisplayMode/CycleDMImage.java fails for Ubuntu 15.10

[Alexander Zvegintsev]

Looks fine.

--
Thanks,
Alexander.

On 10/03/2016 07:38 PM, Semyon Sadetsky wrote:
The fix is updated 
http://cr.openjdk.java.net/~ssadetsky/8141528/webrev.01/


The root case of the issue is that in Unity the screen window size and 
location are get updated several times after display mode is changed, 
and bounds which are set in line 459 of X11GraphicsDevice class are 
overwritten. Since those window updates caused by display mode change 
are not predictable the solution is changed to set the correct 
full-screen size in the ConfigureNotify handler.


--Semyon


On 12/7/2015 2:11 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

Compiz WM requires override redirect for full-screen window to 
preserve the full size upon display mode switching.


--Semyon

Re: [9] Review request for 8159432: [PIT][macosx] StackOverflow in closed/java/awt/Dialog/DialogDeadlock/DialogDeadlockTest

[Alexander Zvegintsev]

looks good to me.

--
Thanks,
Alexander.

On 24.08.2016 22:18, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8159432

webrev: http://cr.openjdk.java.net/~ssadetsky/8159432/webrev.00/

The issue is connected to the restoring focus to the previously 
focused window when there are a lot of focus restore requests are 
coming very often (for example series of windows showing and hiding 
quickly). In this case waiting for asynchronous focus causes nested 
waiting loop, and if number of such requests is big enough the 
StackOverflowError is thrown.


To avoid this the 8139218 solution is revisited to manage the focus 
restore synchronously only if it is possible and send a single 
asynchronous focus request otherwise.


--Semyon

Re: [9] Review request for 8154434: Open the request focus methods of the java.awt.Component which accept FocusEvent.Cause

[Alexander Zvegintsev]

Looks good to me.

--
Thanks,
Alexander.

On 26.04.2016 16:14, Semyon Sadetsky wrote:



On 4/26/2016 3:49 PM, Philip Race wrote:

> In applications one may need to know the reason why focus is requested

I do not mean why should apps want to *know* the cause.

I am asking why apps should be able to *specify* the cause as
proposed by this change.

Being "symmetric" is not a sufficient reason.

I just clarified that there are no a lot of needs for apps here.
But if a framework is developed it may require to develop a custom 
implementation of KeyboardFocusManager, for example. Such KFM may 
simply override any logic that uses focus cause.

Just from general consideration: JFC have been made open for extensions.

--Semyon


-phil.

On 4/26/16, 12:27 AM, Semyon Sadetsky wrote:

On 4/25/2016 10:33 PM, Phil Race wrote:
You will need to convince me of the appropriateness of opening 
these methods.

It seems to me that are for the focus system, not for applications.

Why should an application be allowed to say "I would like component 
X" to
receive focus and tell it the reason is a mouse event" when in fact 
it is nothing of the kind.
This is a continuation of the 8080395. I think it would be mostly 
interesting for framework developers not for applications.
In applications one may need to know the reason why focus is 
requested to the component before the focus is set to it.
As I heard from Anton (that was his task initially) opening the 
cause was requested by some client, but, unfortunately, I don't have 
any references.


--Semyon


I would like to hear from others if they see a valid use case and 
that there are no

down-sides to mis-use.

-phil.

On 04/19/2016 02:30 AM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

To support the new FocusEvent cause concept introduced by 
JDK-8080395 the next package-private methods of the 
java.awt.Component class are opened in the fix:


boolean requestFocus(FocusEvent.Cause cause)  -> public void 
requestFocus(FocusEvent.Cause cause)


boolean requestFocus(boolean temporary, FocusEvent.Cause cause) -> 
protected boolean requestFocus(boolean temporary, FocusEvent.Cause 
cause)


boolean requestFocusInWindow(FocusEvent.Cause cause) -> public 
boolean requestFocusInWindow(FocusEvent.Cause cause)


The methods are changed to be symmetric with the focus request 
methods of the same class which do not accept the cause parameter.
The method requestFocus(FocusEvent.Cause cause) was changed to 
return no value similarly to the requestFocus() because the 
returning boolean true cannot guarantee the focus gain.


--Semyon

Re: [9] Review request for 8165619: Frame is not repainted if created in state=MAXIMIZED_BOTH on Unity

[Alexander Zvegintsev]

Looks good to me.

--
Thanks,
Alexander.

On 23.09.2016 16:15, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8165619

webrev: http://cr.openjdk.java.net/~ssadetsky/8165619/webrev.00/

If frame is created in maximized the window extent property 
notification is not send by WM. So, ConfigureNotify event is skipped 
and the target frame doesn't receive notification about size and its 
content is not repainted.  To handle this scenario the waiting for 
window extents is canceled for maximized window and the extents are 
specified when window state is changed back to normal.


--Semyon

Re: [9] Review Request for 8041519: JVM crash on second DnD after modal dialog opened during Swing DnD operation

[Alexander Zvegintsev]

Looks good to me.


On 7/17/15 7:27 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:
bug: https://bugs.openjdk.java.net/browse/JDK-8041519
webrev: http://cr.openjdk.java.net/~ssadetsky/8041519/webrev.00/

This is regression from JDK-6342381. When a modal dialog blocks drop 
operation in progress state consequent mouse events clear the drop 
context variables (this clearing was introduced by JDK-6342381). The 
solution is to avoid clearing if drop operation is in progress.
Test scenario for this case would be too hard and unstable. Since the 
fix is pretty simple I left it as noreg.


--Semyon




--
Thanks,
Alexander.

Re: [9] Review request for 8161910: [PIT] regression: HW/LW mixing seems broken on Unity

[Alexander Zvegintsev]

Looks good to me.


On 8/30/16 7:18 PM, Semyon Sadetsky wrote:

Sorry for inconvenience, but I have to update this fix once again.

http://cr.openjdk.java.net/~ssadetsky/8161910/webrev.02

I have found the main root cause of the issue. In the 8036815 I have 
missed one insets_corrected = true; statement  in the RaparentNotify 
handler. In my new Unity algo for establishing frame dimensions the 
insets correction should be postponed until the frame extent property 
is set. This is corrected in the updated fix.


Also the frame insets correction optimization is restored with only 
one exclusion when the client area is not set to ensure that the 
target frame will receive the final size and location update.


--Semyon


On 8/24/2016 4:24 PM, Semyon Sadetsky wrote:
The fix was amended to 
http://cr.openjdk.java.net/~ssadetsky/8161910/webrev.01/


Because Unity WM may change initial window location (for example when 
window overlaps desktop bars) I removed the optimization that skipped 
frame bounds revalidation in case of the initial frame insets was 
correct. This should guaranty the ConfigureNotify event always to 
come after the extent size event.


--Semyon

On 8/22/2016 9:19 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8161910

webrev: http://cr.openjdk.java.net/~ssadetsky/8161910/webrev.00/

A regression of the 8036915 in which a new frame dimensioning algo 
was introduced for the Unity WM because the latter differs from 
other WMs a lot.  The algo has a flaw in some scenarios when the 
extent size event is delayed while the initial frame insets were 
established correctly, therefore the frame dimensions do not require 
any tuning. In this case the content window need to be notified 
right in the extent event handler to let the content to receive its 
new dimensions because subsequent XConfigureNotify event may be 
omitted.


--Semyon








--
Thanks,
Alexander.

Re: [9] Review Request: 8004693 TEST_BUG: java/awt/KeyboardFocusmanager/DefaultPolicyChange/DefaultPolicyChange_Swing.java fails

[Alexander Zvegintsev]

Looks good to me.


On 5/30/16 7:39 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

The test DefaultPolicyChange_Swing.java has two issues:
 - It uses invokeLater(), so the test usually pass before the code is 
executed on the EDT, because the main thread completes before.
 - The test fetches the FocusTraversalPolicy from the current 
KeyboardFocusManager. But default FocusTraversalPolicy can be changed 
during the Swing initialization(JDK-7125044). The test should save the 
state before setDefaultFocusTraversalPolicy() but after the Swing 
initialization, and validate that the FocusTraversalPolicy was not 
changed for windows which were already shown.


The fix proposed in the CR is applied + small cleanup(regtesthelpers 
removed and InvokeAndWait is used instead of InvokeLater+realSync)


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




--
Thanks,
Alexander.

[9] Review request for 8140311: SwingInterop crashes at window close

[Alexander Zvegintsev]

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8140311/00/

for the issue

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

This issue is only reproducible when a window using CWarningWindow.
After window close we cancel HidingTask in CWarningWindow at java level, 
however we do not cancel delayed execution of "perform" selector at 
native level.
At this point cached JNIEnv value becomes invalid, which is leading to 
crash.


--
Thanks,
Alexander.

Re: [9] Review Request for 8141528: Test closed/java/awt/FullScreen/DisplayMode/CycleDMImage.java fails for Ubuntu 15.10

[Alexander Zvegintsev]

Hi Semyon,

I believe that this webrev should be updated to use isUnityCompiz().

--
Thanks,
Alexander.

On 07.12.2015 14:11, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

Compiz WM requires override redirect for full-screen window to 
preserve the full size upon display mode switching.


--Semyon

Re: [9] Review request for 8164536: enableSuddenTermination() - Not throws SecurityException if a security manager exists and it will not allow the caller to invoke System.exit

[Alexander Zvegintsev]

Initially showWindowWithoutWarningBanner was added to deny the new API 
functions invocation without permission check,


but now it seems redundant, so I left only canProcessApplicationEvents 
in the new webrev:


cr.openjdk.java.net/~azvegint/jdk/9/8164536/02/

--
Thanks,
Alexander.

On 09/08/2016 04:53 PM, Sergey Bylokhov wrote:

On 31.08.16 3:27, Alexander Zvegintsev wrote:

Hi Sergey,

It could be, but actually RuntimePermission is used by eawt(sure we can
change it too, but I don't see a point)


It is still unclear to me why in most of the place we check two 
permissions? Can you please clarify what is the purpose of 
canProcessApplicationEvents and showWindowWithoutWarningBanner and why 
both are related for example to removeAppEventListener() method.




Please see the updated webrev:

http://cr.openjdk.java.net/~azvegint/jdk/9/8164536/01/

--
Thanks,
Alexander.

On 30.08.2016 22:44, Sergey Bylokhov wrote:

Hi, Alexander.
Probably the name of this permission can be added to 
AWTPermissions.java?

I am not sure why some of the methods require
"canProcessApplicationEvents" permission. For example
Taskbar.getTaskbar()? I guess Desktop.getDesktop() is used a template
but it does not throw such exceptions and/or require similar 
permissions.


On 30.08.16 21:58, Alexander Zvegintsev wrote:

Hello,

Please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8164536/00/

for the issue

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

This fix add check for canProcessApplicationEvents runtime
permission(currently used by eawt) for Desktop and Taskbar classes.

Re: [9] Review request for 8153526: [Unity] Taskbar.getTaskbar().setMenu(null) doesn't remove menu

[Alexander Zvegintsev]

Hi Sergey,

It looks like unity works only with first assigned quicklist 
(unity_launcher_entry_set_quicklist). Assigning a new one has no effect.


--
Thanks,
Alexander.

On 09/08/2016 05:02 PM, Sergey Bylokhov wrote:

Hi, Alexander.
Please add some small evaluation of the bug to the CR.

On 06.09.16 20:56, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8153526/00/

for the issue

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

[9] Review request for 8153526: [Unity] Taskbar.getTaskbar().setMenu(null) doesn't remove menu

[Alexander Zvegintsev]

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8153526/00/

for the issue

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



--
Thanks,
Alexander.

[9] Review request for 8143914: Provide Mac-specific fullscreen support

[Alexander Zvegintsev]

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/00/

for the issue

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


This fix adds the green FullScreen button to a resizable frames by default.

Previous maximize behavior is still accessible by holding Alt while 
clicking on the green button.


setResizable is fixed because of two reasons:

Window will not be able to leave fullscreen if we remove resizable from 
style bits(and even if we set it again)


Window will not have the green button(maximize/zoom or fullscreen) if 
the window was initially not resizable.



--
Thanks,
Alexander.

Re: [9] Review request for 8164536: enableSuddenTermination() - Not throws SecurityException if a security manager exists and it will not allow the caller to invoke System.exit

[Alexander Zvegintsev]

Hi Sergey,

It could be, but actually RuntimePermission is used by eawt(sure we can 
change it too, but I don't see a point)


Please see the updated webrev:

http://cr.openjdk.java.net/~azvegint/jdk/9/8164536/01/

--
Thanks,
Alexander.

On 30.08.2016 22:44, Sergey Bylokhov wrote:

Hi, Alexander.
Probably the name of this permission can be added to AWTPermissions.java?
I am not sure why some of the methods require 
"canProcessApplicationEvents" permission. For example 
Taskbar.getTaskbar()? I guess Desktop.getDesktop() is used a template 
but it does not throw such exceptions and/or require similar permissions.


On 30.08.16 21:58, Alexander Zvegintsev wrote:

Hello,

Please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8164536/00/

for the issue

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

This fix add check for canProcessApplicationEvents runtime
permission(currently used by eawt) for Desktop and Taskbar classes.

[9] Review request for 8164536: enableSuddenTermination() - Not throws SecurityException if a security manager exists and it will not allow the caller to invoke System.exit

[Alexander Zvegintsev]

Hello,

Please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8164536/00/

for the issue

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

This fix add check for canProcessApplicationEvents runtime 
permission(currently used by eawt) for Desktop and Taskbar classes.



--
Thanks,
Alexander.

Re: [9] Review request for 8162840: Desktop. enableSuddenTermination() has no effect

[Alexander Zvegintsev]

Sure, but it is already created by Yuri and attached to the issue.

I can include it with this fix, however as far I know SQE has a separate 
task to cover this JEP with tests and this test is a part of this task.



On 8/16/16 7:27 PM, Sergey Bylokhov wrote:

Hi, Alexander.
Is it possible to create a test for the fix?

On 16.08.16 19:20, Alexander Zvegintsev wrote:

Hello,


please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8162840/00/

for the issue

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


We don't call QuitHandler and using default QuitStrategy if sudden
termination is enabled.







--
Thanks,
Alexander.

[9] Review request for 8162840: Desktop. enableSuddenTermination() has no effect

[Alexander Zvegintsev]

Hello,


please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8162840/00/

for the issue

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


We don't call QuitHandler and using default QuitStrategy if sudden 
termination is enabled.



--
Thanks,
Alexander.

Re: [9] Review request for 8161007: GPL header missing comma in year - not swapped in licensee bundles

[Alexander Zvegintsev]

+1


On 7/19/16 2:28 PM, Alexandr Scherbatiy wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 7/15/2016 2:46 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8161007

webrev: http://cr.openjdk.java.net/~ssadetsky/8161007/webrev.00/

--Semyon






--
Thanks,
Alexander.

Re: [9] Review Request for 8036915: setLocationRelativeTo stopped working in Ubuntu 13.10 (Unity)

[Alexander Zvegintsev]

Hi Semyon,

The fix looks good to me.

Thanks,

Alexander.

On 07/13/2016 01:28 PM, Semyon Sadetsky wrote:
Please review an updated version of the fix: 
http://cr.openjdk.java.net/~ssadetsky/8036915/webrev.02/


The solution was completely changed. The frame insets correction 
algorithm is revised for Unity WM since it is defers from other WMs. 
It seems the safest way to fix this issue and to avoid compatibility 
issues.


--Semyon


On 3/24/2016 6:28 PM, Alexander Scherbatiy wrote:

On 18/02/16 14:26, Semyon Sadetsky wrote:

I have found the way to avoid iterative queries for frame insets.
The updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8036915/webrev.01/

- 269 if (wm_set_insets != null && !isNull(wm_set_insets))
isNull(Insets i) function checks the given argument to null. It seems 
that the first check is unnecessary in this case.


- Could not null insets with zero values be valid insets?

-  396 if (!isNull(correction)) {
Could this construction be changed to "if (isNull(correction)) { 
return; }" ?


-if (!insets_corrected && getDecorations() != 
XWindowAttributesData.AWT_DECOR_NONE) {

+if (getDecorations() != XWindowAttributesData.AWT_DECOR_NONE) {
handleReparentNotifyEvent() calls XWM.getWM().getInsets(this, 
xe.get_window(), xe.get_parent())
in case correctWM is null but handleConfigureNotifyEvent() is updated 
to do it even insets has been corrected.

What is the reason to handle these two cases differently?

- Could you run JCK with the provided fix to check that there are no 
regressions?


Thanks,
Alexandr.



--Semyon


On 10/29/2015 12:20 AM, Sergey Bylokhov wrote:

On 06.10.15 9:15, Semyon Sadetsky wrote:

Sorry. I meant it is not guaranteed to be sent by every WM.


But fetch the data of 100 times also doesn't guarantee that we 
will get the necessary data. It will be better at least to try to 
check the specified atom for this(it seems it is supported by all 
our platfrom). For example on osx insets can be changed on the fly 
w/o notification, because of this we fetch the insets on each 
reshapeEvent and posts the necessary message up to hierarchy.





On 10/6/2015 9:03 AM, Semyon Sadetsky wrote:

In is extended event. In does not guaranteed to be sent by any WM.

On 10/5/2015 6:12 PM, Sergey Bylokhov wrote:
Why we cannot treat such a XA_NET_FRAME_EXTENTS as a 
ConfigureNotify
in which only insets are changed, and the window bounds/insets 
should

be revalidated?

On 05.10.15 14:56, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

The test scenario attached to the jira contains potential errors
because
Componet.getLocation() won't return the location at any moment 
of time.
Anyway a window location issue exists in Unity. The root cause 
is that

the real insets sent with the XA_NET_FRAME_EXTENTS event can be
overridden by the "guessed" insets which are zero on Unity. So the
returned location is increased by real insets while the real 
window

location is correct.
Yet another issue I found in Unity is a window size issue which 
is also
caused by the frame insets detection. The Unity WM doesn't 
provide the
frame insets immediately and XA_NET_FRAME_EXTENTS event may 
come after

the ConfigureNotify event for the frame.
The solution proposes an adaptation of the existing frame insets
request
algorithm to the Unity WM so that makes it more stable.

--Semyon

Re: [9] Review request for 8159460: On Ubuntu Unity, problem with java/awt/Window/FindOwner/FindOwnerTest

[Alexander Zvegintsev]

Looks good to me.

--
Thanks,
Alexander.

On 17.06.2016 11:42, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8159460

webrev: http://cr.openjdk.java.net/~ssadetsky/8159460/webrev.00/

On Unity WM_TAKE_FOCUS notification is sent to the decorated parent 
window if its undecorated child window is clicked by mouse, after that 
ButtonPress notification is sent. Those notifications causes two 
parallel sequences of AWT events to gain focus to different windows. 
Since focus requests are asynchronous the resulting focus may by 
obtained randomly by the clicked child window or by its owner.


The fix solution is : in case of Unity WM, do not change the current 
focused window if the incoming WM_TAKE_FOCUS is addressed the current 
active window and the current focused window owned by the active 
window.  This solution should prevent racing focus requests on Unity.


--Semyon

Re: [9] Review request for 8153512: Taskbar support reported for Xfce4.

[Alexander Zvegintsev]

Hi Semyon, please see the updated webrev
http://cr.openjdk.java.net/~azvegint/jdk/9/8153512/01/

Thanks,

Alexander.

On 07/12/2016 05:22 PM, Semyon Sadetsky wrote:

Hi Alexander,

I would use a static variable and check it once only.

--Semyon

On 12.07.2016 16:56, Alexander Zvegintsev wrote:

Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8153512/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8153512

[9] Review request for 8153512: Taskbar support reported for Xfce4.

[Alexander Zvegintsev]

Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8153512/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8153512

--
Thanks,

Alexander.

Re: [9] Review request for 8149115: [hidpi] Linux: display-wise scaling factor should probably be taken into account

[Alexander Zvegintsev]

+1


On 7/8/16 10:37 AM, Alexandr Scherbatiy wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 7/8/2016 9:25 AM, Semyon Sadetsky wrote:
I have changed variables declaration style in systemScale.c, because 
it produced warnings in Solaris build.


http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.03/

--Semyon

On 7/7/2016 9:57 AM, Alexandr Scherbatiy wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 7/6/2016 11:46 PM, Alexander Zvegintsev wrote:

Still looks good.

--
Thanks,
Alexander.

On 06.07.2016 21:42, Semyon Sadetsky wrote:
Thanks, Alexander. Please see the updated webrev 
http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.02/


--Semyon


On 7/6/2016 9:03 PM, Alexander Zvegintsev wrote:

The fix looks good to me.

Just a minor comment: multiple NULL checks in get_schema_value 
could be wrapped in CHECK_NULL_RETURN macro, e.g.:


CHECH_NULL_RETURN(fp_g_settings_schema_has_key
= dlsym(lib_handle, "g_settings_schema_has_key"), NULL);
CHECH_NULL_RETURN(fp_g_settings_new_full
= dlsym(lib_handle, "g_settings_new_full"), NULL);


On 7/6/16 5:27 PM, Alexandr Scherbatiy wrote:


The fix looks good to me.

Thanks,
Alexandr.

On 7/6/2016 4:46 PM, Semyon Sadetsky wrote:

On 7/6/2016 12:26 PM, Alexandr Scherbatiy wrote:


On 7/5/2016 9:59 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8149115

webrev: http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.00/

Currently the hidpi support on linux only reads GDK_SCALE 
environment variable to get the native scale. Although, 
Gnome3 and Unity DE use own settings to control the interface 
scale. The fix adds possibility to read those settings to 
make java apps hidpi scale similar to the native apps. 
Currently only integer scale values are supported. If native 
scale is not an integer value it is rounded to the nearest 
integer.

  systemScale.c
   Should the while loop have a break statement in the 
getDesktopScale(...) method?
That's make sense. Please look at the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.01/


--Semyon



  Thanks,
  Alexandr.



--Semyon





















--
Thanks,
Alexander.

Re: [9] Review request for 8149115: [hidpi] Linux: display-wise scaling factor should probably be taken into account

[Alexander Zvegintsev]

Still looks good.

--
Thanks,
Alexander.

On 06.07.2016 21:42, Semyon Sadetsky wrote:
Thanks, Alexander. Please see the updated webrev 
http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.02/


--Semyon


On 7/6/2016 9:03 PM, Alexander Zvegintsev wrote:

The fix looks good to me.

Just a minor comment: multiple NULL checks in get_schema_value could 
be wrapped in CHECK_NULL_RETURN macro, e.g.:


CHECH_NULL_RETURN(fp_g_settings_schema_has_key
= dlsym(lib_handle, "g_settings_schema_has_key"), NULL);
CHECH_NULL_RETURN(fp_g_settings_new_full
= dlsym(lib_handle, "g_settings_new_full"), NULL);


On 7/6/16 5:27 PM, Alexandr Scherbatiy wrote:


The fix looks good to me.

Thanks,
Alexandr.

On 7/6/2016 4:46 PM, Semyon Sadetsky wrote:

On 7/6/2016 12:26 PM, Alexandr Scherbatiy wrote:


On 7/5/2016 9:59 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8149115

webrev: http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.00/

Currently the hidpi support on linux only reads GDK_SCALE 
environment variable to get the native scale. Although, Gnome3 
and Unity DE use own settings to control the interface scale. The 
fix adds possibility to read those settings to make java apps 
hidpi scale similar to the native apps. Currently only integer 
scale values are supported. If native scale is not an integer 
value it is rounded to the nearest integer.

  systemScale.c
   Should the while loop have a break statement in the 
getDesktopScale(...) method?
That's make sense. Please look at the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.01/


--Semyon



  Thanks,
  Alexandr.



--Semyon

Re: [9] Review request for 8149115: [hidpi] Linux: display-wise scaling factor should probably be taken into account

[Alexander Zvegintsev]

The fix looks good to me.

Just a minor comment: multiple NULL checks in get_schema_value could be 
wrapped in CHECK_NULL_RETURN macro, e.g.:


CHECH_NULL_RETURN(fp_g_settings_schema_has_key
= dlsym(lib_handle, "g_settings_schema_has_key"), NULL);
CHECH_NULL_RETURN(fp_g_settings_new_full
= dlsym(lib_handle, "g_settings_new_full"), NULL);


On 7/6/16 5:27 PM, Alexandr Scherbatiy wrote:


The fix looks good to me.

Thanks,
Alexandr.

On 7/6/2016 4:46 PM, Semyon Sadetsky wrote:

On 7/6/2016 12:26 PM, Alexandr Scherbatiy wrote:


On 7/5/2016 9:59 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8149115

webrev: http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.00/

Currently the hidpi support on linux only reads GDK_SCALE 
environment variable to get the native scale. Although, Gnome3 and 
Unity DE use own settings to control the interface scale. The fix 
adds possibility to read those settings to make java apps hidpi 
scale similar to the native apps. Currently only integer scale 
values are supported. If native scale is not an integer value it is 
rounded to the nearest integer.

  systemScale.c
   Should the while loop have a break statement in the 
getDesktopScale(...) method?
That's make sense. Please look at the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8149115/webrev.01/


--Semyon



  Thanks,
  Alexandr.



--Semyon









--
Thanks,
Alexander.

[9] Review request for 8159374 : Taskbar.setIconBadge() spec omits mention of exception for ICON_BADGE_TEXT feature

[Alexander Zvegintsev]

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8159374/00
for the issue
https://bugs.openjdk.java.net/browse/JDK-8159374

--
--
Thanks,
Alexander.

Re: CFV: New AWT Group Lead: Sergey Bylokhov

[Alexander Zvegintsev]

Vote: yes

--
Thanks,
Alexander.

On 10.06.2016 18:39, Artem Ananiev wrote:

Hi, AWT team,

I hereby nominate Sergey Bylokhov (OpenJDK user name: serb) to AWT 
Group Lead [1].


Sergey is a member of Java Client group at Oracle. He has been one of 
the most active contributors to AWT (and other Java client libs: 
Swing, Java2D, JavaFX, Java Sound, Java Beans) last few years and 
demonstrated very deep knowledge of the library, its architecture and 
implementation on various platforms.


Votes are due by June 24, 2016.

Only current AWT group members [2] are eligible to vote on this 
nomination. Votes must be cast in the open by replying to this mailing 
list.


For Two-Thirds Majority voting instructions, see [3]

[1] http://openjdk.java.net/bylaws#group-lead
[2] http://openjdk.java.net/census#awt
[3] http://openjdk.java.net/bylaws#two-thirds-majority

Thanks,

Artem

Re: Taking screenshots on x11 composite desktop produce wrong result

[Alexander Zvegintsev]

Looks fine to me.

Thanks,
Alexander.

On 31/05/16 17:29, Sergey Bylokhov wrote:

Hi, Mario.
Thanks for your contribution! there is tiny typo in the native: 
isGtkSupported should be useGtk. It will be good if someone run jprt 
job to confirm that the build is ok, since make file was changed.


On 31.05.16 13:45, Semyon Sadetsky wrote:



On 5/31/2016 1:36 PM, Mario Torre wrote:
2016-05-30 17:53 GMT+02:00 Semyon Sadetsky 
:



The rest is OK to me.

Great, thanks!

This is the final webrev:

http://cr.openjdk.java.net/~neugens/8150954/webrev.04/

Thanks a lot!


Assuming it's still OK for you, I believe I need another reviewer's OK
to push?

Yes, reviewing is not that fast currently. Probably azvegint could help,
he is an author of the GTK screenshoting code. I'll ping him.

--Semyon


Cheers,
Mario

Re: [8u/9] Review request 8156116 : [macosx] two JNI locals to delete in AWTWindow.m, CGraphicsEnv.m

[Alexander Zvegintsev]

Looks fine to me too.

Thanks,

Alexander.

On 05/17/2016 06:36 PM, Anton Tarasov wrote:

Would anyone else please review the fix?

Thanks
Anton.

On 5/5/2016 12:45 PM, Sergey Bylokhov wrote:

Looks fine.

On 05.05.16 10:50, Anton Tarasov wrote:

Hello,

Please, review a simple fix:

webrev: http://cr.openjdk.java.net/~ant/JDK-8156116/jdk9/webrev.0
bug: https://bugs.openjdk.java.net/browse/JDK-8156116

Thanks,
Anton.

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Fixed: http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/09/

--
Thanks,
Alexander.

On 03/05/2016 04:10 AM, Mandy Chung wrote:

On Mar 4, 2016, at 11:07 AM, Phil Race  wrote:

Approved.

-phil.

One comment on something you can't do anything about *yet*, is
that adding a new API package right in the same time frame jigsaw is
merging in to JDK9 creates a probable time window for a non-obvious conflict.

See 
http://cr.openjdk.java.net/~alanb/8142968/0/jdk/src/java.desktop/share/classes/module-info.java.html
where java.awt.desktop will need to be added to the list of exported packages.


I don’t see modules.xml updated from this webrev:


http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/08/

Please update /modules.xml to export java.awt.desktop if present (it’s an 
interim solution in JDK 9 that will be replaced with module-info.java as Phil points 
out).  When we merge jake with jdk9, we make appropriate changes based on the 
changeset in modules.xml.

Mandy

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

I think that this fine is at is.
Regarding permissions I filed the JDK-8151190 issue:
https://bugs.openjdk.java.net/browse/JDK-8151190 Revise permissions for 
java.awt.Desktop and java.awt.Taskbar


--
Thanks,
Alexander.

On 02/24/2016 08:12 PM, Sergey Bylokhov wrote:
Small question about moveToTrash(). Should we describe that the user 
can delete folders as well? Also I am not sure about permissions which 
we check. For example the usual File.delete() calls "SM.checkDelete()" 
but we check SM.checkWrite() for "<>". Moreover 
checkFileValidation() checks the read permission for the file as well. 
At the end it is an interesting question: what permissions are needed 
to delete the file.


On 21.02.16 14:38, Alexander Zvegintsev wrote:

Hi Phil,
please see updated the webrev here
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/08/
and comments inline

On 18.02.2016 0:10, Phil Race wrote:

Desktop.java looks fine now. Moving on to the other API classes

Top-level question :
Why are AppEvent and TaskBar in java.awt and not java.awt.desktop ?

Agreed on AppEvent, but Taskbar is placed in java.awt on purpose to make
it adjacent with Desktop and SystemTray.

And AppEvent seems to use a lot of nested classes.
If they were in java.awt.desktop then they could be top-level classes
without creating too much clutter. I don't think it matters that 'this
is how the com.apple APIs were done' - people need to recode anyway


AppEvent.java :
more wildcards ..
   28 import java.awt.desktop.*;

I notice the docs in this file (and perhaps more I have yet to see)
reference types without @code :-
eg :
  * Constructs a FilesEvent
* Constructs an OpenFilesEvent
..
  * Get the URI the app was asked to open
  146  * @return the URI

and more. Not essential I suppose but preferable.

   69 public List getFiles() {
   70 return files;
   71 }

Is there any need to return a copy for any of these ?

   95  * Gets the search term. The platform may additionally 
provide the search
   96  * term that was used to find the files. This is for 
example the case
   97  * on Mac OS X, when the files were opened using the 
Spotlight search

   98  * menu or a Finder search window.
   99  *
  100  * This is useful for highlighting the search term in 
the documents when

  101  * they are opened.
  102  * @return the search term used to find the files

Could we say "optionally" instead of "additionally"
and make clear that it may be "null"

a very long line :-

209  * Event sent when the application has become the foreground 
app, and when it has resigned being the foreground app.


And "resigned" is a voluntary term. If something else is placed into
the foreground the
this app may not get asked if it wants to allow that.
Maybe instead say "and when it is no longer the foreground app."


Done.

Also is there anything we can (or should) say about how some of these
events interact ?

eg, if a foreground app is hidden does only the appHidden listener get
called ?


Both listeners could be called, it's already mentioned here:

 * Called when the hidden app is shown again (but not necessarily
brought to
 * the foreground).
 *
 * @param e event
 */
public void appUnhidden(final AppHiddenEvent e);



BTW the term 'unhidden' makes sense if the app was previously 'hidden'
but less so
if the app is being newly shown.


  /**
  252  * The system does not provide a reason for a session 
change.

  253  */
  254 @Native
  255 public static final int REASON_UNSPECIFIED = 0x0;
  256

I think we should use an enum here.


Also unless there is a reason I have not yet come across, as many as
possible
of these classes should be marked 'final'


Taskbar.java :

One meta-concern I have here is that it seems to have a very blurry
distinction
with existing API :
https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html
https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html

Aren't both of these APIs dealing with the same area ?

Should we really add this new class or tweak system tray as needed ?

On Windows they dealing in the same area with taskbar, but on Mac OS
SystemTray and Taskbar(Dock) are separated. So I prefer to left it as 
it is.


/**
  112  * Stops displaying the progress.
  113  */
  114 @Native
  115 public static final int STATE_OFF = 0x0;

Another case where an enum could be used.


   sun.awt.AppContext context = sun.awt.AppContext.getAppContext();

these days this sometimes can return null ..


There are many usages of getAppContext() without null check all over our
code, so I think that this should be fixed under a separate bug.

AppForegroundEvent :
* @param e the app became foreground notification.

I am not entirely sure what that means or even what it
should say, perha

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi,

CCC is now in final state.

For these raised questions I filed following issues:
https://bugs.openjdk.java.net/browse/JDK-8151184 [macosx] window does 
not receive normal window events indicating the window is hidden
https://bugs.openjdk.java.net/browse/JDK-8151189 Possible 
getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar


--
Thanks,
Alexander.

On 02/24/2016 07:27 AM, Philip Race wrote:

Hi,

API-wise this now looks like it could go to CCC.
I have not reviewed the implementation yet but that
should not stop the CCC from being submitted as final.

Perhaps the only concern I have is around AppHidden/Appreopened
where I am not convinced that it is the right thing to not send normal
window events indicating the window is hidden. What would the harm
be in sending such events so that apps that aren't able to use these
Mac-specific events because they pre-date these APIs don't get a
notification which seems to mean the same thing to me.
That could be an implementation detail or something we can
refine later in an targeted CCC if needed.

The AppContext issue is an implementation issue and we can discuss
it later but the reason other places do not check is it never used to
return null. Now it can. So do we really want to add more places
that we need to fix ? In other words I don't think the fix is going
to be to revert to it not returning null, so you are going to have
to deal with it directly here.

-phil.

On 2/21/16, 5:08 PM, Alexander Zvegintsev wrote:

Hi Phil,
please see updated the webrev here
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/08/
and comments inline

On 18.02.2016 0:10, Phil Race wrote:

Desktop.java looks fine now. Moving on to the other API classes

Top-level question :
Why are AppEvent and TaskBar in java.awt and not java.awt.desktop ?
Agreed on AppEvent, but Taskbar is placed in java.awt on purpose to 
make it adjacent with Desktop and SystemTray.

And AppEvent seems to use a lot of nested classes.
If they were in java.awt.desktop then they could be top-level classes
without creating too much clutter. I don't think it matters that 'this
is how the com.apple APIs were done' - people need to recode anyway


AppEvent.java :
more wildcards ..
   28 import java.awt.desktop.*;

I notice the docs in this file (and perhaps more I have yet to see)
reference types without @code :-
eg :
  * Constructs a FilesEvent
* Constructs an OpenFilesEvent
..
  * Get the URI the app was asked to open
  146  * @return the URI

and more. Not essential I suppose but preferable.

   69 public List getFiles() {
   70 return files;
   71 }

Is there any need to return a copy for any of these ?

   95  * Gets the search term. The platform may additionally provide 
the search
   96  * term that was used to find the files. This is for example the 
case
   97  * on Mac OS X, when the files were opened using the Spotlight 
search
   98  * menu or a Finder search window.
   99  *
  100  * This is useful for highlighting the search term in the 
documents when
  101  * they are opened.
  102  * @return the search term used to find the files

Could we say "optionally" instead of "additionally"
and make clear that it may be "null"

a very long line :-

209  * Event sent when the application has become the foreground app, and 
when it has resigned being the foreground app.

And "resigned" is a voluntary term. If something else is placed into 
the foreground the

this app may not get asked if it wants to allow that.
Maybe instead say "and when it is no longer the foreground app."


Done.
Also is there anything we can (or should) say about how some of 
these events interact ?


eg, if a foreground app is hidden does only the appHidden listener 
get called ?



Both listeners could be called, it's already mentioned here:
 * Called when the hidden app is shown again (but not 
necessarily brought to

 * the foreground).
 *
 * @param e event
 */
public void appUnhidden(final AppHiddenEvent e);


BTW the term 'unhidden' makes sense if the app was previously 
'hidden' but less so

if the app is being newly shown.


  /**
  252  * The system does not provide a reason for a session change.
  253  */
  254 @Native
  255 public static final int REASON_UNSPECIFIED = 0x0;
  256

I think we should use an enum here.


Also unless there is a reason I have not yet come across, as many as 
possible

of these classes should be marked 'final'


Taskbar.java :

One meta-concern I have here is that it seems to have a very blurry 
distinction

with existing API :
https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html
https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html

Aren't both of these APIs dealing with the same area ?

Should we really add this new class or tweak system tray as needed ?
On 

Re: [9] Review Request for 8021961: setAlwaysOnTop doesn't behave correctly in Linux/Solaris under certain scenarios

[Alexander Zvegintsev]

+1

--
Thanks,
Alexander.

On 10.02.2016 14:06, Sergey Bylokhov wrote:

Looks fine.

On 01.10.15 21:27, Semyon Sadetsky wrote:

The fix is amended : plese review the new version
http://cr.openjdk.java.net/~ssadetsky/8021961/webrev.01/
It is taken into account that the transientFor chain may be longer and
also the fact that visibility of the parent window may be changed after
the setting the alwaysOnTop property value.
Sorry for possible inconveniences.

--Semyon

On 10/1/2015 1:25 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

X-window manager cannot correctly handle "alwaysOnTop" for the child
windows (dialogs) which parent is invisible. The reason for that that
transientFor hint is wrong in this scenario because it points to an
invisible window. To fix that on Java side the transientFor is set to
the root window for the childs of invisible parent and their type is
set to normal window (not dialog).

--Semyon

Re: [9] Review reuest for 8036915: setLocationRelativeTo stopped working in Ubuntu 13.10 (Unity)

[Alexander Zvegintsev]

Hi Sergey,

Yes,  it is, anyway I can see only a part of the fix in this webrev.

--
Thanks,
Alexander.

On 26.02.2016 16:57, Sergey Bylokhov wrote:

Hi, Alexander.
It seems that this request is withdrawn?

On 24.11.14 18:07, Alexander Zvegintsev wrote:

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8036915/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8036915

The sample in JBS issue reusing one window with setLocationRelativeTo.
This issues can be reproduced for a second and following re-appearance
of the window,
it is shifted exactly by insets size.
This fix bypasses insets optimization for a first reparent notify event.

--
Thanks,

Alexander.

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

windows are hidden,
but in this case windows may not receive such notifications and only 
gets AppHiddenEvent.


/**
   31  * Implementors receive notification when the app has been asked to open 
again.
   32  *
   33  * This notification is useful for showing a new document when your app 
has no open windows.
   34  */
   35 public interface AppReopenedListener extends SystemEventListener {

So .. this means something more than that the windows have been hiddent ?
Does it mean that all windows have been disposed() but the app is still running 
?
Is this the case when the system menu bar can be used be the user to request
a new window ? Can you please give an example.


It is another Mac specific feature, it does exactly how it described.
You can have no windows(not even created), no system menu bar and some 
running thread, AppReopenedListener
will be called on click in dock icon or on double click on the 
application executable.



-phil.


On 02/16/2016 07:04 AM, Alexander Zvegintsev wrote:

Hi Phil,

thanks for the review, please see updated webrev here:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/07/
--
Thanks,
Alexander.
On 02/15/2016 09:58 PM, Phil Race wrote:
Desktop.java : 28 import java.awt.desktop.*; Can we replace this 
wild card import with an enumeration of the imported classes. I am 
sure you need all of them but it is still recommended practice. Also 
I do not see where you are adding the new package to the ones for 
which javadoc is generated. This would be an edit to 
$TOPLEVEL/common/CORE_PKGS.gmk I am focusing first on the API part 
of this so you can finalize the CCC.

> 43  * The {@code Desktop} class allows interact with various desktop 
capabilities.
interact -> interaction

Or better
The {@code Desktop} class allows interaction with various platform 
desktop capabilities.


> * @since 1.9 These should all be @since 9 < 130 * Returns the {@code Desktop} 
instance of the current > 268 * Returns the Desktop 
instance of the current


I am surprised to see this changed .. we are making the opposite update 
elsewhere.
All the cases you have changed need to be changed back.

>641 * Adds sub-types of {@link SystemEventListener} to listen for 
notifications Does that link work ? The referenced class is in a 
different package. I am fairly sure that in all such cases you need 
to fully qualify the class using the package. I believe that javac 
should run doclint as part of the JDK build so you should examine 
the output/logs.


686 * @param aboutHandler the handler to respond to the
687 * @throws UnsupportedOperationException if the current platform 
seems like line 686 is incomplete. Same for line 704 : 704 * @param 
preferencesHandler the handler to respond to the


801
802 /**
803 * Sets the default strategy used to quit this application. The 
default is

804 * calling SYSTEM_EXIT_0.
805 *
806 * @param strategy the way this application should be shutdown
807 * @throws UnsupportedOperationException if the current platform
808 * does not support the {@link Desktop.Action#APP_QUIT_STRATEGY} 
action

809 * @see QuitStrategy
810 * @since 1.9
811 */
812 public void setQuitStrategy(final QuitStrategy strategy) {
813 checkActionSupport(Action.APP_QUIT_STRATEGY);
814 peer.setQuitStrategy(strategy);


Some of the actions discuss permissions and others do not.


Shouldn't this say something about permissions ? You do need permission to
exit the VM, so should you be able to set a QuitStrategy if you don't have
such permission ??



More later ..

-phil.

On 01/21/2016 11:16 PM, Alexander Zvegintsev wrote:

Hi Semyon,

I am not sure about flexibility, but providing additional info for 
a user session change  could be useful


http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/05/
(changed awt_Toolkit.cpp,  AppEvent.java, UserSessionListener.java, 
_AppEventHandler.java)

--
Thanks,
Alexander.
On 01/19/2016 11:52 AM, Semyon Sadetsky wrote:

Hi Alexander,

What do you think about accompanying the application events with a 
flag containing the extended information that may be provided by 
various native platforms? For example, the session events may 
provide a reason on the Windows platform.
Also this may add flexibility that allows to support new features 
in the future.


--Semyon

On 1/17/2016 11:55 PM, Alexander Zvegintsev wrote:

Hi Semyon,
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION? 

Sure, so please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/04/

awt_Toolkit.cpp:
extended list of supported messages for user session listener.

awt_Desktop.cpp, awt_Toolkit.cpp, WDesktopPeer.java:
added SuddenTerminaton support for Windows
--
Thanks,
Alexander.
On 01/15/2016 04:14 PM, Semyon Sadetsky wrote:

Hi,

On 1/15/2016 12:39 PM, Alexander Zvegintsev wrote:

Hi Semyon,

Yes, it is just LOCK/UNLOCK for now,  because it is the most 
common scenario of desktop usage.


Do you mean that we should consider remote des

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Phil,

thanks for the review, please see updated webrev here:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/07/

--
Thanks,
Alexander.

On 02/15/2016 09:58 PM, Phil Race wrote:
Desktop.java : 28 import java.awt.desktop.*; Can we replace this wild 
card import with an enumeration of the imported classes. I am sure you 
need all of them but it is still recommended practice. Also I do not 
see where you are adding the new package to the ones for which javadoc 
is generated. This would be an edit to $TOPLEVEL/common/CORE_PKGS.gmk 
I am focusing first on the API part of this so you can finalize the CCC.

> 43  * The {@code Desktop} class allows interact with various desktop 
capabilities.
interact -> interaction

Or better
The {@code Desktop} class allows interaction with various platform 
desktop capabilities.


> * @since 1.9 These should all be @since 9 < 130 * Returns the {@code Desktop} 
instance of the current > 268 * Returns the Desktop 
instance of the current


I am surprised to see this changed .. we are making the opposite update 
elsewhere.
All the cases you have changed need to be changed back.

>641 * Adds sub-types of {@link SystemEventListener} to listen for 
notifications Does that link work ? The referenced class is in a 
different package. I am fairly sure that in all such cases you need to 
fully qualify the class using the package. I believe that javac should 
run doclint as part of the JDK build so you should examine the 
output/logs.


686 * @param aboutHandler the handler to respond to the
687 * @throws UnsupportedOperationException if the current platform 
seems like line 686 is incomplete. Same for line 704 : 704 * @param 
preferencesHandler the handler to respond to the


801
802 /**
803 * Sets the default strategy used to quit this application. The 
default is

804 * calling SYSTEM_EXIT_0.
805 *
806 * @param strategy the way this application should be shutdown
807 * @throws UnsupportedOperationException if the current platform
808 * does not support the {@link Desktop.Action#APP_QUIT_STRATEGY} action
809 * @see QuitStrategy
810 * @since 1.9
811 */
812 public void setQuitStrategy(final QuitStrategy strategy) {
813 checkActionSupport(Action.APP_QUIT_STRATEGY);
814 peer.setQuitStrategy(strategy);


Some of the actions discuss permissions and others do not.


Shouldn't this say something about permissions ? You do need permission to
exit the VM, so should you be able to set a QuitStrategy if you don't have
such permission ??



More later ..

-phil.

On 01/21/2016 11:16 PM, Alexander Zvegintsev wrote:

Hi Semyon,

I am not sure about flexibility, but providing additional info for a 
user session change  could be useful


http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/05/
(changed awt_Toolkit.cpp,  AppEvent.java, UserSessionListener.java, 
_AppEventHandler.java)

--
Thanks,
Alexander.
On 01/19/2016 11:52 AM, Semyon Sadetsky wrote:

Hi Alexander,

What do you think about accompanying the application events with a 
flag containing the extended information that may be provided by 
various native platforms? For example, the session events may 
provide a reason on the Windows platform.
Also this may add flexibility that allows to support new features in 
the future.


--Semyon

On 1/17/2016 11:55 PM, Alexander Zvegintsev wrote:

Hi Semyon,
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION? 

Sure, so please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/04/

awt_Toolkit.cpp:
extended list of supported messages for user session listener.

awt_Desktop.cpp, awt_Toolkit.cpp, WDesktopPeer.java:
added SuddenTerminaton support for Windows
--
Thanks,
Alexander.
On 01/15/2016 04:14 PM, Semyon Sadetsky wrote:

Hi,

On 1/15/2016 12:39 PM, Alexander Zvegintsev wrote:

Hi Semyon,

Yes, it is just LOCK/UNLOCK for now,  because it is the most 
common scenario of desktop usage.


Do you mean that we should consider remote desktop logins too? 
Something like:


 if (wParam == WTS_CONSOLE_CONNECT
  || wParam == WTS_CONSOLE_DISCONNECT
  || wParam == WTS_REMOTE_CONNECT
  || wParam == WTS_REMOTE_DISCONNECT
  || wParam == WTS_SESSION_UNLOCK
  || wParam == WTS_SESSION_LOCK) {

  BOOL activate = wParam == WTS_CONSOLE_CONNECT
|| wParam == WTS_REMOTE_CONNECT
|| wParam == WTS_SESSION_UNLOCK;
env->CallStaticVoidMethod(clzz, AwtToolkit::userSessionMID,
activate
? JNI_TRUE
: JNI_FALSE);
  }


That's seems better to me.

Or if you refer to WTS_SESSION_LOGOFF, then it seems useless for us,
AFAIK only services will receive this notification, and anyway 
all apps the user was running are already killed by this time. 
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION?


--Semyon

--
Thanks,
Alexander.
On 12.01.2016 19:21, Semyon Sadetsky

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Semyon,

I am not sure about flexibility, but providing additional info for a 
user session change  could be useful


http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/05/
(changed awt_Toolkit.cpp,  AppEvent.java, UserSessionListener.java, 
_AppEventHandler.java)


--
Thanks,
Alexander.

On 01/19/2016 11:52 AM, Semyon Sadetsky wrote:

Hi Alexander,

What do you think about accompanying the application events with a 
flag containing the extended information that may be provided by 
various native platforms? For example, the session events may provide 
a reason on the Windows platform.
Also this may add flexibility that allows to support new features in 
the future.


--Semyon

On 1/17/2016 11:55 PM, Alexander Zvegintsev wrote:

Hi Semyon,
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION? 

Sure, so please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/04/

awt_Toolkit.cpp:
extended list of supported messages for user session listener.

awt_Desktop.cpp, awt_Toolkit.cpp, WDesktopPeer.java:
added SuddenTerminaton support for Windows
--
Thanks,
Alexander.
On 01/15/2016 04:14 PM, Semyon Sadetsky wrote:

Hi,

On 1/15/2016 12:39 PM, Alexander Zvegintsev wrote:

Hi Semyon,

Yes, it is just LOCK/UNLOCK for now,  because it is the most common 
scenario of desktop usage.


Do you mean that we should consider remote desktop logins too? 
Something like:


 if (wParam == WTS_CONSOLE_CONNECT
  || wParam == WTS_CONSOLE_DISCONNECT
  || wParam == WTS_REMOTE_CONNECT
  || wParam == WTS_REMOTE_DISCONNECT
  || wParam == WTS_SESSION_UNLOCK
  || wParam == WTS_SESSION_LOCK) {

  BOOL activate = wParam == WTS_CONSOLE_CONNECT
|| wParam == WTS_REMOTE_CONNECT
|| wParam == WTS_SESSION_UNLOCK;
  env->CallStaticVoidMethod(clzz, 
AwtToolkit::userSessionMID,

activate
? JNI_TRUE
: JNI_FALSE);
  }


That's seems better to me.

Or if you refer to WTS_SESSION_LOGOFF, then it seems useless for us,
AFAIK only services will receive this notification, and anyway all 
apps the user was running are already killed by this time. 
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION?


--Semyon

--
Thanks,
Alexander.
On 12.01.2016 19:21, Semyon Sadetsky wrote:

Hi Alexander,

awt_Toolkit.cpp:

  case WM_WTSSESSION_CHANGE: {
  jclass clzz = 
env->FindClass("sun/awt/windows/WDesktopPeer");

  DASSERT(clzz != NULL);
  if (!clzz) throw std::bad_alloc();

  if (wParam == WTS_SESSION_LOCK || wParam == 
WTS_SESSION_UNLOCK) {
  env->CallStaticVoidMethod(clzz, 
AwtToolkit::userSessionMID,
wParam == 
WTS_SESSION_UNLOCK

? JNI_TRUE
: JNI_FALSE);
  }
  break;
  }

So only the WTS_SESSION_UNLOCK is propagated to java as a session 
act/deact and other messages just ignored?


Other messages:
#define WTS_CONSOLE_CONNECT 0x1
#define WTS_CONSOLE_DISCONNECT 0x2
#define WTS_REMOTE_CONNECT 0x3
#define WTS_REMOTE_DISCONNECT 0x4
#define WTS_SESSION_LOGON 0x5
#define WTS_SESSION_LOGOFF 0x6
#define WTS_SESSION_LOCK 0x7
#define WTS_SESSION_REMOTE_CONTROL

some of them seems to me more suitable to be chosen as the session 
act/deact event.  Could you comment that for me?


--Semyon

On 11/24/2015 6:02 PM, Alexander Zvegintsev wrote:

Please review the updated fix:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/

removed fullscreen related code (moved to JDK-8143914 [0])
fix serialization in AppEvent

[0] https://bugs.openjdk.java.net/browse/JDK-8143914

Thanks,

Alexander.
On 11/21/2015 03:33 AM, Alexander Zvegintsev wrote:

Hi Phil,

Can someone explain why this is needed given the existing 
support of
GraphicsDevice.setFullScreenWindow(Window) ? 


GraphicsDevice.setFullScreenWindow is used for an exclusive full 
screen mode.
Mac OS has another option to create a virtual desktop with 
provided window in it.

You can switch between them by three-finger horizontal swipe.

Why does it have to be a RootPaneContainer ? Why is this tied 
to Swing ?
This appears to narrow it to JDialog and JWindow. 

It reuses swing code to set native windows style bits.


Please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/
updated permission
added missing @throws @since and @implNote
browseFileDirectory is now return void
RootPaneContainer -> JDialog and JWindow

--
Thanks,
Alexander.

On 11/20/2015 09:03 PM, Phil Race wrote:

On 11/20/2015 09:12 AM, Sergey Bylokhov wrote:


I am worried about setWindowCanFullScreen and 
requestToggleFullScreen.
On the latest osx this fu

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Semyon,
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION? 

Sure, so please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/04/

awt_Toolkit.cpp:
extended list of supported messages for user session listener.

awt_Desktop.cpp, awt_Toolkit.cpp, WDesktopPeer.java:
added SuddenTerminaton support for Windows

--
Thanks,
Alexander.

On 01/15/2016 04:14 PM, Semyon Sadetsky wrote:

Hi,

On 1/15/2016 12:39 PM, Alexander Zvegintsev wrote:

Hi Semyon,

Yes, it is just LOCK/UNLOCK for now,  because it is the most common 
scenario of desktop usage.


Do you mean that we should consider remote desktop logins too? 
Something like:


 if (wParam == WTS_CONSOLE_CONNECT
  || wParam == WTS_CONSOLE_DISCONNECT
  || wParam == WTS_REMOTE_CONNECT
  || wParam == WTS_REMOTE_DISCONNECT
  || wParam == WTS_SESSION_UNLOCK
  || wParam == WTS_SESSION_LOCK) {

  BOOL activate = wParam == WTS_CONSOLE_CONNECT
|| wParam == WTS_REMOTE_CONNECT
|| wParam == WTS_SESSION_UNLOCK;
  env->CallStaticVoidMethod(clzz, AwtToolkit::userSessionMID,
activate
? JNI_TRUE
: JNI_FALSE);
  }


That's seems better to me.

Or if you refer to WTS_SESSION_LOGOFF, then it seems useless for us,
AFAIK only services will receive this notification, and anyway all 
apps the user was running are already killed by this time. 
Is it possible to use WM_QUERYENDSESSION for 
Action.APP_SUDDEN_TERMINATION?


--Semyon

--
Thanks,
Alexander.
On 12.01.2016 19:21, Semyon Sadetsky wrote:

Hi Alexander,

awt_Toolkit.cpp:

  case WM_WTSSESSION_CHANGE: {
  jclass clzz = env->FindClass("sun/awt/windows/WDesktopPeer");
  DASSERT(clzz != NULL);
  if (!clzz) throw std::bad_alloc();

  if (wParam == WTS_SESSION_LOCK || wParam == 
WTS_SESSION_UNLOCK) {
  env->CallStaticVoidMethod(clzz, 
AwtToolkit::userSessionMID,

wParam == WTS_SESSION_UNLOCK
? JNI_TRUE
: JNI_FALSE);
  }
  break;
  }

So only the WTS_SESSION_UNLOCK is propagated to java as a session 
act/deact and other messages just ignored?


Other messages:
#define WTS_CONSOLE_CONNECT 0x1
#define WTS_CONSOLE_DISCONNECT 0x2
#define WTS_REMOTE_CONNECT 0x3
#define WTS_REMOTE_DISCONNECT 0x4
#define WTS_SESSION_LOGON 0x5
#define WTS_SESSION_LOGOFF 0x6
#define WTS_SESSION_LOCK 0x7
#define WTS_SESSION_REMOTE_CONTROL

some of them seems to me more suitable to be chosen as the session 
act/deact event.  Could you comment that for me?


--Semyon

On 11/24/2015 6:02 PM, Alexander Zvegintsev wrote:

Please review the updated fix:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/

removed fullscreen related code (moved to JDK-8143914 [0])
fix serialization in AppEvent

[0] https://bugs.openjdk.java.net/browse/JDK-8143914

Thanks,

Alexander.
On 11/21/2015 03:33 AM, Alexander Zvegintsev wrote:

Hi Phil,


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ? 


GraphicsDevice.setFullScreenWindow is used for an exclusive full 
screen mode.
Mac OS has another option to create a virtual desktop with 
provided window in it.

You can switch between them by three-finger horizontal swipe.

Why does it have to be a RootPaneContainer ? Why is this tied to 
Swing ?
This appears to narrow it to JDialog and JWindow. 

It reuses swing code to set native windows style bits.


Please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/
updated permission
added missing @throws @since and @implNote
browseFileDirectory is now return void
RootPaneContainer -> JDialog and JWindow

--
Thanks,
Alexander.

On 11/20/2015 09:03 PM, Phil Race wrote:

On 11/20/2015 09:12 AM, Sergey Bylokhov wrote:


I am worried about setWindowCanFullScreen and 
requestToggleFullScreen.
On the latest osx this functionality was merged with maximize 
button. So probably it will be better to change behavior of 
window.setExtendedState() + MAXIMIZED_BOTH?


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ?

And what happens if you use *both* ? They still need to play well 
together

if there is some reason the new one is needed.

Other comments :
>   * Note, Aqua Look and Feel should be active to support this 
on Mac OS.



Needs @implNote

There seems to be lots of missing SecurityException tags given 
all the checkAWTPermission() calls.
is checkAWTPermission() really the right call for all of these 
actions ?
Does it "cover" being able to delete files and quit the app ? I 
am n

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Semyon,

Yes, it is just LOCK/UNLOCK for now,  because it is the most common 
scenario of desktop usage.


Do you mean that we should consider remote desktop logins too? Something 
like:


 if (wParam == WTS_CONSOLE_CONNECT
  || wParam == WTS_CONSOLE_DISCONNECT
  || wParam == WTS_REMOTE_CONNECT
  || wParam == WTS_REMOTE_DISCONNECT
  || wParam == WTS_SESSION_UNLOCK
  || wParam == WTS_SESSION_LOCK) {

  BOOL activate = wParam == WTS_CONSOLE_CONNECT
|| wParam == WTS_REMOTE_CONNECT
|| wParam == WTS_SESSION_UNLOCK;
  env->CallStaticVoidMethod(clzz, AwtToolkit::userSessionMID,
activate
? JNI_TRUE
: JNI_FALSE);
  }

Or if you refer to WTS_SESSION_LOGOFF, then it seems useless for us,
AFAIK only services will receive this notification, and anyway all apps 
the user was running are already killed by this time.


--
Thanks,
Alexander.

On 12.01.2016 19:21, Semyon Sadetsky wrote:

Hi Alexander,

awt_Toolkit.cpp:

  case WM_WTSSESSION_CHANGE: {
  jclass clzz = env->FindClass("sun/awt/windows/WDesktopPeer");
  DASSERT(clzz != NULL);
  if (!clzz) throw std::bad_alloc();

  if (wParam == WTS_SESSION_LOCK || wParam == 
WTS_SESSION_UNLOCK) {

  env->CallStaticVoidMethod(clzz, AwtToolkit::userSessionMID,
wParam == WTS_SESSION_UNLOCK
? JNI_TRUE
: JNI_FALSE);
  }
  break;
  }

So only the WTS_SESSION_UNLOCK is propagated to java as a session 
act/deact and other messages just ignored?


Other messages:
#define WTS_CONSOLE_CONNECT 0x1
#define WTS_CONSOLE_DISCONNECT 0x2
#define WTS_REMOTE_CONNECT 0x3
#define WTS_REMOTE_DISCONNECT 0x4
#define WTS_SESSION_LOGON 0x5
#define WTS_SESSION_LOGOFF 0x6
#define WTS_SESSION_LOCK 0x7
#define WTS_SESSION_REMOTE_CONTROL

some of them seems to me more suitable to be chosen as the session 
act/deact event.  Could you comment that for me?


--Semyon

On 11/24/2015 6:02 PM, Alexander Zvegintsev wrote:

Please review the updated fix:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/

removed fullscreen related code (moved to JDK-8143914 [0])
fix serialization in AppEvent

[0] https://bugs.openjdk.java.net/browse/JDK-8143914

Thanks,

Alexander.
On 11/21/2015 03:33 AM, Alexander Zvegintsev wrote:

Hi Phil,


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ? 


GraphicsDevice.setFullScreenWindow is used for an exclusive full 
screen mode.
Mac OS has another option to create a virtual desktop with provided 
window in it.

You can switch between them by three-finger horizontal swipe.

Why does it have to be a RootPaneContainer ? Why is this tied to 
Swing ?
This appears to narrow it to JDialog and JWindow. 

It reuses swing code to set native windows style bits.


Please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/
updated permission
added missing @throws @since and @implNote
browseFileDirectory is now return void
RootPaneContainer -> JDialog and JWindow

--
Thanks,
Alexander.

On 11/20/2015 09:03 PM, Phil Race wrote:

On 11/20/2015 09:12 AM, Sergey Bylokhov wrote:


I am worried about setWindowCanFullScreen and 
requestToggleFullScreen.
On the latest osx this functionality was merged with maximize 
button. So probably it will be better to change behavior of 
window.setExtendedState() + MAXIMIZED_BOTH?


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ?

And what happens if you use *both* ? They still need to play well 
together

if there is some reason the new one is needed.

Other comments :
>   * Note, Aqua Look and Feel should be active to support this on 
Mac OS.



Needs @implNote

There seems to be lots of missing SecurityException tags given all 
the checkAWTPermission() calls.
is checkAWTPermission() really the right call for all of these 
actions ?
Does it "cover" being able to delete files and quit the app ? I am 
not sure it is

correct in all cases.

And also there are missing @since tags.


 Opens a folder containing the {@code file} in a default system 
file manager.

 933  * @param file the file
 934  * @return returns true if successfully opened
 935  * @throws NullPointerException if {@code file} is {@code 
null}
 936  * @throws IllegalArgumentException if the specified file 
doesn't

 937  * exist
 938  */
 939 public boolean browseFileDirectory(File file) {

So what happens if there is no "support" for this ? Excepti

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Please review the updated fix:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/

removed fullscreen related code (moved to JDK-8143914 [0])
fix serialization in AppEvent

[0] https://bugs.openjdk.java.net/browse/JDK-8143914

Thanks,

Alexander.

On 11/21/2015 03:33 AM, Alexander Zvegintsev wrote:

Hi Phil,


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ? 


GraphicsDevice.setFullScreenWindow is used for an exclusive full 
screen mode.
Mac OS has another option to create a virtual desktop with provided 
window in it.

You can switch between them by three-finger horizontal swipe.


Why does it have to be a RootPaneContainer ? Why is this tied to Swing ?
This appears to narrow it to JDialog and JWindow. 

It reuses swing code to set native windows style bits.


Please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/
updated permission
added missing @throws @since and @implNote
browseFileDirectory is now return void
RootPaneContainer -> JDialog and JWindow

--
Thanks,
Alexander.

On 11/20/2015 09:03 PM, Phil Race wrote:

On 11/20/2015 09:12 AM, Sergey Bylokhov wrote:


I am worried about setWindowCanFullScreen and requestToggleFullScreen.
On the latest osx this functionality was merged with maximize 
button. So probably it will be better to change behavior of 
window.setExtendedState() + MAXIMIZED_BOTH?


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ?

And what happens if you use *both* ? They still need to play well 
together

if there is some reason the new one is needed.

Other comments :
>   * Note, Aqua Look and Feel should be active to support this on 
Mac OS.



Needs @implNote

There seems to be lots of missing SecurityException tags given all 
the checkAWTPermission() calls.

is checkAWTPermission() really the right call for all of these actions ?
Does it "cover" being able to delete files and quit the app ? I am 
not sure it is

correct in all cases.

And also there are missing @since tags.


 Opens a folder containing the {@code file} in a default system file 
manager.

 933  * @param file the file
 934  * @return returns true if successfully opened
 935  * @throws NullPointerException if {@code file} is {@code null}
 936  * @throws IllegalArgumentException if the specified file 
doesn't

 937  * exist
 938  */
 939 public boolean browseFileDirectory(File file) {

So what happens if there is no "support" for this ? Exception or 
"false" ?
Are you comfortable that all these APIs that return "true" if 
successful are

implementable on all platforms. i.e I mean that does the platform return
a value you can pass on as success/failure.

---

861  * Attaches a {@link FullScreenListener} to the specified 
top-level

 862  * {@link Window}.
 863  *
 864  * @param window to attach the {@link FullScreenListener} to
 865  * @param listener to be notified when a full screen event 
occurs

 866  * @throws IllegalArgumentException if window is not a
 867  * {@link javax.swing.RootPaneContainer}
 868  */
 869 public void addWindowFullScreenListener(final Window window,
 870   final 
FullScreenListener listener) {


---

Why does it have to be a RootPaneContainer ? Why is this tied to Swing ?
This appears to narrow it to JDialog and JWindow.

-phil.

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hello Yuri,

Thanks for you testing!

If I start the application from dash, I have  progress reported and the
menu on the right click. I _don't have_, however, application menu
("File" etc.). 
By etc.  menu did you mean a global menu[0] (which may be implemented by 
Desktop.setMenuBar() later)

or a Taskbar.setMenu()[1]?

Note that if I start it from a command line, the behavior is different:
the list of reported actions/features is the same but
progress doesn't work at all.
For Taskbar we are reporting progress as supported If we successfully 
load libunity and all its required functions.
When some of these supported features doesn't work it means that unity 
does not bind .desktop  and your

app for some reason (or you have libunity.so and running KDE :))


Also, try to start the application twice:
the taskbar menu will be distorted (there always will be
2 garbage lines),
I guess it should be fixed someway. 

I'll take a look at this at your machine later, since I cannot reproduce it.


[0] 
http://cloud.addictivetips.com/wp-content/uploads/2011/10/Global-Menu.jpg
[1] 
https://wiki.ubuntu.com/Unity/LauncherAPI?action=AttachFile=get=quicklilst-static-entry.png


--
Thanks,
Alexander.

On 11/20/2015 05:36 PM, Yuri Nesterenko wrote:

Hi Alexander,

On 11/19/2015 06:23 PM, Alexander Zvegintsev wrote:

Hi Yuri,

Nice catch! Just tested on Ubuntu 14.04.3, it does not have libunity.so
symlink to libuity.so.9.
so we should try to load version 9 instead of 0 in awt_Taskbar.c:
-#define UNITY_LIB_VERSIONED VERSIONED_JNI_LIB_NAME("unity", "0")
+#define UNITY_LIB_VERSIONED VERSIONED_JNI_LIB_NAME("unity", "9")


Rebuilt and tried.
If you are interested, look at the screenshot of the Snip
application in http://cr.openjdk.java.net/~yan/8143227/Snip1.png
It was started from dash.

If I start the application from dash, I have  progress reported and the
menu on the right click. I _don't have_, however, application menu
("File" etc.).

Note that if I start it from a command line, the behavior is different:
the list of reported actions/features is the same but
progress doesn't work at all.

Also, try to start the application twice:
the taskbar menu will be distorted (there always will be
2 garbage lines),
I guess it should be fixed someway.

If I start the application from a command line in a remote session,
the list of taskbar options will be the same but
neither taskbar menu nor progress will work.



Updated in place.

Browse action works fine for me on mine Ubuntu 14.04.3. Supported
actions fills dynamically
in gtk2_interface.c  update_supported_actions()[0]. So probably you are
running 32 bit JDK on 64 bit
Ubuntu and there are some missing libraries.

Well, I build it on another 14.04 machine but both of them are x64,
and the build is default, x64 too.
Actually I have mail and browse actions available if the session is
local which sure is right.

Thank you!
-yan


[0]
http://hg.openjdk.java.net/jdk9/client/jdk/file/e8e7a00c1bff/src/java.desktop/unix/native/libawt_xawt/awt/gtk2_interface.c#l478 




Thanks,

Alexander.

On 11/19/2015 05:41 PM, Yuri Nesterenko wrote:

Hi Alexander,

I tried it with Ubuntu 14.04.3, and it started indeed
from dash or a command line as a gray rectangle. Once I added
isSupported() for every Action and separately for the Taskbar,
it reported, just like jdk9b91, that only OPEN was supported
and nothing else.
Is it OK?

dconf editor in com/canonical/unity/launcher reports
firefox as, I presume, a default browser, but I don't see
BROWSE supported.


Thanks,
-yan

On 11/18/2015 05:01 PM, Alexander Zvegintsev wrote:

Hi Alexander,

resending the same webrev under the new ID
https://bugs.openjdk.java.net/browse/JDK-8143227
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/

The approach is pretty the same as it was in Desktop before:
Check feature with isSupported() call.
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/Snip.java

For testing it on Ubuntu you should create a .desktop file e.g.:
~/.local/share/applications/SomeApp.desktop

[Desktop Entry]
Name=SomeApp
Path=/path/to/your/directory/with/Snip
Exec=/path/to/jdk9/java -Djava.desktop.appName=SomeApp.desktop -jar
Snip.jar
#or Exec=/path/to/jdk9/java -Djava.desktop.appName=SomeApp.desktop 
Snip

Terminal=false
Type=Application

After that you can run it from dash with specified name.

--
Thanks,
Alexander.

On 11/18/2015 02:52 PM, Alexander Scherbatiy wrote:

On 11/18/2015 10:12 AM, Alexander Zvegintsev wrote:

Hello

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8048731/
for
https://bugs.openjdk.java.net/browse/JDK-8048731

This fix provides public support Mac OS X
features(com.apple.{eawt,eio}), adds support for various desktop
features such as progress indication, dock overlays, dock menus, 
etc.

This is done by extending java.awt.Desktop and adding
java.awt.Taskbar classes


   Could you provide some code snippets which illustrate how the
introduce

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

 -> in the task area
* system provided -> system-provided (2x)
* Some platforms does -> Some platforms do
* and accepts only -> and accept only
* In this case pass an integer represented as string as a parameter. 
-> In this case, pass an integer represented as a string as parameter.


http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/AboutHandler.java.html 


* it's about dialog -> its about dialog (2x)

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/AppReopenedListener.java.html 


* re-opened -> reopened
* re-open -> reopen

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/FileManager.java.html 


* a codes -> a code

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/OpenFilesHandler.java.html 


* it's Info.plist. -> its Info.plist.

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/PreferencesHandler.java.html 


* it's preferences UI -> its preferences UI (2x)

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/QuitStrategy.java.html 


* The strategy use to -> The strategy used to

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/ScreenSleepListener.java.html 


* have awoke -> have awoken

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/desktop/SystemSleepListener.java.html 


* has awoke -> has awoken


On 18/11/2015 8:12, Alexander Zvegintsev wrote:

Hello

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8048731/
for
https://bugs.openjdk.java.net/browse/JDK-8048731

This fix provides public support Mac OS X 
features(com.apple.{eawt,eio}), adds support for various desktop

features such as progress indication, dock overlays, dock menus, etc.
This is done by extending java.awt.Desktop and adding 
java.awt.Taskbar classes


Linux support is limited by Unity, however this is not the only 
limitation :) An app should be run via
AppName.desktop file [0] with specified system property 
-Djava.desktop.appName=AppName.desktop


[0] https://help.ubuntu.com/community/UnityLaunchersAndDesktopFiles

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Do we have the similar functionality already in the Desktop.open() if 
the folder is passed to it? Or this method do some additional things? 

it also selects the specified file, doc updated in .02

--
Thanks,
Alexander.

On 11/20/2015 08:12 PM, Sergey Bylokhov wrote:

On 20.11.15 8:32, Alexander Zvegintsev wrote:

Hi Anthony,

many thanks for your input!
You are right, most of FileManager API looks like an alien in
java.awt.desktop, so let it be revised as separate CR.
This CR will cover only Desktop and Taskbar classes.

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/01/

FileManager removed, moveToTrash() and browseFileDirectory()
(revealInFinder) added to Desktop


Do we have the similar functionality already in the Desktop.open() if 
the folder is passed to it? Or this method do some additional things?



Moved TaskbarPeer to the correct location.
Javadoc fixes


I am worried about setWindowCanFullScreen and requestToggleFullScreen.
On the latest osx this functionality was merged with maximize button. 
So probably it will be better to change behavior of 
window.setExtendedState() + MAXIMIZED_BOTH?




--
Thanks,
Alexander.

On 20.11.2015 0:40, Anthony Vanelverdinghe wrote:

Hi Alexander

One thing I strongly feel should be revisited, is the FileManager API.
Here's my view on this:
- the functionality for the HFS/HFS+ attributes should be provided by
interfaces in java.nio.file.attribute, analog to dos/posix file
attributes
- the getResource/getPathToApplicationBundle methods should be moved
to a separate, OS-independent "application bundle" feature in
java.awt.Desktop. Even a .jar is in essence an application bundle.
- the findFolder methods are OS X-specific, even though most OSes
provides analog functionality. For example, Windows calls this concept
"Known Folders" [
https://msdn.microsoft.com/en-us/library/windows/desktop/bb776911%28v=vs.85%29.aspx 


]
- the moveToTrash method is a common and long-requested RFE, which
should be supported on multiple platforms
- revealInFinder should be renamed to a common name like "explore",
"browse" or "open"
In summary, I believe that the FileManager API should be made
OS-independent, even if initially this feature is only supported on 
OS X.


On another note, I guess TaskbarPeer should be in the package
java.awt.peer instead of java.awt?

Further, I went through the Javadoc of the public API:
- AppEvent has "{@link Application}" in its Javadoc. I think this
should be removed, since there's no such class in the public API.

- AppEvent has the following Javadoc: "If the files were opened using
the Spotlight search menu or a Finder search window, this method
obtains the search term used to find the files.". In my opinion, this
should be rephrased to be OS-independent, but use OS X as an example,
for example: "The platform may additionally provide the search term
that was used to find the files. This is for example the case on Mac
OS X, when the files were opened using the Spotlight search menu or a
Finder search window."

- Taskbar has the following Javadoc: "Linux support is limited to
Unity". In my opinion, this should say "currently limited", and the
paragraph should be prefixed with an @implNote annotation.

- Taskbar::getTaskbar mentions "browser context" 2 times. However, I
have no clue what this means. Can this be reworded, or the term
clarified?

Below is the remainder of my Javadoc nitpicking.

Kind regards,
Anthony


http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/Desktop.java.udiff.html 



* Mac OS notifications -> Mac OS, notifications
* present in it's -> present in its
* a AppForegroundListener -> an AppForegroundListener
* a AppHiddenListener -> an AppHiddenListener
* a AppReopenedListener -> an AppReopenedListener
* a AboutHandler -> an AboutHandler
* a OpenFilesHandler -> an OpenFilesHandler
* a OpenURIHandler -> an OpenURIHandler
* platform specific -> platform-specific
* on current -> on the current (2x)
* user initiated -> user-initiated

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/AppEvent.java.html 



* it's about window -> its about window
* it's preferences window -> its preferences window
* @see AppReOpenedListener#appReOpened(AppEvent.AppReOpenedEvent) ->
AppReopenedListener#appReopened(AppEvent.AppReopenedEvent)

http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/share/classes/java/awt/Taskbar.java.html 



* system task area(Taskbar, Dock, etc.). -> the system task area
(taskbar, Dock, etc.).
* on running platform -> on the current platform
* appending user specified menu to application -> appending a
user-specified menu to the application
* Linux support is limited to Unity, however to make this features
work on Unity the app [...] -> Linux supp

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Phil,


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ? 


GraphicsDevice.setFullScreenWindow is used for an exclusive full screen 
mode.
Mac OS has another option to create a virtual desktop with provided 
window in it.

You can switch between them by three-finger horizontal swipe.


Why does it have to be a RootPaneContainer ? Why is this tied to Swing ?
This appears to narrow it to JDialog and JWindow. 

It reuses swing code to set native windows style bits.


Please see updated webrev:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/
updated permission
added missing @throws @since and @implNote
browseFileDirectory is now return void
RootPaneContainer -> JDialog and JWindow

--
Thanks,
Alexander.

On 11/20/2015 09:03 PM, Phil Race wrote:

On 11/20/2015 09:12 AM, Sergey Bylokhov wrote:


I am worried about setWindowCanFullScreen and requestToggleFullScreen.
On the latest osx this functionality was merged with maximize button. 
So probably it will be better to change behavior of 
window.setExtendedState() + MAXIMIZED_BOTH?


Can someone explain why this is needed given the existing support of
GraphicsDevice.setFullScreenWindow(Window) ?

And what happens if you use *both* ? They still need to play well 
together

if there is some reason the new one is needed.

Other comments :
>   * Note, Aqua Look and Feel should be active to support this on Mac 
OS.



Needs @implNote

There seems to be lots of missing SecurityException tags given all the 
checkAWTPermission() calls.

is checkAWTPermission() really the right call for all of these actions ?
Does it "cover" being able to delete files and quit the app ? I am not 
sure it is

correct in all cases.

And also there are missing @since tags.


 Opens a folder containing the {@code file} in a default system file 
manager.

 933  * @param file the file
 934  * @return returns true if successfully opened
 935  * @throws NullPointerException if {@code file} is {@code null}
 936  * @throws IllegalArgumentException if the specified file 
doesn't

 937  * exist
 938  */
 939 public boolean browseFileDirectory(File file) {

So what happens if there is no "support" for this ? Exception or 
"false" ?
Are you comfortable that all these APIs that return "true" if 
successful are

implementable on all platforms. i.e I mean that does the platform return
a value you can pass on as success/failure.

---

861  * Attaches a {@link FullScreenListener} to the specified 
top-level

 862  * {@link Window}.
 863  *
 864  * @param window to attach the {@link FullScreenListener} to
 865  * @param listener to be notified when a full screen event 
occurs

 866  * @throws IllegalArgumentException if window is not a
 867  * {@link javax.swing.RootPaneContainer}
 868  */
 869 public void addWindowFullScreenListener(final Window window,
 870   final 
FullScreenListener listener) {


---

Why does it have to be a RootPaneContainer ? Why is this tied to Swing ?
This appears to narrow it to JDialog and JWindow.

-phil.

Re: Review-request for 8143227: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Yuri,

Nice catch! Just tested on Ubuntu 14.04.3, it does not have libunity.so 
symlink to libuity.so.9.

so we should try to load version 9 instead of 0 in awt_Taskbar.c:
-#define UNITY_LIB_VERSIONED VERSIONED_JNI_LIB_NAME("unity", "0")
+#define UNITY_LIB_VERSIONED VERSIONED_JNI_LIB_NAME("unity", "9")

Updated in place.

Browse action works fine for me on mine Ubuntu 14.04.3. Supported 
actions fills dynamically
in gtk2_interface.c  update_supported_actions()[0]. So probably you are 
running 32 bit JDK on 64 bit

Ubuntu and there are some missing libraries.

[0] 
http://hg.openjdk.java.net/jdk9/client/jdk/file/e8e7a00c1bff/src/java.desktop/unix/native/libawt_xawt/awt/gtk2_interface.c#l478


Thanks,

Alexander.

On 11/19/2015 05:41 PM, Yuri Nesterenko wrote:

Hi Alexander,

I tried it with Ubuntu 14.04.3, and it started indeed
from dash or a command line as a gray rectangle. Once I added
isSupported() for every Action and separately for the Taskbar,
it reported, just like jdk9b91, that only OPEN was supported
and nothing else.
Is it OK?

dconf editor in com/canonical/unity/launcher reports
firefox as, I presume, a default browser, but I don't see
BROWSE supported.


Thanks,
-yan

On 11/18/2015 05:01 PM, Alexander Zvegintsev wrote:

Hi Alexander,

resending the same webrev under the new ID
https://bugs.openjdk.java.net/browse/JDK-8143227
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/

The approach is pretty the same as it was in Desktop before:
Check feature with isSupported() call.
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/Snip.java

For testing it on Ubuntu you should create a .desktop file e.g.:
~/.local/share/applications/SomeApp.desktop

[Desktop Entry]
Name=SomeApp
Path=/path/to/your/directory/with/Snip
Exec=/path/to/jdk9/java -Djava.desktop.appName=SomeApp.desktop -jar
Snip.jar
#or Exec=/path/to/jdk9/java -Djava.desktop.appName=SomeApp.desktop Snip
Terminal=false
Type=Application

After that you can run it from dash with specified name.

--
Thanks,
Alexander.

On 11/18/2015 02:52 PM, Alexander Scherbatiy wrote:

On 11/18/2015 10:12 AM, Alexander Zvegintsev wrote:

Hello

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8048731/
for
https://bugs.openjdk.java.net/browse/JDK-8048731

This fix provides public support Mac OS X
features(com.apple.{eawt,eio}), adds support for various desktop
features such as progress indication, dock overlays, dock menus, etc.
This is done by extending java.awt.Desktop and adding
java.awt.Taskbar classes


   Could you provide some code snippets which illustrate how the
introduced API should be used?

  Thanks,
  Alexandr.


Linux support is limited by Unity, however this is not the only
limitation :) An app should be run via
AppName.desktop file [0] with specified system property
-Djava.desktop.appName=AppName.desktop

[0] https://help.ubuntu.com/community/UnityLaunchersAndDesktopFiles

Re: Review-request for 8048731: JEP 272: Platform-Specific Desktop Features

[Alexander Zvegintsev]

It is only on the AWT side currently.
There is no such documentation yet, however you can see the mapping at 
[0] and [1].


[0] 
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/macosx/classes/sun/lwawt/macosx/CDesktopPeer.java.udiff.html
[1] 
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/00/src/java.desktop/macosx/classes/sun/lwawt/macosx/CTaskbarPeer.java.html


Thanks,

Alexander.

On 11/19/2015 01:17 AM, Michael Hall wrote:
On Nov 18, 2015, at 1:12 AM, Alexander Zvegintsev 
<alexander.zvegint...@oracle.com 
<mailto:alexander.zvegint...@oracle.com>> wrote:


This fix provides public support Mac OS X features(com.apple.{eawt,eio}),


Is replacement of the Apple legacy API’s going to done between this 
area, awt-dev, and the javafx group? I thought I saw a post on the 
javafx list that they were going to try and make sure clickable file 
handling was correctly handled by javafx api’s.
Is there going to be some central place where the mapping between the 
old api’s and the new ones is documented? Also indicating what api’s 
will no longer be supported.


Michael Hall

Review-request for 8048731: JEP 272: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hello

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8048731/
for
https://bugs.openjdk.java.net/browse/JDK-8048731

This fix provides public support Mac OS X 
features(com.apple.{eawt,eio}), adds support for various desktop

features such as progress indication, dock overlays, dock menus, etc.
This is done by extending java.awt.Desktop and adding java.awt.Taskbar 
classes


Linux support is limited by Unity, however this is not the only 
limitation :) An app should be run via
AppName.desktop file [0] with specified system property 
-Djava.desktop.appName=AppName.desktop


[0] https://help.ubuntu.com/community/UnityLaunchersAndDesktopFiles


--
--
Thanks,
Alexander.

Re: JDK9 Review Request for 8037575: JFrame on Windows doesn't animate when setting ICONIFIED state

[Alexander Zvegintsev]

Looks good to me too.

--
Thanks,
Alexander.

On 11/16/2015 08:30 AM, Rajeev Chamyal wrote:

Hello All,

I need one more review for this bug.
Can someone please review it.
http://cr.openjdk.java.net/~psadhukhan/rajeev/8037575/webrev.00/

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 11 November 2015 19:57
To: Rajeev Chamyal
Cc: Sergey Bylokhov; Alexander Zvegintsev; awt-dev@openjdk.java.net
Subject: Re:  JDK9 Review Request for 8037575: JFrame on Windows 
doesn't animate when setting ICONIFIED state


The fix looks good to me.

Thanks,
Alexandr.

On 10/13/2015 11:12 AM, Rajeev Chamyal wrote:

Hello All,

Please review the following webrev.

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

Webrev:
http://cr.openjdk.java.net/~psadhukhan/rajeev/8037575/webrev.00/
<http://cr.openjdk.java.net/%7Epsadhukhan/rajeev/8037575/webrev.00/>

Regards,

Rajeev Chamyal

*From:*Rajeev Chamyal
*Sent:* 29 September 2015 16:41
*To:* Sergey Bylokhov; Alexander Zvegintsev;
swing-...@openjdk.java.net; Alexander Scherbatiy
*Subject:*  JDK9 Review Request for 8037575: JFrame on
Windows doesn't animate when setting ICONIFIED state

Hello,

Please review the following fix for Jdk9:
   
Bug:https://bugs.openjdk.java.net/browse/JDK-8037575


Webrev:
http://cr.openjdk.java.net/~psadhukhan/rajeev/8037575/webrev.00/
<http://cr.openjdk.java.net/%7Epsadhukhan/rajeev/8037575/webrev.00/>

Issue: JFrame with ICONIFIED state on Windows is not minimizing
gradually with animation it simply disappears.

Fix: Added a call to ShowWindow function to set the specified Frame show state 
ICONIFIED/ZOOM etc.
   
Regards,

Rajeev Chamyal

Re: [9] Review Request: 8011541 [TEST_BUG] closed/javax/swing/plaf/metal/MetalUtils/bug6190373.java fails NPE since 7u25b03

[Alexander Zvegintsev]

looks fine to me.

--
Thanks,
Alexander.

On 10/21/2015 07:32 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

The test emulate implementation of two applets inside one vm. It has 
two threads in two different threads groups, for one of them it 
creates an appcontext, and the test assumes that for the main thread 
the appcontext will be created automatically. This assumption is 
incorrect since we removed main appcontext long time ago.


In the fix:
- Everything from the main thread were moved to the second threads group
- Some synchronization and call to dispose() were added
- Now we iterates over all supported l

I have checked that the updated test failed if the fix for JDK-6190373 
is reverted.

The test was moved to open.

Bug: https://bugs.openjdk.java.net/browse/JDK-8011541
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8011541/webrev.00
Only useful changes for convenience: 
http://cr.openjdk.java.net/~serb/8011541/webrev.diff

Re: [9] Review Request for 8081457: TrayIcon tests fail in OEL 7 only

[Alexander Zvegintsev]

Hi Semyon,

As I can see there are a lot similar checks in tests for OEL7, ideally 
it should be checked against  gnome-shell(probably from DESKTOP_SESSION 
env variable, or check for running gnome-shell process), but I am OK 
with the current check. Could you please extract this check to some 
helper method? It would be helpful, if we will change this check in future.


Thanks,

Alexander.

On 10/13/2015 02:24 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

In OEL7 the system tray was changed a lot. It is totally hidden by 
default and its tray icons only capable to receive click events. Mouse 
motion events and double click is not supported in the tray.
Also input events are sent to the embedded frame instead of the canvas 
with the icon. The mouse press event is sent to the embedded frame 
only after button is released.
In the proposed fix the mouse click listener is added to the embedded 
frame. The tray icon test suite is modified to detect and pass OEL7 
system tray.


--Semyon

Re: Review Request for JDK-8040322 : TextArea.replaceRange() and insert() are broken with setText(null)

[Alexander Zvegintsev]

Looks fine to me too.

Thanks,

Alexander.

On 09/28/2015 02:11 PM, Ambarish Rapte wrote:

Hi Sergey,
Thanks for the review.
Executed regression tests for TextArea, TextField & also other tests 
which use any of the TextArea, TextField or TextComponent.
=> There are NO failures due this patch change.

Dear All,
One more review is required for this patch.
Please review.


Many Thanks,
Ambarish Rapte

-Original Message-
From: Sergey Bylokhov
Sent: Wednesday, September 23, 2015 6:36 PM
To: Ambarish Rapte; Alexander Scherbatiy; awt-dev@openjdk.java.net
Subject: Re: Review Request for JDK-8040322 : TextArea.replaceRange() and 
insert() are broken with setText(null)

The fix looks fine to me. Thanks.
You can also run related regression tests from java/awt to check that there are 
no regressions.

On 21.09.15 12:38, Ambarish Rapte wrote:

Hi,

  The previous mail for review of this issue
JDK-8040322,

  is filtered with another issue & issues are merged.

  So I am writing a new mail to continue the review on
separate thread.

Dear Sergey,

  Thanks for the review. Updated the patch according to
your review comments.
  Please take a look.

  Please review this patch at

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

  Webrev :
http://cr.openjdk.java.net/~psadhukhan/ambarish/8040322/webrev.01/

Below is history from previous mail discussion,

Hi, Ambarish.

A few comments.

- The fix changed the order of methods calls. Note that if the user
overrides the "appendText" then it is called after an "append".

- It seems that before the fix we always tried to use the empty
string for a null text(we replace null to "" in the constructor and
setText), the new code should maintain the same assumption.

On 18.09.15 14:36, Ambarish Rapte wrote:

  > Hi,

  >

  >  Please review the following fix for jdk9.

  >

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

  > Webrev:http://cr.openjdk.java.net/~psadhukhan/ambarish/8040322/webrev.

  > 00/

  >

  > Issue:

  > /TextArea.setText(null)/   does not set TextArea text to

  > /null/ when called after

  > /TextArea.replaceRange(), TextArea.insert(),  TextArea.append()./

  >

  > Cause:

  >  Variable "/text/"  was not correctly updated in

  >

  > /TextArea.replaceRange(), TextArea.insert(),  TextArea.append()/
in

  > awt / TextArea.java

  >

  > Fix:

  >  Update the variable "text" correctly with the calls

  > to

  >

  > /TextArea.replaceRange(), TextArea.insert(),  TextArea.append()/

  >

  > In file awt / TextArea.java

  >

  > Many Thanks,

  > Ambarish Rapte

  >

--

Best regards, Sergey.

Many Thanks,
Ambarish Rapte



--
Best regards, Sergey.

Re: JEP 272: Platform-Specific Desktop Features

[Alexander Zvegintsev]

Hi Anthony,

Thank you for your feedback!
It seems correct and appropriate to me, so I'll incorporate your 
suggestions into the JEP.


Thanks,

Alexander.

On 09/27/2015 09:42 AM, Anthony Vanelverdinghe wrote:

Hi

I have a few questions/comments about the proposed API:
- why not add a class java.awt.Taskbar instead, containing all the 
functionality of the "setDock..." methods? In my opinion, this would 
be a more logical place for this. Plus it would have the nice 
side-effect that "SystemTray" and "Taskbar" would be adjacent to each 
other in the Javadoc listing, aiding even more in discoverability.
- I'm not fond of the naming of the listeners. In my opinion, 
AppEventListener should be renamed to SystemEventListener, and 
listeners for "external" events (such as 
ScreenSleep/SystemSleep/UserSession) should not be prefixed with App. 
So for example you would have UserSessionListener which extends 
SystemEventListener instead of the current AppUserSessionListener 
which extends AppEventListener.
- on a final note: the correct spelling is "reopen", so 
AppReOpenedListener should be renamed to AppReopenedListener.


Thanks in advance for any feedback.

Kind regards,
Anthony

Re: [9] Review Request for 8022334: After calling frame.toBack() dialog goes to the back on Ubuntu 12.04

[Alexander Zvegintsev]

The fix looks good to me.

Thanks,

Alexander.

On 09/29/2015 08:51 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

The Unity WM cannot switch top-level windows of different peers inside 
one Java app, because they have different WM_CLASS property and 
different WM_CLASS are treated as different applications. This issue 
is the root cause why a single Java app that contains multiple 
top-level windows is shown as several groups of apps in the taskbar.
And it makes impossible to "lock" multi-window Java app in the Unity 
taskbar because desktop shortcut should contain one WM_CLASS per app.
As a fix it is proposed to use the same WM_CLASS value for all peer 
classes.


--Semyon

Re: RFR: JDK-8134808 Remove support for serialized applets from java.desktop

[Alexander Zvegintsev]

+1

--
Thanks,
Alexander.

On 15.09.2015 14:31, Sergey Bylokhov wrote:

Looks fine.

14.09.2015 22:12, Daniil Titov wrote:

Hello,

Please review java.desktop part of the changes for jdk9 that remove 
the support for serialized applets. Support for serialized applets 
was added in JDK 1.1 to allow an applet to be shipped in a 
pre-initialized state. Today, with modern compression and JVM 
performance there is no benefit to continuing to support this 
technology.


Jira issue: https://bugs.openjdk.java.net/browse/JDK-8134808
Webrev: http://cr.openjdk.java.net/~dtitov/8134808/webrev.00/

Thanks!

Best regards,
Daniil

Re: < AWT- Dev> [9] Review Request:JDK-8136354 [TEST_BUG]Test java/awt/image/RescaleOp/RescaleAlphaTest.java with Bad action for script

[Alexander Zvegintsev]

looks fine

Thanks,

Alexander.

On 09/11/2015 01:09 PM, pooja chopra wrote:

Hello,

Please review a fix for issue :-

8136354 [TEST_BUG]Test java/awt/image/RescaleOp/RescaleAlphaTest.java 
with Bad action for script

Test bug fix.
https://bugs.openjdk.java.net/browse/JDK-8136354
The webrev is : http://cr.openjdk.java.net/~pchopra/8136354/webrev.00/

Regards,
Pooja

Re: [9] Review Request for 8014725: closed/java/awt/Clipboard/HTMLTransferTest/HTMLTransferTest.html failed intermittently

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 09/04/2015 05:43 PM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 9/3/2015 4:21 PM, Semyon Sadetsky wrote:

Alexanders? Any questions?

On 8/5/2015 3:11 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

The test is moved from closed to open repo. Initially it was a test 
issue: new document html flavor parameters introduced in 7075105 
were not taken into account.
After the test is fixed and run an issue was found in the code. In 
case InputStream is used as transferable object it is converted to a 
number of compatible formats but during this conversion InputStream 
can be read many times without resetting its position after each 
read. As result the InputStream is exhausted during the first flavor 
conversion and then all consequent flavors receive empty content. 
Stream reset was added as solution to this.


--Semyon

Re: [9] Review request for 8133453 Remove AWTKeyStroke.registerSubclass(Class) method

[Alexander Zvegintsev]

+1

--
Thanks,
Alexander.

On 01.09.2015 16:39, Sergey Bylokhov wrote:

Looks fine to me.

On 01.09.15 13:49, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8133453
   webrev: http://cr.openjdk.java.net/~alexsch/8133453/webrev.00

   The class KeyStroke has been added to the javax.swing package first
and then copied to the java.awt.AWTKeyStroke class.
   Most of KeyStroke class methods looks like registering KeyStroke
class and call to the parent AWTKeyStroke method.

   AWTKeyStroke class is intended to contain keyChar, keyCode,
modifiers, and keyRelease flag and it seems
   there are no reasons that this class can be subclassed.

   The suggestion is to remove the AWTKeyStroke.registerSubclass(Class)
method.

   The AWTKeyStroke.registerSubclass(Class) method can be deprecated so
only AWTKeyStroke and KeyStroke instances is allowed to be created.
   The result will be that a user code get ClassCastException when
casting a returned key stroke to the custom key stroke which has the
similar effect.

   Removing the method in question leads to the binary compatibility
issue if someone uses the method. The following code will not work:
   --
 public class CustomKeyStroke extends AWTKeyStroke {

 public static void registerCustomKeyStroke() {
AWTKeyStroke.registerSubclass(CustomKeyStroke.class); //
removed method
 }
 }

 CustomKeyStroke.registerCustomKeyStroke();
 AWTKeyStroke keyStroke = AWTKeyStroke.getAWTKeyStroke('C');
 CustomKeyStroke customKeyStroke = (CustomKeyStroke) keyStroke; //
CustomKeyStroke instance is not returned
   --

   The proposed workaround is to use a map between unique AWTKeyStroke
instances and custom key strokes:
   --
 private static HashMap keyStrokeMap
= new HashMap<>();

 public static CustomKeyStroke getCustomKeyStroke(char c) {
 AWTKeyStroke ketStroke = AWTKeyStroke.getAWTKeyStroke(c);
 CustomKeyStroke customKeyStroke = keyStrokeMap.get(ketStroke);

 if (customKeyStroke == null) {
 customKeyStroke = new CustomKeyStroke(ketStroke);
 keyStrokeMap.put(ketStroke, customKeyStroke);
 }

 return customKeyStroke;
 }
   --


   AWT/Swing library only registers KeyStroke class
(AWTKeyStroke.registerSubclass(KeyStroke.class) calls in the KeyStroke
class)
   and never registers AWTKeyStroke.class.
   After the first KeyStroke.getKeyStroke(...) call in the keyStroke
class only KeyStroke instances will be created by AWTKeyStroke.

   The solution proposed in the fix is to always create
javax.swing.KeyStroke instances.

Thanks,
Alexandr.

Re: AWT Dev [9] Review Request: 8134603 Incorrect destination is used in CGLLayer surface

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 08/28/2015 01:30 PM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 8/27/2015 4:49 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.
The CGLLayer.java has a typo it uses the peer as a destination, but 
the target should be used instead. This breaks the bufferPerWIndow 
functionality, because this destination is used in 
BufferStrategyPaintManager.



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

Re: AWT Dev Swing Dev [9] Review Request:JDK-8130481 [TEST_BUG] javax/swing/JPopupMenu/6583251/bug6583251.java failed with UnsupportedOperation exception

[Alexander Zvegintsev]

BTW, there is no need in try/catch block.

Thanks,

Alexander.

On 08/27/2015 03:08 PM, Alexander Scherbatiy wrote:

On 8/27/2015 1:12 PM, pooja chopra wrote:

Hi Alexandr,

Please review updated webrev link :-

http://cr.openjdk.java.net/~pchopra/8130481/webrev.06/



There are some problems with the code indentation like :
   --
if (SystemTray.isSupported()) {
SwingUtilities.invokeAndWait(new Runnable() {
public void run() {
createGui();
}
});
  ---
  We have some requirements about it. See for example:
 Java Code Conventions: 
http://www.oracle.com/technetwork/java/codeconventions-150003.pdf
 new discussed Java Style Guidelines: 
http://cr.openjdk.java.net/~alundblad/styleguide


  You could also use code formatting in your IDE to properly format 
the code.


  Thanks,
  Alexandr.



Regards,
Pooja
On 8/5/2015 2:46 PM, Alexander Scherbatiy wrote:

On 8/5/2015 10:31 AM, pooja chopra wrote:

Hi Alexandr,

Please review  update webrev  link :-

The webrev is  : 
http://cr.openjdk.java.net/~pchopra/8130481/webrev.03/


This looks better. There are just minor issues:
- run swing components on EDT like: menu.show(frame, 0, 0)
- format the code in 'if' block or check the mercurial settings 
in .hgrc file like diff = -w  // ignore white space when comparing 
lines


  Thanks,
  Alexandr.



Regards,
Pooja
On 7/29/2015 4:04 PM, Alexander Scherbatiy wrote:

On 7/29/2015 9:12 AM, pooja chopra wrote:

Hello Sergey ,
Please review  update webrev link below :-

The webrev is : 
http://cr.openjdk.java.net/~pchopra/8130481/webrev.02/


   After the SystemTray.isSupported()  is checked it is not 
expected that the UnsupportedOperationException is thrown. The 
test should fail in this case.


  Thanks,
  Alexandr.



Regards,
Pooja
On 7/16/2015 12:32 AM, Sergey Bylokhov wrote:

Hello,
I suggest to check support of systemTray at the beginning of the 
test.


On 15.07.15 13:07, pooja chopra wrote:

Hello ,

Corrected the webrev link below . Please review.

Regards,
Pooja
On 7/15/2015 3:26 PM, pooja chopra wrote:

Hello,

Please review a fix for issue :-

8130481 [TEST_BUG] 
javax/swing/JPopupMenu/6583251/bug6583251.java failed with 
UnsupportedOperation exception

Test bug fix.
https://bugs.openjdk.java.net/browse/JDK-8130481
The webrev is : 
http://cr.openjdk.java.net/~pchopra/8130481/webrev.01/


Regards,
Pooja

Re: AWT Dev [9] Review Request: 8047226 closed/java/awt/Component/GetScreenLocTest/GetScreenLocTest.html clicks on Unity's tool bar

[Alexander Zvegintsev]

Looks fine.

Thanks,

Alexander.

On 08/25/2015 06:28 PM, Sergey Bylokhov wrote:

On 25.08.15 17:55, Alexander Zvegintsev wrote:
I think that there is no need in RuntimeException in mousePressed() 
callback,

it could be replaced with println and break.

makes sense, the new version:
http://cr.openjdk.java.net/~serb/8047226/webrev.01



Thanks,

Alexander.

On 08/25/2015 05:36 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9. Test was fixed(the windows were 
moved to the center of the screen), cleaned and was moved to the 
open. The diff to the previous version is provided [1].


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

[1] http://cr.openjdk.java.net/~serb/8047226/closed.00

Re: AWT Dev [9] Review Request: 8047226 closed/java/awt/Component/GetScreenLocTest/GetScreenLocTest.html clicks on Unity's tool bar

[Alexander Zvegintsev]

Hello Sergey,

I think that there is no need in RuntimeException in mousePressed() 
callback,

it could be replaced with println and break.

Thanks,

Alexander.

On 08/25/2015 05:36 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9. Test was fixed(the windows were moved 
to the center of the screen), cleaned and was moved to the open. The 
diff to the previous version is provided [1].


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

[1] http://cr.openjdk.java.net/~serb/8047226/closed.00

AWT Dev [9] Request for review 8134028: [PIT] XToolkit, strange behavior of robot.createScreenCapture(): looks like a native crash in X11/GTK

[Alexander Zvegintsev]

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8134028/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8134028

In previous robot fix I forgot to wrap gtk calls with 
gdk_threads_enter/gdk_threads_leave, therefore we have such failure.


--
Thanks,

Alexander.

Re: AWT Dev Swing Dev [9] Review Request: JDK-8081764 [TEST_BUG] Test javax/swing/plaf/aqua/CustomComboBoxFocusTest.java fails on Solaris Sparcv9 and Linux but passes on MacOSX

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 07/29/2015 01:39 PM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 7/29/2015 10:00 AM, pooja chopra wrote:

Hi ,
As default look and feel in mac is aqua only so there is no need of 
setting system look and feel explicitly.


Please review updated webrev link below:-

8081764  [TEST_BUG] Test 
javax/swing/plaf/aqua/CustomComboBoxFocusTest.java fails on Solaris 
Sparcv9 and Linux but passes on MacOSX

Test bug fix.
https://bugs.openjdk.java.net/browse/JDK-8081764
The webrev is : http://cr.openjdk.java.net/~pchopra/8081764/webrev.01/

Regards,
Pooja

On 6/11/2015 2:23 PM, pooja chopra wrote:

Hi Andrew ,
The test  passes with GTKLookAndFeel on Solaris but with out 
GTKLookAndFeel test fails with same error as mentioned in the bug . 
I had explicitly set the look and feel as system look and feel as 
test was to be run for Mac only and this test was placed in 
plaf/aqua and it is comparing screenshots so I thought in other look 
and feel there can be a possibility that this matching fails. Please 
let me know if some changes are required .

Regards,
Pooja
On 6/9/2015 1:15 PM, Andrew Brygin wrote:

Hello Pooja,

 In general I tend to agree with idea to limit the scope of the 
test by macosx only.


 However, could you please clarify following questions:
a) the test failure on linux/solaris: doesn't it indicate a similar 
problem with gtk laf, does it?

b) what is purpose of the explicit LaF setup (lines 68 - 69)?

Thanks,
Andrew

On 6/8/2015 12:59 PM, pooja chopra wrote:

Hi All,
Correcting the webrev link below . Please review below fix .
Regards,
Pooja
On 6/8/2015 3:27 PM, pooja chopra wrote:

Hello,

Please review a fix for issue :-
8081764  [TEST_BUG] Test 
javax/swing/plaf/aqua/CustomComboBoxFocusTest.java fails on 
Solaris Sparcv9 and Linux but passes on MacOSX

Test bug fix.
https://bugs.openjdk.java.net/browse/JDK-8081764
The webrev is : 
http://cr.openjdk.java.net/~pchopra/8081764/webrev.00


Regards,
Pooja

Re: AWT Dev [9] Review Request: 7124271 [macosx] RealSync test failure

[Alexander Zvegintsev]

Hello Sergey,

the fix looks good to me.

--
Thanks,
Alexander.

On 10.08.2015 14:21, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.
Two bugs were fixed:
 - The changes in NSApplicationAWT.waitForDummyEvent() is a fix for a 
regression of JDK-8080504, which does not take into account that we 
should not call an unlock if a lock operation was failed with timeout.
 - Initial bug which was filed is about use of two different API to 
post events to the application, [NSApplication postEvent] and 
CGEventPost(). It was found that if we post an event via CGEventPost 
in robot and immediately after this we will post the second event via 
[NSApp postEvent] in realsync then sometimes the second event will be 
handled first. The opposite isn't proved, but I changed the code to 
use both to be safer.


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

Re: AWT Dev [9] Review Request: 4379403 Need to disable the magic AWT dump key (CTRL+SHIFT+F1)

[Alexander Zvegintsev]

Hello Sergey,

the fix looks good to me.

Thanks,

Alexander.

On 08/10/2015 02:35 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.
After the fix a Magic dump key is disabled by default, to enable it 
the user needs to enable debug mode(via property file or system 
property).


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

Re: AWT Dev JDK-8015471 Deadlock or infinit loop in XAWT on Ubuntu 12.04

[Alexander Zvegintsev]

Hi Cristian,

it looks like that I've already fixed this issue as [1]
Could you please check if the problem is gone with 8u60?

[1] https://bugs.openjdk.java.net/browse/JDK-8129116 Deadlock with 
multimonitor fullscreen windows.


Thanks,

Alexander.

On 07/30/2015 05:23 PM, Christian wrote:
Sorry if you dont want bug talk here, I just feel that i need to send 
some information somewhere. I'm talking about


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

This exact issue happen to me quite frequently. I'm running the below 
jdk version, but it has happened during 1.7.0 jdks also. The bug is 
closed as not reproducable.


java version 1.8.0_45
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

By running jstack -l on the jvm I see these threads in deadlock with 
each other.


AWT-EventQueue-0 #21 prio=6 os_prio=0 tid=0x7fa8b05ec800 
nid=0x316c waiting on condition [0x7fa892e4f000]

   java.lang.Thread.State: WAITING (parking)
at sun.misc.Unsafe.park(Native Method)
- parking to wait for  0xf064a248 (a 
java.util.concurrent.locks.ReentrantLock$NonfairSync)
at 
java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
at 
java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:836)
at 
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(AbstractQueuedSynchronizer.java:870)
at 
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1199)
at 
java.util.concurrent.locks.ReentrantLock$NonfairSync.lock(ReentrantLock.java:209)
at 
java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:285)

at sun.awt.SunToolkit.awtLock(SunToolkit.java:253)
at sun.awt.X11GraphicsDevice.getConfigVisualId(Native Method)
at 
sun.awt.X11GraphicsDevice.makeDefaultConfiguration(X11GraphicsDevice.java:235)
at 
sun.awt.X11GraphicsDevice.getDefaultConfiguration(X11GraphicsDevice.java:227)

- locked 0xf062a418 (a java.lang.Object)
at 
javax.swing.RepaintManager.getDoubleBufferMaximumSize(RepaintManager.java:1153)
at 
javax.swing.RepaintManager.getVolatileOffscreenBuffer(RepaintManager.java:1035)
at 
javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1482)

at javax.swing.RepaintManager.paint(RepaintManager.java:1265)
at javax.swing.JComponent.paint(JComponent.java:1042)
at 
java.awt.GraphicsCallback$PaintCallback.run(GraphicsCallback.java:39)
at 
sun.awt.SunGraphicsCallback.runOneComponent(SunGraphicsCallback.java:79)
at 
sun.awt.SunGraphicsCallback.runComponents(SunGraphicsCallback.java:116)

at java.awt.Container.paint(Container.java:1973)
at java.awt.Window.paint(Window.java:3912)
at javax.swing.RepaintManager$4.run(RepaintManager.java:835)
at javax.swing.RepaintManager$4.run(RepaintManager.java:807)
at java.security.AccessController.doPrivileged(Native Method)
at 
java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:75)
at 
javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:807)
at 
javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:782)
at 
javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:731)

at javax.swing.RepaintManager.access$1300(RepaintManager.java:64)
at 
javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1720)
at 
java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)

at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:756)
at java.awt.EventQueue.access$500(EventQueue.java:97)
at java.awt.EventQueue$3.run(EventQueue.java:709)
at java.awt.EventQueue$3.run(EventQueue.java:703)
at java.security.AccessController.doPrivileged(Native Method)
at 
java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:75)

at java.awt.EventQueue.dispatchEvent(EventQueue.java:726)
at 
java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
at 
java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
at 
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
at 
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at 
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)

at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

   Locked ownable synchronizers:
- None

AWT-Shutdown #22 prio=5 os_prio=0 tid=0x7fa8b05eb000 nid=0x316b 
in Object.wait() [0x7fa892f53000]

   java.lang.Thread.State: WAITING (on object monitor)
at java.lang.Object.wait(Native Method)
- waiting on 

Re: AWT Dev [9] Review request for 8014212: Robot captures black screen

[Alexander Zvegintsev]

Hello Sergey,

please see the updated version
http://cr.openjdk.java.net/~azvegint/jdk/9/8014212/02/
GetPrimitiveArrayCritical() is used instead of manual array allocation.

Thanks,

Alexander.

On 07/27/2015 08:35 PM, Alexander Zvegintsev wrote:

Please see the updated version:
http://cr.openjdk.java.net/~azvegint/jdk/9/8014212/01/

robot makes shot of intersection of provided rectangle with root 
window. Old robot implementation works for negative x and y,
but may return garbage outside of root window behavior, so it was 
updated too.


ShapeNotSetSometimes reduced iteration count to prevent killing by 
jtreg for timeout.


TranslucentJAppletTest updated to use getPixelColor() directly.
Thanks,

Alexander.
On 07/24/2015 07:08 AM, Sergey Bylokhov wrote:

Hi, Alexander.
I am not sure that an assumption that we should not make a screen 
shot if the x and y are negative is correct. I guess that we should 
take a screenshot of the intersection of the requested area and the 
display.


Can you also check this comment in the TranslucentJAppletTest.java:
 106 // unfortunately, robot.getPixelColor() doesn't work for 
some unknown reason

 107 // Color newColor2 = r.getPixelColor(200, 200);

On 23.07.15 19:37, Alexander Zvegintsev wrote:

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8014212/00
for the issue
https://bugs.openjdk.java.net/browse/JDK-8014212

This fix trying to use gdk_pixbuf_get_from_drawable () [0] to 
capture screenshot(basic image data handling could be found here[1]).

If it fails then we come back to our old method.
Translucency support[1] is also covered by this fix, but there is 
another issue with toFront() call which blocks passing of a JCK test.


P.S. On Solaris 11 windows appears with animation, thus sleep() call 
was added to work around this.


[0] 
https://developer.gnome.org/gdk2/stable/gdk2-Pixbufs.html#gdk-pixbuf-get-from-drawable
[1] 
https://developer.gnome.org/gdk-pixbuf/stable/gdk-pixbuf-The-GdkPixbuf-Structure.html#image-data
[2] https://bugs.openjdk.java.net/browse/JDK-7043845 Robot should be 
able take screenshots with translucent windows


--
Thanks,

Alexander.



--
Best regards, Sergey.

Re: AWT Dev [9] Review Request: 8132355 Incorrect guard block in HPkeysym.h, awt_Event.h

[Alexander Zvegintsev]

Looks fine.

Thanks,

Alexander.

On 07/27/2015 02:36 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for a typo in jdk9.
The HPkeysym.h is from external library, so I have filed the bug in 
upstream too:

https://bugs.freedesktop.org/show_bug.cgi?id=91469

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

Re: AWT Dev [9] Review request for 8014212: Robot captures black screen

[Alexander Zvegintsev]

Please see the updated version:
http://cr.openjdk.java.net/~azvegint/jdk/9/8014212/01/

robot makes shot of intersection of provided rectangle with root window. 
Old robot implementation works for negative x and y,
but may return garbage outside of root window behavior, so it was 
updated too.


ShapeNotSetSometimes reduced iteration count to prevent killing by jtreg 
for timeout.


TranslucentJAppletTest updated to use getPixelColor() directly.

Thanks,

Alexander.

On 07/24/2015 07:08 AM, Sergey Bylokhov wrote:

Hi, Alexander.
I am not sure that an assumption that we should not make a screen shot 
if the x and y are negative is correct. I guess that we should take a 
screenshot of the intersection of the requested area and the display.


Can you also check this comment in the TranslucentJAppletTest.java:
 106 // unfortunately, robot.getPixelColor() doesn't work for 
some unknown reason

 107 // Color newColor2 = r.getPixelColor(200, 200);

On 23.07.15 19:37, Alexander Zvegintsev wrote:

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8014212/00
for the issue
https://bugs.openjdk.java.net/browse/JDK-8014212

This fix trying to use gdk_pixbuf_get_from_drawable () [0] to capture 
screenshot(basic image data handling could be found here[1]).

If it fails then we come back to our old method.
Translucency support[1] is also covered by this fix, but there is 
another issue with toFront() call which blocks passing of a JCK test.


P.S. On Solaris 11 windows appears with animation, thus sleep() call 
was added to work around this.


[0] 
https://developer.gnome.org/gdk2/stable/gdk2-Pixbufs.html#gdk-pixbuf-get-from-drawable
[1] 
https://developer.gnome.org/gdk-pixbuf/stable/gdk-pixbuf-The-GdkPixbuf-Structure.html#image-data
[2] https://bugs.openjdk.java.net/browse/JDK-7043845 Robot should be 
able take screenshots with translucent windows


--
Thanks,

Alexander.



--
Best regards, Sergey.

AWT Dev [9] Review request for 8014212: Robot captures black screen

[Alexander Zvegintsev]

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8014212/00
for the issue
https://bugs.openjdk.java.net/browse/JDK-8014212

This fix trying to use gdk_pixbuf_get_from_drawable () [0] to capture 
screenshot(basic image data handling could be found here[1]).

If it fails then we come back to our old method.
Translucency support[1] is also covered by this fix, but there is 
another issue with toFront() call which blocks passing of a JCK test.


P.S. On Solaris 11 windows appears with animation, thus sleep() call was 
added to work around this.


[0] 
https://developer.gnome.org/gdk2/stable/gdk2-Pixbufs.html#gdk-pixbuf-get-from-drawable
[1] 
https://developer.gnome.org/gdk-pixbuf/stable/gdk-pixbuf-The-GdkPixbuf-Structure.html#image-data
[2] https://bugs.openjdk.java.net/browse/JDK-7043845 Robot should be 
able take screenshots with translucent windows


--
Thanks,

Alexander.

AWT Dev [9] Review request for 8131752 [Regression] Test java/awt/GraphicsDevice/CheckDisplayModes.java fails

[Alexander Zvegintsev]

Hello,

Please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8131752/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8131752

This fix simply restores fullscreen windows check. Actually fullscreen 
is not necessary to change display mode,
but since I am planning  to backport it to 8u66 it is too risky to 
change behavior. It should be reconsidered along with 8022810 [0].


[0] https://bugs.openjdk.java.net/browse/JDK-8022810 Cannot list all the 
available display modes on Ubuntu linux in case of two screen devices


--
Thanks,

Alexander.

Re: AWT Dev [9] Review request for 8130478 Reconsider awt.toolkit property usage in java.awt.Toolkit getDefaultToolkit() method

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 07/22/2015 12:14 AM, Sergey Bylokhov wrote:

+1
On 21.07.15 21:24, Phil Race wrote:

Seems fine to me.
Since this is specification visible you will need an approved CCC 
request before commiting the change.


-phil.

On 07/21/2015 07:49 AM, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8130478
  webrev: http://cr.openjdk.java.net/~alexsch/8130478/webrev.00

  The fix suggest to remove the awt.toolkit property mentioning 
from the public API because
  it is not so useful to use it in external programs since the peers 
refactoring.


  For more details see 
http://mail.openjdk.java.net/pipermail/awt-dev/2015-February/008924.html 



Thanks,
Alexandr.

Re: AWT Dev Awt Dev [9] Review Request for 8130769: The new menu can't be shown on the menubar after clicking the Add button.

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 07/22/2015 10:51 AM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 7/22/2015 12:32 AM, Sergey Bylokhov wrote:

Hi, Semyon.
The fix looks good.

On 16.07.15 19:08, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

This is regression from JDK-7155957. Menu should be added to MenuBar 
collection before the adding it to MenuBar's peer otherwise it is 
not painted.


--Semyon

Re: AWT Dev [9] Review Request: 8067093 Fix windows-specific deprecation warnings in the java.desktop module

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 07/22/2015 11:49 AM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 7/22/2015 1:01 AM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

In the fix I replace the usage of the deprecated api in WListPeer. 
Most of the other cases in the bug description are related to 
getPeer(), which was already removed.
The usage of deprecated constructor of Win32GraphicsConfig in its 
subclasses was not changed.


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

Re: AWT Dev [9] Review Request for 8131673: [TEST_BUG] add @modules to some OS X-specific regtests

[Alexander Zvegintsev]

+1

Thanks,

Alexander.

On 07/16/2015 06:07 PM, Sergey Bylokhov wrote:

The fix looks fine.
On 16.07.15 17:16, Yuri Nesterenko wrote:

I cannot, however, easily fix them for OS X 10.10 where full screen
button is in different place. Let's make another bug for that.

Current CR is
https://bugs.openjdk.java.net/browse/JDK-8131673
Webrev is
http://cr.openjdk.java.net/~yan/8131673/webrev.00/

Thanks,
-yan

Re: AWT Dev Awt Dev [9] Review Request for 8025815: Child FileDialog of modal dialog does not get focus on Gnome

[Alexander Zvegintsev]

still looks good to me.

Thanks,

Alexander.

On 07/15/2015 03:59 PM, Semyon Sadetsky wrote:

Hi Alexander,

In the next version I have updated the test and Sergey's notes are 
taken into account.


http://cr.openjdk.java.net/~ssadetsky/8025815/webrev.01/

--Semyon

On 7/13/2015 7:21 PM, Alexander Zvegintsev wrote:

Hello Semyon,

the fix itself look good to me, but the test seems to be broken, it 
passes for me before the fix and fails after( Ubuntu 14.04, Unity)


Thanks,

Alexander.

On 07/13/2015 04:59 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

The root cause is a mixing of GTK and XLib windows. Normally Java 
uses XLib windows and manage focus transitivity on its side. But for 
the file chooser we use a native GTK widget if it is available and 
this widget is crated without parent window because we don't have 
any other GTK windows. This breaks the focus transitivity chain on 
Java side because focus is managed by the WM for GTK dialog. Without 
parent window the GTK file dialog is managed by the WM as a detached 
standalone window. So modality and focus transitivity with Java 
windows do not work correctly: the dialog can be overlapped by other 
java windows and focus can be transferred to them.
The proposed solution obtains XID of the GTK dialog window and pass 
it to the java side to take into account in the focus transitivity 
chain by sending messages to the WM.


Yet another issue I found in the GTK file chooser code is a dispose 
concurrency issue. The GTK dialog widget is being created in a 
separate thread upon the setVisible(true) call but it can be hidden 
or disposed without any synchronization with this thread. It can 
prevent the VM from stopping if the dialog is hidden or disposed 
right after its creation. This also was fixed in the proposed solution.


--Semyon

Re: AWT Dev Awt Dev [9] Review Request for 8130242: DataFlavorComparator transitivity exception

[Alexander Zvegintsev]

looks good to me too.

Thanks,

Alexander.

On 07/08/2015 11:50 AM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 7/6/2015 5:33 PM, Semyon Sadetsky wrote:


Transitivity (2) violated:
X: 
java.awt.datatransfer.DataFlavor[mimetype=text/uri-list;representationclass=java.nio.ByteBuffer;charset=UTF-8], 

Y: 
java.awt.datatransfer.DataFlavor[mimetype=application/x-java-text-encoding;representationclass=java.io.InputStream], 

Z: 
java.awt.datatransfer.DataFlavor[mimetype=application/x-java-serialized-object;representationclass=java.lang.String]


X  Y  Z but X  Z

--Semyon


On 7/6/2015 5:21 PM, Alexander Scherbatiy wrote:

On 7/6/2015 2:17 PM, Semyon Sadetsky wrote:


Hello,

Please review fix for JDK9:

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

Data transfer's flavor comparator violates transitivity.


  Could you give an example of DataFlavors which violated the 
comparator transitivity contracts before the fix?


  Thanks,
  Alexandr.



--Semyon

Re: AWT Dev Awt Dev [9] Review Request for 8025815: Child FileDialog of modal dialog does not get focus on Gnome

[Alexander Zvegintsev]

Hello Semyon,

the fix itself look good to me, but the test seems to be broken, it 
passes for me before the fix and fails after( Ubuntu 14.04, Unity)


Thanks,

Alexander.

On 07/13/2015 04:59 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

The root cause is a mixing of GTK and XLib windows. Normally Java uses 
XLib windows and manage focus transitivity on its side. But for the 
file chooser we use a native GTK widget if it is available and this 
widget is crated without parent window because we don't have any other 
GTK windows. This breaks the focus transitivity chain on Java side 
because focus is managed by the WM for GTK dialog. Without parent 
window the GTK file dialog is managed by the WM as a detached 
standalone window. So modality and focus transitivity with Java 
windows do not work correctly: the dialog can be overlapped by other 
java windows and focus can be transferred to them.
The proposed solution obtains XID of the GTK dialog window and pass it 
to the java side to take into account in the focus transitivity chain 
by sending messages to the WM.


Yet another issue I found in the GTK file chooser code is a dispose 
concurrency issue. The GTK dialog widget is being created in a 
separate thread upon the setVisible(true) call but it can be hidden or 
disposed without any synchronization with this thread. It can prevent 
the VM from stopping if the dialog is hidden or disposed right after 
its creation. This also was fixed in the proposed solution.


--Semyon

Re: AWT Dev [9] Review Request: 8129894 NSApplicationAWT.m:343:72: error: comparison of constant 777 with expression of type 'NSEventSubtype'

[Alexander Zvegintsev]

Hello Sergey,

the fix looks good to me.

Thanks,

Alexander.

On 07/09/2015 01:49 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

This is the last issue, which prevents us to build the jdk on osx 
using the latest clang w/o --disable-warnings-as-errors.
This code was integrated in 
http://cr.openjdk.java.net/~anashaty/8068886/webrev.00. The new clang 
complains that our constant is not related to NSEventSubtype. But 
actually this is not a problem, because our custom events are 
NSApplicationDefined and subtype is a numeric identifier that further 
differentiates custom events[1].
To supress the warning we can change constant 777 to some value from 
NSEventSubtype. + small cleanup. I also tried to split the long lines 
but in this case the change became much bigger, because there are a 
number of such places.


[1] 
https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSEvent_Class/


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

Re: AWT Dev Awt Dev [9] Review Request for 8130390: Applet fails to launch on virtual desktop

[Alexander Zvegintsev]

looks fine to me.

Thanks,

Alexander.

On 07/07/2015 01:38 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

The FlipBufferStrategy did not take an Applet possibility into account.

--Semyon

Re: AWT Dev [9] Review Request: 8130525 Build failure on jdk9-client solaris-sparcv9

[Alexander Zvegintsev]

Looks fine to me.

Thanks,

Alexander.

On 07/06/2015 04:20 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

In the fix for JDK-7188942 I forgot to cleanup the mapfiles when I 
removed initPbuffer() method.


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

Re: AWT Dev [9] Review Request: JDK-8080953 [TEST_BUG]Test java/awt/FontClass/DebugFonts.java fails due to wrongly typed bugid

[Alexander Zvegintsev]

Looks fine to me.

Thanks,

Alexander.

On 07/02/2015 10:29 AM, pooja chopra wrote:

Hi All,
Please  review below fix.
Regards,
Pooja
On 6/17/2015 12:06 PM, pooja chopra wrote:

Hi All ,
Gentle Reminder. Please review below fix .
Regards,
Pooja
On 5/22/2015 10:34 PM, Sergey Bylokhov wrote:

cc 2d
On 22.05.15 19:59, pooja chopra wrote:

Hello,
Please review a fix for issue:
8080953 [TEST_BUG]Test java/awt/FontClass/DebugFonts.java fails due 
to wrongly typed bugid

Test bug fix.
https://bugs.openjdk.java.net/browse/JDK-8080953
The webrev is : http://cr.openjdk.java.net/~pchopra/8080953/webrev.00

Thanks,
Pooja

Re: AWT Dev RFR: JDK-8080246 JNLP app cannot be launched due to deadlock

[Alexander Zvegintsev]

Hi Daniil,

the fix looks good to me too.

--
Thanks,
Alexander.

On 24.06.2015 22:42, Daniil Titov wrote:


Hello,

Could you, please, review the following fix:

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

Webrev : http://cr.openjdk.java.net/~dtitov/8080246.8/ 
http://cr.openjdk.java.net/%7Edtitov/8080246.8/


It is a deadlock regression from  8u45.

Thanks!

Best regards,

Daniil

AWT Dev [9] Review request for 8129116: Deadlock with multimonitor fullscreen windows.

[Alexander Zvegintsev]

Hello

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8129116/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8129116

This deadlock occurs when we setting a fullscreen window on each 
graphics device sequentially without a delay.

EventQueue.invokeAndWait(() - {
for (GraphicsDevice device : devices) {
device.setFullScreenWindow(new Frame());
}
});

It happens even for non-exclusive fullscreen mode too (before the 
JDK-8051617[0] fix).


Currently we are dispatching events in XToolkit under the awtLock. The 
fix simply releases this acquired awtLock to avoid deadlock.
However we have a couple of issues ([1], [2]) with similar fixes. This 
repetitive pattern doesn't look good to me,
so I created JDK-8129119 [3] to consider releasing awtLock upon 
dispatchEvent call in XToolkit.


[0] https://bugs.openjdk.java.net/browse/JDK-8051617 Fullscreen mode is 
not working properly on Xorg
[1] https://bugs.openjdk.java.net/browse/JDK-7158311 
GraphicsDevice.setDisplayMode(...) leads to hang when DISPLAY variable 
points to Oracle Linux
[2] https://bugs.openjdk.java.net/browse/JDK-7155963 Deadlock in 
SystemFlavorMap.getFlavorsForNative and SunToolkit.awtLock
[3] https://bugs.openjdk.java.net/browse/JDK-8129119 Consider reducing 
the time while the AWTLock is held


--
Thanks,

Alexander.

AWT Dev [9] Review request for 8081371: [PIT] Test closed/java/awt/FullScreen/DisplayMode/CycleDMImage.java switches Linux to the single device mode

[Alexander Zvegintsev]

Hello,

please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8081371/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8081371

As known we are currently return a wrong list of display modes in multi 
screen environment[0].
Apparently the recent fullscreen mode fix[1] enables setDisplayMode() in 
such case. As result user gets an unwanted result.
This fix disables display mode change support for multi screen cases but 
keeps exclusive fullscreen support enabled.


[0] https://bugs.openjdk.java.net/browse/JDK-8022810 Cannot list all the 
available display modes on Ubuntu linux in case of two screen devices
[1] https://bugs.openjdk.java.net/browse/JDK-8051617 Fullscreen mode is 
not working properly on Xorg


--
Thanks,

Alexander.

Re: AWT Dev [9] Review Request: 7178683 [macosx] The default directory for open dialog is different for FileDialogOpenDirTest.html

[Alexander Zvegintsev]

Hello Sergey,

the fix looks good to me.

--
Thanks,
Alexander.

On 09.06.2015 22:34, Sergey Bylokhov wrote:

Hello.
Please review the small fix for jdk9.
The test was created for the JDK-4974135 and checks specific to 
xawt/mawt behavior. It is not applicable to the osx as well as windows.

The test was moved to the open, but the diff is provided [1].

[1] http://cr.openjdk.java.net/~serb/7178683/diff/
Bug: https://bugs.openjdk.java.net/browse/JDK-7178683
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7178683/webrev.00

Re: AWT Dev [OpenJDK 2D-Dev] RFR: 8081315: 8077982 giflib upgrade breaks system giflib builds with earlier versions

[Alexander Zvegintsev]

Sure, looks good to me.

Thanks,

Alexander.

On 06/05/2015 06:34 PM, Andrew Hughes wrote:

- Original Message -

Hi Andrew,

We have removed a workaround[0] for interlaced images support, there is
no need in it after 5.0.0 [1]:

DGifSlurp() and EGifSpew() now read and write interlaced images properly.

I didn't test it, but it seems to me that interlaced images will look
bad after building against GIFLIB  5.
So I suggest to revert SplashDecodeGif() to its state before GIFLIB
upgrade and make interlaced if conditional[3]:

diff -r 58535e641739
src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
--- a/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
+++ b/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
@@ -213,16 +213,14 @@ SplashDecodeGif(Splash * splash, GifFile
   byte_t *pSrc = image-RasterBits;
   ImageFormat srcFormat;
   ImageRect srcRect, dstRect;
-int pass, npass;
+int pass = 4, npass = 5;

+#if GIFLIB_MAJOR  5
   if (desc-Interlace) {
   pass = 0;
   npass = 4;
   }
-else {
-pass = 4;
-npass = 5;
-}
+#endif

   srcFormat.colorMap = colorMapBuf;
   srcFormat.depthBytes = 1;

And again, I haven't tested it against GIFLIB 4.*.

[0]
http://cr.openjdk.java.net/~azvegint/jdk/9/8077982/00/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c.sdiff.html
[1] http://sourceforge.net/p/giflib/code/ci/master/tree/NEWS
[3] http://cr.openjdk.java.net/~azvegint/jdk/9/8081315/03/

Thanks,

Alexander.


Thanks for the heads up on this. It's not very obvious from the code.

I went back through the original 8077982 changes to that file and
produced the resulting webrev:

http://cr.openjdk.java.net/~andrew/8081315/webrev.03/

Comparing with the original (giflib  5) splashscreen_gif.c now gives:

@@ -213,16 +213,16 @@
  byte_t *pSrc = image-RasterBits;
  ImageFormat srcFormat;
  ImageRect srcRect, dstRect;
-int pass, npass;
+int pass = 4, npass = 5;
  
+#if GIFLIB_MAJOR  5

+   /* Interlaced gif support is broken in giflib  5
+  so we need to work around this */
  if (desc-Interlace) {
  pass = 0;
  npass = 4;
  }
-else {
-pass = 4;
-npass = 5;
-}
+#endif
  
  srcFormat.colorMap = colorMapBuf;

  srcFormat.depthBytes = 1;

much as you have above.

Ok to push this version?

Thanks,

Re: AWT Dev [OpenJDK 2D-Dev] RFR: 8081315: 8077982 giflib upgrade breaks system giflib builds with earlier versions

[Alexander Zvegintsev]

Hi Andrew,

We have removed a workaround[0] for interlaced images support, there is 
no need in it after 5.0.0 [1]:

DGifSlurp() and EGifSpew() now read and write interlaced images properly.
I didn't test it, but it seems to me that interlaced images will look 
bad after building against GIFLIB  5.
So I suggest to revert SplashDecodeGif() to its state before GIFLIB 
upgrade and make interlaced if conditional[3]:


diff -r 58535e641739 
src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c

--- a/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
+++ b/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c
@@ -213,16 +213,14 @@ SplashDecodeGif(Splash * splash, GifFile
 byte_t *pSrc = image-RasterBits;
 ImageFormat srcFormat;
 ImageRect srcRect, dstRect;
-int pass, npass;
+int pass = 4, npass = 5;

+#if GIFLIB_MAJOR  5
 if (desc-Interlace) {
 pass = 0;
 npass = 4;
 }
-else {
-pass = 4;
-npass = 5;
-}
+#endif

 srcFormat.colorMap = colorMapBuf;
 srcFormat.depthBytes = 1;

And again, I haven't tested it against GIFLIB 4.*.

[0] 
http://cr.openjdk.java.net/~azvegint/jdk/9/8077982/00/src/java.desktop/share/native/libsplashscreen/splashscreen_gif.c.sdiff.html

[1] http://sourceforge.net/p/giflib/code/ci/master/tree/NEWS
[3] http://cr.openjdk.java.net/~azvegint/jdk/9/8081315/03/

Thanks,

Alexander.

On 06/03/2015 03:34 AM, Andrew Hughes wrote:

- Original Message -

I think you meant the subject to be RFR: 8081315 ...


Ah yes, I moved the 8077982 in front of the colon when it's actually part
of the bug description i.e. RFR: 8081315: 8077982 giflib upgrade :(


if (DGifCloseFile(gif, NULL) == GIF_ERROR)
return 0;

I suppose we probably ought to be checking the (new) error returns
although I am not sure what impact a failure on this particular
call would have if we already had read the data.

I'm not sure about this myself; it's much like the IOException that
can be thrown from closing a Java stream. As far as I can see from [0],
it would throw an error if it fails in reading the GIF terminating block
or deallocating used memory.


But could you write this as

if (DGifCloseFile(gif, NULL) == GIF_ERROR){
  return 0;
}


Done: http://cr.openjdk.java.net/~andrew/8081315/webrev.02/


?


-phil.




[0] http://giflib.sourceforge.net/gif_lib.html