<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Desktop.java looks fine now. Moving on
      to the other API classes <br>
      <br>
      Top-level question :<br>
      Why are AppEvent and TaskBar in java.awt and not java.awt.desktop
      ?<br>
      And AppEvent seems to use a lot of nested classes.<br>
      If they were in java.awt.desktop then they could be top-level
      classes<br>
      without creating too much clutter. I don't think it matters that
      'this<br>
      is how the com.apple APIs were done' - people need to recode
      anyway<br>
      <br>
      <br>
      AppEvent.java :<br>
      more wildcards ..<br>
      <pre>  28 import java.awt.desktop.*;</pre>
      <br>
      I notice the docs in this file (and perhaps more I have yet to
      see)<br>
      reference types without @code :-<br>
      <pre>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.
</pre>
      <br>
      <pre>  69         public List<File> getFiles() {
  70             return files;
  71         }

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

</pre>
      <pre>  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</pre>
      <br>
      Could we say "optionally" instead of "additionally"<br>
      and make clear that it may be "null"<br>
      <br>
      a very long line :-<br>
      <br>
      <pre>209      * Event sent when the application has become the foreground app, and when it has resigned being the foreground app.</pre>
      <br>
      And "resigned" is a voluntary term. If something else is placed
      into the foreground the<br>
      this app may not get asked if it wants to allow that. <br>
      Maybe instead say "and when it is no longer the foreground app."<br>
      <br>
      Also is there anything we can (or should) say about how some of
      these events interact ?<br>
      <br>
      eg, if a foreground app is hidden does only the appHidden listener
      get called ?<br>
      <br>
      BTW the term 'unhidden' makes sense if the app was previously
      'hidden' but less so<br>
      if the app is being newly shown.<br>
      <br>
      <br>
      <pre> /**
 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.

</pre>
      <br>
      Also unless there is a reason I have not yet come across, as many
      as possible<br>
      of these classes should be marked 'final'<br>
      <br>
      <br>
      Taskbar.java :<br>
      <br>
      One meta-concern I have here is that it seems to have a very
      blurry distinction<br>
      with existing API :<br>
      <a class="moz-txt-link-freetext" href="https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html">https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html</a><br>
      <a class="moz-txt-link-freetext" href="https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html">https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html</a><br>
      <br>
      Aren't both of these APIs dealing with the same area ?<br>
      <br>
      Should we really add this new class or tweak system tray as needed
      ?<br>
      <br>
      <pre>/**
 112      * Stops displaying the progress.
 113      */
 114     @Native
 115     public static final int STATE_OFF = 0x0;

Another case where an enum could be used.

</pre>
      <br>
      <pre>  sun.awt.AppContext context = sun.awt.AppContext.getAppContext();

these days this sometimes can return null .. 
</pre>
      <br>
      <pre>AppForegroundEvent :
* @param e the app became foreground notification.

I am not entirely sure what that means or even what it
should say, perhaps because  AppForegroundEvent does not
have any fields.
How about
"@param e an event indicating that the receiving application is now the foreground app" 

  30 /**
  31  * Implementors are notified when the app is hidden or shown by the user.
  32  * This notification is helpful for discontinuing a costly animation if it's not visible to the user.
  33  */
  34 public interface AppHiddenListener extends SystemEventListener {
</pre>
        .. we already can get notification when a window is hidden.<br>
      Can I assume that this event indicates *all* windows are hidden
      and<br>
      each hidden window still gets its hidden event ? And that there is<br>
      no guarantee of order of delivery ?<br>
      <br>
       <br>
      <pre>/**
  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.
</pre>
      <br>
      -phil.<br>
      <br>
      <br>
      On 02/16/2016 07:04 AM, Alexander Zvegintsev wrote:<br>
    </div>
    <blockquote cite="mid:56C33A87.4040609@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      Hi Phil,<br>
      <br>
      thanks for the review, please see updated webrev here:<br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Eazvegint/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>
    </blockquote>
    <br>
  </body>
</html>