<AWT Dev> [8] Review request for CR 8006406: lightweight embedding in other Java UI toolkits

[Anton V. Tarasov]

Hi Anthony,

(sorry, I missed your reply)

On 2/11/13 5:46 PM, Anthony Petrov wrote:
> Hi Anton,
>
> If I read your explanation correctly, the current calls chain is:
>
>> popups -> SunToolkit.grab -> (JLightweightFrame)Window.grabFocus -> 
>> WindowPeer.grabFocus
>
> I.e. we want to end up in WindowPeer.grabFocus() anyway. Note that for 
> non-lightweight frames we want to call WindowPeer.grabFocus() as well 
> (even though currently it's named e.g. WWindowPeer.grab() instead of 
> grabFocus(), etc.)
>
> I suggest to reduce this call chain to just:
>
> popups -> SunToolkit.grab -> WindowPeer.grabFocus
>
> which would work for both lightweight and heavyweight frames.
>
> In other words, I suggest to rename the current 
> *WindowPeer.grab()/ungrab() methods in all toolkits to "@Override 
> grab/ungrabFocus()", and implement the SunToolkit.grab/ungrab() as 
> final methods just delegating to the peer's methods. For lightweight 
> frames, the *LightweightFramePeer classes must delegate the requests 
> to FX.

The main difference here is that you suggest to delegate to FX from 
peers, while I tend to put delegation to the target (JLightweightFrame).
 From my point of view, delegating from peers has the following 
disadvantages:

- grab/ungrab delegation to FX is platform independent, so peers don't 
seem the right place to implement it

- anyway, a *LightweightFramePeer only connects to its target, it knows 
nothing about the LightweightContent delegate, so a peer would need to 
delegate via a JLightweightFrame,
    for which it would need to either call a suitable public method or 
use "shared secrets" approach to workaround access control. Passing a 
LightweightContent delegate to a peer
    is also not good from my point of view.

>
> Note that we can't remove the SunToolkit.grab/ungrab() interface 
> anyway since JFXPanel code relies on it already.

I think we can rewrite it in JFXPanel (it's implementation details) 
right after we have a new AWT grab API.

> Therefore, I think that we don't want to introduce the Window. (or 
> currently just LightweightFrame.)grab/ungrabFocus() API at this time. 
> I believe that a proper peer interface should be enough for now.

I supposed we could, at least, rewrite our own code to use 
Window.grabFocus and declare SunToolkit.grab deprecated. At last, this 
is an _internal_ public API which has a disclaimer.

Thanks,
Anton.
>
>
> -- 
> best regards,
> Anthony
>
> On 02/11/13 12:39, Anton V. Tarasov wrote:
>> Hi Anthony,
>>
>> JLightweightFrame overrides focusGrab/focusUngrab and forwards the calls
>> to the client app. Ideally, when the grab API is public, it should be
>> called directly via Window.focusGrab/focusUngrab. Currently it is called
>> via SunToolkit.grab/ungrab (by swing popups). So, normal call chain
>> should look like:
>>
>> popups -> (JLightweightFrame)Window.grabFocus -> WindowPeer.grabFocus
>>
>> Temporarily it's as follows:
>>
>> popups -> SunToolkit.grab -> (JLightweightFrame)Window.grabFocus ->
>> WindowPeer.grabFocus
>>
>> If I called it directly it would be:
>>
>> popups -> SunToolkit.grab -> WindowPeer.grabFocus ->
>> (JLightweightFrame)Window.grabFocus -> WindowPeer.grabFocus -> ...
>>
>> So, this is quite the reverse direction.
>>
>> Thanks,
>> Anton.
>>
>> On 2/8/13 7:23 PM, Anthony Petrov wrote:
>>> Hi Anton,
>>>
>>> src/windows/classes/sun/awt/windows/WToolkit.java
>>>> 987 public void grab(Window w) {
>>>> 991 if (w instanceof LightweightFrame) {
>>>> 992 ((LightweightFrame)w).grabFocus();
>>>> 993 return;
>>>> 994 } 995 if (w.getPeer() != null) {
>>>> 996 ((WWindowPeer)w.getPeer()).grab();
>>>> 997 }
>>>> 998 }
>>>
>>> You've already added grab/ungrabFocus() to the WindowPeer interface.
>>> Why not use them here directly instead of these instanceof checks?
>>> Applies to other toolkits as well.
>>>
>>> The fix looks good otherwise.
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 2/8/2013 21:27, Anton V. Tarasov wrote:
>>>> Hi All,
>>>>
>>>> Please, review the changes for the CR:
>>>> http://bugs.sun.com/view_bug.do?bug_id=8006406
>>>>
>>>> webrev: http://cr.openjdk.java.net/~ant/8006406/webrev.6
>>>>
>>>> It introduces sun.swing.JLightweightFrame class, aimed at lightweight
>>>> embedding of Swing components into java-based toolkits.
>>>> The primary target is JavaFX toolkit, however the class is not
>>>> limited to this usage and the API it provides is quite generic.
>>>>
>>>> Below I'm giving a link to the jfx side implementation. This
>>>> implementation should not be reviewed in this thread (it is in a
>>>> pre-review phase),
>>>> it should just clarify how the introduced API is supposed to be used.
>>>> Namely, SwingNode.SwingNodeContent which implements
>>>> sun.swing.LightweightContent and forwards requests from
>>>> sun.swing.JLightweightFrame to NGExternalNode which does the 
>>>> rendering.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~ant/RT-27887/webrev.1
>>>>
>>>> Some comments on the awt/swing part:
>>>>
>>>> - Only Win and Mac implementation is currently available, X11 will
>>>> come lately.
>>>>
>>>> - Win implementation uses a heavyweight window behind the lightweight
>>>> frame, while it is not actually needed for lightweight embedding.
>>>> This is due to the architecture of the Win AWT peers which are
>>>> strongly tight to the native code, and it's not a trivial task to
>>>> separate them.
>>>> On Mac the lightweight frame peer is truly lightweight, meaning that
>>>> it doesn't create an NSWindow object behind it. The Mac port LW
>>>> abstraction
>>>> allows to override and substitute CPlatform* classes with their
>>>> lightweight stubs.
>>>>
>>>> - LightweightFrame, among others, introduces two new methods -
>>>> grabFocus() and ungrabFocus(boolean). Ideally, these methods should
>>>> go to
>>>> the super Window class where the grab API becomes public (which is a
>>>> long-term project...). Current host of the grab API is SunToolkit, 
>>>> which
>>>> now forwards the calls to LightweightFrame. This is necessary to
>>>> intercommunicate with the client when grab/ungrab happens on both 
>>>> sides.
>>>>
>>>> - Unresolved issues exist, like modal dialogs, d&d etc. They are to
>>>> be addressed further.
>>>>
>>>> Thanks,
>>>> Anton.
>>>>
>>