<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hi Phil,<br>
    <br>
    thanks for the review, please see updated webrev here:<br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/07/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/07/</a><br>
    <pre class="moz-signature" cols="72">--
Thanks,
Alexander.</pre>
    <div class="moz-cite-prefix">On 02/15/2016 09:58 PM, Phil Race
      wrote:<br>
    </div>
    <blockquote cite="mid:56C21FC1.3010804@oracle.com" type="cite">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      <div class="moz-cite-prefix">
        <pre><span class="new">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.</span>
</pre>
        <pre><span class="changed">> 43  * The {@code Desktop} class allows interact with various desktop capabilities.</span>
</pre>
        interact -> interaction<br>
        <br>
        Or better <br>
        <pre><span class="changed">The {@code Desktop} class allows interaction with various platform desktop capabilities.</span></pre>
        <br>
        <pre><span class="changed">> * @since 1.9

These should all be @since 9

< </span><span class="changed">130      * Returns the {@code Desktop} instance of the current
</span><span class="changed">> 268      * Returns the <code>Desktop</code> instance of the current</span>

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.

> <span class="new"> 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.
</span></pre>
        <br>
        <pre><span class="new">686      * @param aboutHandler the handler to respond to the</span>
<span class="new">687      * @throws UnsupportedOperationException if the current platform

seems like line 686 is incomplete. Same for line 704 :

</span><span class="new">704      * @param preferencesHandler the handler to respond to the</span>

<span class="new">801 </span>
<span class="new"> 802     /**</span>
<span class="new"> 803      * Sets the default strategy used to quit this application. The default is</span>
<span class="new"> 804      * calling SYSTEM_EXIT_0.</span>
<span class="new"> 805      *</span>
<span class="new"> 806      * @param strategy the way this application should be shutdown</span>
<span class="new"> 807      * @throws UnsupportedOperationException if the current platform</span>
<span class="new"> 808      * does not support the {@link Desktop.Action#APP_QUIT_STRATEGY} action</span>
<span class="new"> 809      * @see QuitStrategy</span>
<span class="new"> 810      * @since 1.9</span>
<span class="new"> 811      */</span>
<span class="new"> 812     public void setQuitStrategy(final QuitStrategy strategy) {</span>
<span class="new"> 813         checkActionSupport(Action.APP_QUIT_STRATEGY);</span>
<span class="new"> 814         peer.setQuitStrategy(strategy);</span>


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.

</pre>
        On 01/21/2016 11:16 PM, Alexander Zvegintsev wrote:<br>
      </div>
      <blockquote cite="mid:56A1D755.4060303@oracle.com" type="cite">
        <meta content="text/html; charset=utf-8"
          http-equiv="Content-Type">
        Hi Semyon,<br>
        <br>
        I am not sure about flexibility, but providing additional info
        for a user session change  could be useful<br>
        <br>
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Eazvegint/jdk/9/8143227/05/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/05/</a><br>
        (changed awt_Toolkit.cpp,  AppEvent.java,
        UserSessionListener.java, _AppEventHandler.java)<br>
        <pre class="moz-signature" cols="72">--
Thanks,
Alexander.</pre>
        <div class="moz-cite-prefix">On 01/19/2016 11:52 AM, Semyon
          Sadetsky wrote:<br>
        </div>
        <blockquote cite="mid:569DF949.5090102@oracle.com" type="cite">
          <meta content="text/html; charset=utf-8"
            http-equiv="Content-Type">
          Hi Alexander,<br>
          <br>
          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. <br>
          Also this may add flexibility that allows to support new
          features in the future.<br>
          <br>
          --Semyon<br>
          <br>
          <div class="moz-cite-prefix">On 1/17/2016 11:55 PM, Alexander
            Zvegintsev wrote:<br>
          </div>
          <blockquote cite="mid:569BFFB3.4020807@oracle.com" type="cite">
            <meta content="text/html; charset=utf-8"
              http-equiv="Content-Type">
            Hi Semyon,<br>
            <blockquote type="cite">Is it possible to use
              WM_QUERYENDSESSION for Action.APP_SUDDEN_TERMINATION? </blockquote>
            Sure, so please see updated webrev:<br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Eazvegint/jdk/9/8143227/04/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/04/</a><br>
            <br>
            awt_Toolkit.cpp:<br>
            extended list of supported messages for user session
            listener.<br>
            <br>
            awt_Desktop.cpp, awt_Toolkit.cpp, WDesktopPeer.java:<br>
            added SuddenTerminaton support for Windows<br>
            <pre class="moz-signature" cols="72">--
Thanks,
Alexander.</pre>
            <div class="moz-cite-prefix">On 01/15/2016 04:14 PM, Semyon
              Sadetsky wrote:<br>
            </div>
            <blockquote cite="mid:5698F0C2.9080907@oracle.com"
              type="cite">
              <meta content="text/html; charset=utf-8"
                http-equiv="Content-Type">
              Hi,<br>
              <br>
              <div class="moz-cite-prefix">On 1/15/2016 12:39 PM,
                Alexander Zvegintsev wrote:<br>
              </div>
              <blockquote cite="mid:5698BE51.2000305@oracle.com"
                type="cite">
                <meta http-equiv="Context-Type" content="text/html; ">
                Hi Semyon,<br>
                <br>
                Yes, it is just LOCK/UNLOCK for now,  because it is the
                most common scenario of desktop usage.<br>
                <br>
                Do you mean that we should consider remote desktop
                logins too? Something like:<br>
                <br>
                 <tt>         if (wParam == WTS_CONSOLE_CONNECT </tt><tt><br>
                </tt><tt>                  || wParam ==
                  WTS_CONSOLE_DISCONNECT </tt><tt><br>
                </tt><tt>                  || wParam ==
                  WTS_REMOTE_CONNECT </tt><tt><br>
                </tt><tt>                  || wParam ==
                  WTS_REMOTE_DISCONNECT </tt><tt><br>
                </tt><tt>                  || wParam ==
                  WTS_SESSION_UNLOCK </tt><tt><br>
                </tt><tt>                  || wParam ==
                  WTS_SESSION_LOCK) {</tt><tt><br>
                </tt><tt><br>
                </tt><tt>              BOOL activate = wParam ==
                  WTS_CONSOLE_CONNECT </tt><tt><br>
                </tt><tt>                                || wParam ==
                  WTS_REMOTE_CONNECT</tt><tt><br>
                </tt><tt>                                || wParam ==
                  WTS_SESSION_UNLOCK;</tt><tt><br>
                </tt><tt>             
                  env->CallStaticVoidMethod(clzz,
                  AwtToolkit::userSessionMID,</tt><tt><br>
                </tt><tt>                                               
                  activate</tt><tt><br>
                </tt><tt>                                               
                  ? JNI_TRUE </tt><tt><br>
                </tt><tt>                                               
                  : JNI_FALSE);</tt><tt><br>
                </tt><tt>          }<br>
                  <br>
                </tt></blockquote>
              That's seems better to me.<br>
              <blockquote cite="mid:5698BE51.2000305@oracle.com"
                type="cite"><tt> </tt>Or if you refer to
                WTS_SESSION_LOGOFF, then it seems useless for us, <br>
                AFAIK only services will receive this notification, and
                anyway all apps the user was running are already killed
                by this time. </blockquote>
              Is it possible to use WM_QUERYENDSESSION for
              Action.APP_SUDDEN_TERMINATION?
              <meta http-equiv="content-type" content="text/html;
                charset=utf-8">
              <meta http-equiv="content-type" content="text/html;
                charset=utf-8">
              <br>
              <br>
              --Semyon<br>
              <blockquote cite="mid:5698BE51.2000305@oracle.com"
                type="cite">
                <pre class="moz-signature" cols="72">--
Thanks,
Alexander.</pre>
                <div class="moz-cite-prefix">On 12.01.2016 19:21, Semyon
                  Sadetsky wrote:<br>
                </div>
                <blockquote cite="mid:569527F3.7000408@oracle.com"
                  type="cite"> Hi Alexander,<br>
                  <br>
                  awt_Toolkit.cpp:<br>
                  <br>
                        case WM_WTSSESSION_CHANGE: {<br>
                            jclass clzz =
                  env->FindClass("sun/awt/windows/WDesktopPeer");<br>
                            DASSERT(clzz != NULL);<br>
                            if (!clzz) throw std::bad_alloc();<br>
                  <br>
                            if (wParam == WTS_SESSION_LOCK || wParam ==
                  WTS_SESSION_UNLOCK) {<br>
                                env->CallStaticVoidMethod(clzz,
                  AwtToolkit::userSessionMID,<br>
                                                              wParam ==
                  WTS_SESSION_UNLOCK<br>
                                                                  ?
                  JNI_TRUE <br>
                                                                  :
                  JNI_FALSE);<br>
                            }<br>
                            break;<br>
                        }<br>
                  <br>
                  So only the WTS_SESSION_UNLOCK is propagated to java
                  as a session act/deact and other messages just
                  ignored? <br>
                    <br>
                  Other messages:<br>
                  #define WTS_CONSOLE_CONNECT 0x1<br>
                  #define WTS_CONSOLE_DISCONNECT 0x2<br>
                  #define WTS_REMOTE_CONNECT 0x3<br>
                  #define WTS_REMOTE_DISCONNECT 0x4<br>
                  #define WTS_SESSION_LOGON 0x5<br>
                  #define WTS_SESSION_LOGOFF 0x6<br>
                  #define WTS_SESSION_LOCK 0x7<br>
                  #define WTS_SESSION_REMOTE_CONTROL<br>
                  <br>
                  some of them seems to me more suitable to be chosen as
                  the session act/deact event.  Could you comment that
                  for me?<br>
                  <br>
                  --Semyon<br>
                  <br>
                  <div class="moz-cite-prefix">On 11/24/2015 6:02 PM,
                    Alexander Zvegintsev wrote:<br>
                  </div>
                  <blockquote cite="mid:56547C07.7090502@oracle.com"
                    type="cite"> Please review the updated fix:<br>
                    <a moz-do-not-send="true"
                      class="moz-txt-link-freetext"
                      href="http://cr.openjdk.java.net/%7Eazvegint/jdk/9/8143227/03/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/</a><br>
                    <br>
                    removed fullscreen related code (moved to
                    JDK-8143914 [0])<br>
                    fix serialization in AppEvent <br>
                    <br>
                    [0] <a moz-do-not-send="true"
                      class="moz-txt-link-freetext"
                      href="https://bugs.openjdk.java.net/browse/JDK-8143914">https://bugs.openjdk.java.net/browse/JDK-8143914</a><br>
                    <br>
                    <pre class="moz-signature" cols="72">Thanks,

Alexander.</pre>
                    <div class="moz-cite-prefix">On 11/21/2015 03:33 AM,
                      Alexander Zvegintsev wrote:<br>
                    </div>
                    <blockquote cite="mid:564FBBD3.1040208@oracle.com"
                      type="cite">Hi Phil, <br>
                      <br>
                      <blockquote type="cite">Can someone explain why
                        this is needed given the existing support of <br>
                        GraphicsDevice.setFullScreenWindow(Window) ? </blockquote>
                      <br>
                      GraphicsDevice.setFullScreenWindow is used for an
                      exclusive full screen mode. <br>
                      Mac OS has another option to create a virtual
                      desktop with provided window in it. <br>
                      You can switch between them by three-finger
                      horizontal swipe. <br>
                      <br>
                      <blockquote type="cite">Why does it have to be a
                        RootPaneContainer ? Why is this tied to Swing ?
                        <br>
                        This appears to narrow it to JDialog and
                        JWindow. </blockquote>
                      It reuses swing code to set native windows style
                      bits. <br>
                      <br>
                      <br>
                      Please see updated webrev: <br>
                      <a moz-do-not-send="true"
                        class="moz-txt-link-freetext"
                        href="http://cr.openjdk.java.net/%7Eazvegint/jdk/9/8143227/02/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/</a>
                      <br>
                      updated permission <br>
                      added missing @throws @since and @implNote <br>
                      browseFileDirectory is now return void <br>
                      RootPaneContainer -> JDialog and JWindow <br>
                      <br>
                      -- <br>
                      Thanks, <br>
                      Alexander. <br>
                      <br>
                      On 11/20/2015 09:03 PM, Phil Race wrote: <br>
                      <blockquote type="cite">On 11/20/2015 09:12 AM,
                        Sergey Bylokhov wrote: <br>
                        <blockquote type="cite"> <br>
                          I am worried about setWindowCanFullScreen and
                          requestToggleFullScreen. <br>
                          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? <br>
                        </blockquote>
                        <br>
                        Can someone explain why this is needed given the
                        existing support of <br>
                        GraphicsDevice.setFullScreenWindow(Window) ? <br>
                        <br>
                        And what happens if you use *both* ? They still
                        need to play well together <br>
                        if there is some reason the new one is needed. <br>
                        <br>
                        Other comments : <br>
                        >   * Note, Aqua Look and Feel should be
                        active to support this on Mac OS. <br>
                        <br>
                        <br>
                        Needs @implNote <br>
                        <br>
                        There seems to be lots of missing
                        SecurityException tags given all the
                        checkAWTPermission() calls. <br>
                        is checkAWTPermission() really the right call
                        for all of these actions ? <br>
                        Does it "cover" being able to delete files and
                        quit the app ? I am not sure it is <br>
                        correct in all cases. <br>
                        <br>
                        And also there are missing @since tags. <br>
                        <br>
                        <br>
                         Opens a folder containing the {@code file} in a
                        default system file manager. <br>
                         933      * @param file the file <br>
                         934      * @return returns true if successfully
                        opened <br>
                         935      * @throws NullPointerException if
                        {@code file} is {@code null} <br>
                         936      * @throws IllegalArgumentException if
                        the specified file doesn't <br>
                         937      * exist <br>
                         938      */ <br>
                         939     public boolean browseFileDirectory(File
                        file) { <br>
                        <br>
                        So what happens if there is no "support" for
                        this ? Exception or "false" ? <br>
                        Are you comfortable that all these APIs that
                        return "true" if successful are <br>
                        implementable on all platforms. i.e I mean that
                        does the platform return <br>
                        a value you can pass on as success/failure. <br>
                        <br>
                        --- <br>
                        <br>
                        861      * Attaches a {@link FullScreenListener}
                        to the specified top-level <br>
                         862      * {@link Window}. <br>
                         863      * <br>
                         864      * @param window to attach the {@link
                        FullScreenListener} to <br>
                         865      * @param listener to be notified when
                        a full screen event occurs <br>
                         866      * @throws IllegalArgumentException if
                        window is not a <br>
                         867      * {@link
                        javax.swing.RootPaneContainer} <br>
                         868      */ <br>
                         869     public void
                        addWindowFullScreenListener(final Window window,
                        <br>
                         870                                              

                        final FullScreenListener listener) { <br>
                        <br>
                        ------- <br>
                        <br>
                        Why does it have to be a RootPaneContainer ? Why
                        is this tied to Swing ? <br>
                        This appears to narrow it to JDialog and
                        JWindow. <br>
                        <br>
                        -phil. <br>
                        <br>
                      </blockquote>
                      <br>
                    </blockquote>
                    <br>
                  </blockquote>
                  <br>
                </blockquote>
                <br>
              </blockquote>
              <br>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>