<AWT Dev> Review-request for 8143227: Platform-Specific Desktop Features
Phil Race
philip.race at oracle.com
Mon Feb 15 18:58:09 UTC 2016
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.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160215/e450448d/attachment-0001.html>
More information about the awt-dev
mailing list