<AWT Dev> Review-request for 8143227: Platform-Specific Desktop Features

Semyon Sadetsky semyon.sadetsky at oracle.com
Tue Feb 2 13:22:13 UTC 2016


Hi Alexander,

Thank you for the update. It works for me.

One additional remark. You do not call ::CoUninitialize() in 
awt_Taskbar.cpp.
And I'm not sure that COM init/uninit can be done from arbitrary thread 
without aftereffects. MSDN states the next:

A thread must call CoUninitialize once for each successful call it has 
made to the CoInitialize or CoInitializeEx function, including any call 
that returns S_FALSE. Only the CoUninitialize call corresponding to the 
CoInitialize or CoInitializeEx call that initialized the library can 
close it.

--Semyon

On 1/22/2016 10:16 AM, 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/20160202/cc1d3c6b/attachment-0001.html>


More information about the awt-dev mailing list