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 <code>Desktop</code>
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 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 ? 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.
