atom feed13 messages in net.java.openjdk.awt-devRe: <AWT Dev> Proposal for consolidat...
FromSent OnAttachments
Roman KennkeNov 14, 2011 1:34 pm 
Oleg SukhodolskyNov 14, 2011 10:16 pm 
Roman KennkeNov 14, 2011 11:51 pm 
Anton TarasovNov 15, 2011 12:10 am 
Oleg SukhodolskyNov 15, 2011 12:12 am 
Roman KennkeNov 15, 2011 1:43 am.patch
Oleg SukhodolskyNov 15, 2011 1:54 am 
Anton V. TarasovNov 15, 2011 3:10 am 
Roman KennkeNov 15, 2011 3:22 am 
Oleg SukhodolskyNov 15, 2011 3:32 am 
Anton V. TarasovNov 15, 2011 4:00 am 
Oleg SukhodolskyNov 16, 2011 3:51 am 
Oleg SukhodolskyNov 18, 2011 8:47 pm 
Subject:Re: <AWT Dev> Proposal for consolidation of KeyboardFocusManagerPeer
From:Oleg Sukhodolsky (son.@gmail.com)
Date:Nov 15, 2011 1:54:40 am
List:net.java.openjdk.awt-dev

On Tue, Nov 15, 2011 at 1:44 PM, Roman Kennke <rom@kennke.org> wrote:

Find attached a patch that illustrates the idea (couldn't find a recent version of webrev tool online, at least not in http://openjdk.java.net/guide/codeReview.html/webrevHelp.html).

It basically moves identical/equivalent code from the peers to java.awt package. Notice that this is not yet complete, with this change, the methods shouldNativelyFocusHeavyweight() and processSynchronousLightweightTransfer() can be removed from the KeyboardFocusManagerAccessor and KeyboardFocusManagerPeerImpl classes.

Maybe I am missing something important, i.e. why this code has to be in the peers and needs to jump through hoops to satisfy the KFM. ?

I think this is due to the fact that [WX]ComponentPeer are not the only implementation of ComponentPeer interface, we also have NullComponentPeer (or something like that) for lightweight components. and the code you are moving should be called for heavyweight components only.

Oleg.

Regards, Roman

Am Montag, den 14.11.2011, 22:35 +0100 schrieb Roman Kennke:

Hi there,

One thing that's bugging me for a while is how the ComponentPeer's requestFocus() method is supposed to work. As far as I could figure out, it's basically always like this (I use KFMHelper to call the corresponding KeyboardFocusManager's private methods by reflection):

    public boolean requestFocus(Component lightweightChild, boolean temporary,             boolean focusedWindowChangeAllowed, long time, Cause cause) {         if (KFMHelper.processSynchronousLightweightTransfer(window,                 lightweightChild, temporary, focusedWindowChangeAllowed, time)) {             return true;         }

        int result = KFMHelper.shouldNativelyFocusHeavyweight(window,                 lightweightChild, temporary, focusedWindowChangeAllowed, time,                 cause);

        switch (result) {         case KFMHelper.SNFH_FAILURE:             return false;         case KFMHelper.SNFH_SUCCESS_PROCEED:                     requestFocusImpl(window, lightweightChild, temporary,                                  focusedWindowChangeAllowed, time, cause);         case KFMHelper.SNFH_SUCCESS_HANDLED:             // Either lightweight or excessive request - all events are             // generated.             return true;         default:             return false;         }     }

The only thing that really differs between implementations would be the requestFocusImpl() method call in the SNFH_SUCCESS_PROCEED case. The rest seems to be the same in all implementations, except that in one case (Windows I believe) it is done in JNI while in others (X11) it's done by reflection.

I think this can be consolidated by doing the above directly in the KeyboardFocusManager, before calling the peer requestFocus(), and have the peer's requestFocus() only do the requestFocusImpl() handling. This way we could avoid duplicate code and avoid reflection/JNI altogether.

Maybe I am missing something?

If not, I would work on a patch to move the above KeyboardFocusManager calls into the KFM and have the peer only bothers with the part that is requestFocusImpl() in the above example. Does that sound reasonable? It would certainly make some things simpler in OpenJDK as well as Cacio and the JavaFX SwingView that I am working on.

Best regards, Roman