<AWT Dev> Review request #1: 6402325 (Swing toolbars vs native toolbars on Windows)

[Anthony Petrov]

Hello AWT team,

The next version of the fix for

6402325 (Swing toolbars vs native toolbars on Windows)
http://bugs.sun.com/view_bug.do?bug_id=6402325

Please review at:

http://cr.openjdk.java.net/~anthony/7-33-utilityWindows-6402325.1/

This version introduces three window types:

NORMAL - for regular windows.

UTILITY - for toolboxes (either a WS_EX_TOOLWINDOW on Windows, or
_NET_WM_WINDOW_TYPE_UTILITY on X11)

POPUP - for menus and tooltips (WS_POPUP on Windows, or
_NET_WM_WINDOW_TYPE_POPUP_MENU on X11)

I ran related Swing and AWT automatic regression tests. They all passed
on MS Windows and Gnome.

--
best regards,
Anthony

[Artem Ananiev-2]

Hi, Anthony,

here are some comments from my side:

1. Window.Type seems not to be a perfect name for the enum, it's too
generic. What about Window.WindowType?

2. Could you move the declaration of the enum to the top of Window.java,
so it can be easily located, please?

3. I don't see a strong reason to use the proposed approach, when we
store the window type at java.awt level and then use AWTAccessor to get
its value at the peers level. Why don't you want to add a method to
WindowPeer interface which is called when Window.setType() is invoked?
It would eliminate AWTAccessor and getType_NoClientCode().

Thanks,

Artem

Anthony Petrov wrote:

> Hello AWT team,
>
> The next version of the fix for
>
> 6402325 (Swing toolbars vs native toolbars on Windows)
> http://bugs.sun.com/view_bug.do?bug_id=6402325
>
> Please review at:
>
> http://cr.openjdk.java.net/~anthony/7-33-utilityWindows-6402325.1/
>
> This version introduces three window types:
>
> NORMAL - for regular windows.
>
> UTILITY - for toolboxes (either a WS_EX_TOOLWINDOW on Windows, or
> _NET_WM_WINDOW_TYPE_UTILITY on X11)
>
> POPUP - for menus and tooltips (WS_POPUP on Windows, or
> _NET_WM_WINDOW_TYPE_POPUP_MENU on X11)
>
> I ran related Swing and AWT automatic regression tests. They all passed
> on MS Windows and Gnome.
>
> --
> best regards,
> Anthony
>
>

[Anthony Petrov]

Thanks for your review, Artem! Here's the next version of the fix. Some
comments follow.

On 10/27/2009 02:55 PM, Artem Ananiev wrote:
> 1. Window.Type seems not to be a perfect name for the enum, it's too
> generic. What about Window.WindowType?

That's exactly what I want to avoid: having it read as Window.WindowType
- too many "window" words, imo. Besides, take a look at the
Thread.State, for instance. The concept of a "state" is quite common
also, however, since this is an inner enum, that does not bring any
problems.


> 2. Could you move the declaration of the enum to the top of Window.java,
> so it can be easily located, please?

Sure.


> 3. I don't see a strong reason to use the proposed approach, when we
> store the window type at java.awt level and then use AWTAccessor to get
> its value at the peers level. Why don't you want to add a method to
> WindowPeer interface which is called when Window.setType() is invoked?
> It would eliminate AWTAccessor and getType_NoClientCode().

Indeed, I got rid of the AWTAccessor and reworked the X-side peers to
avoid its usage in that case. However, we can't just introduce a peer's
level method because:

a) Calling Window.setType() is prohibited after the peer goes live.
b) MS Window restricts tweaking some window styles after the window has
been created. And the WS_POPUP is exactly that kind of style.

Therefore, we need the window type information before we create the
hwnd. But the hwnd is created at the time of creation of the native part
(the AwtWindow class instance). Since we don't want to directly access
the Window.windowType field via JNI, we just don't have any other option
but to cache the value of the type in the WWindowPeer.


So, the updated version is available at:

http://cr.openjdk.java.net/~anthony/7-33-utilityWindows-6402325.2/

Also, last time I forgot to mention that the POPUP window type on X11
also means the override-redirect sort of window.

--
best regards,
Anthony