| From | Sent On | Attachments |
|---|---|---|
| Roman Kennke | Nov 14, 2011 1:34 pm | |
| Oleg Sukhodolsky | Nov 14, 2011 10:16 pm | |
| Roman Kennke | Nov 14, 2011 11:51 pm | |
| Anton Tarasov | Nov 15, 2011 12:10 am | |
| Oleg Sukhodolsky | Nov 15, 2011 12:12 am | |
| Roman Kennke | Nov 15, 2011 1:43 am | .patch |
| Oleg Sukhodolsky | Nov 15, 2011 1:54 am | |
| Anton V. Tarasov | Nov 15, 2011 3:10 am | |
| Roman Kennke | Nov 15, 2011 3:22 am | |
| Oleg Sukhodolsky | Nov 15, 2011 3:32 am | |
| Anton V. Tarasov | Nov 15, 2011 4:00 am | |
| Oleg Sukhodolsky | Nov 16, 2011 3:51 am | |
| Oleg Sukhodolsky | Nov 18, 2011 8:47 pm |
| Subject: | Re: <AWT Dev> Proposal for consolidation of KeyboardFocusManagerPeer | |
|---|---|---|
| From: | Roman Kennke (rom...@kennke.org) | |
| Date: | Nov 15, 2011 1:43:29 am | |
| List: | net.java.openjdk.awt-dev | |
| Attachments: | ||
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. ?
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
diff -r 28f768c41a90 src/share/classes/java/awt/Component.java --- a/src/share/classes/java/awt/Component.java Sat Nov 12 04:13:38 2011 +0400 +++ b/src/share/classes/java/awt/Component.java Tue Nov 15 10:38:32 2011 +0100 @@ -7649,8 +7649,8 @@
// Focus this Component long time = EventQueue.getMostRecentEventTime(); - boolean success = peer.requestFocus - (this, temporary, focusedWindowChangeAllowed, time, cause); + boolean success = peerRequestFocus + (heavyweight, temporary, focusedWindowChangeAllowed, time, cause); if (!success) { KeyboardFocusManager.getCurrentKeyboardFocusManager (appContext).dequeueKeyEvents(time, this); @@ -7665,6 +7665,32 @@ return success; }
+ private boolean peerRequestFocus(Component hw, boolean temporary, boolean
focusedWindowChangeAllowed, long time, Cause cause) {
+
+ if (KeyboardFocusManager.
+ processSynchronousLightweightTransfer(hw, this, temporary,
+ focusedWindowChangeAllowed,
time))
+ {
+ return true;
+ }
+
+ int result = KeyboardFocusManager.
+ shouldNativelyFocusHeavyweight(hw, this,
+ temporary,
focusedWindowChangeAllowed,
+ time, cause);
+
+ switch (result) {
+ case KeyboardFocusManager.SNFH_FAILURE:
+ return false;
+ case KeyboardFocusManager.SNFH_SUCCESS_PROCEED:
+ return hw.peer.requestFocus(this, temporary,
focusedWindowChangeAllowed, time, cause);
+ case KeyboardFocusManager.SNFH_SUCCESS_HANDLED:
+ // Either lightweight or excessive request - all events are
generated.
+ return true;
+ }
+ return false;
+ }
+
private boolean isRequestFocusAccepted(boolean temporary,
boolean focusedWindowChangeAllowed,
CausedFocusEvent.Cause cause)
diff -r 28f768c41a90 src/solaris/classes/sun/awt/X11/XComponentPeer.java
--- a/src/solaris/classes/sun/awt/X11/XComponentPeer.java Sat Nov 12 04:13:38
2011 +0400
+++ b/src/solaris/classes/sun/awt/X11/XComponentPeer.java Tue Nov 15 10:38:32
2011 +0100
@@ -290,66 +290,48 @@
boolean focusedWindowChangeAllowed, long
time,
CausedFocusEvent.Cause cause)
{
- if (XKeyboardFocusManagerPeer.
- processSynchronousLightweightTransfer(target, lightweightChild,
temporary,
- focusedWindowChangeAllowed,
time))
- {
- return true;
+ // Currently we just generate focus events like we deal with
lightweight instead of calling
+ // XSetInputFocus on native window
+ if (focusLog.isLoggable(PlatformLogger.FINER)) {
+ focusLog.finer("Proceeding with request to "
+ + lightweightChild + " in " + target);
}
+ /**
+ * The problems with requests in non-focused window arise because
shouldNativelyFocusHeavyweight
+ * checks that native window is focused while appropriate
WINDOW_GAINED_FOCUS has not yet
+ * been processed - it is in EventQueue. Thus, SNFH allows native
request and stores request record
+ * in requests list - and it breaks our requests sequence as first
record on WGF should be the last
+ * focus owner which had focus before WLF. So, we should not add
request record for such requests
+ * but store this component in mostRecent - and return true as before
for compatibility.
+ */
+ Window parentWindow = SunToolkit.getContainingWindow(target);
+ if (parentWindow == null) {
+ return rejectFocusRequestHelper("WARNING: Parent window is null");
+ }
+ XWindowPeer wpeer = (XWindowPeer) parentWindow.getPeer();
+ if (wpeer == null) {
+ return rejectFocusRequestHelper("WARNING: Parent window's peer is
null");
+ }
+ /*
+ * Passing null 'actualFocusedWindow' as we don't want to restore focus
on it
+ * when a component inside a Frame is requesting focus.
+ * See 6314575 for details.
+ */
+ boolean res = wpeer.requestWindowFocus(null);
- int result = XKeyboardFocusManagerPeer.
- shouldNativelyFocusHeavyweight(target, lightweightChild,
- temporary,
focusedWindowChangeAllowed,
- time, cause);
-
- switch (result) {
- case XKeyboardFocusManagerPeer.SNFH_FAILURE:
- return false;
- case XKeyboardFocusManagerPeer.SNFH_SUCCESS_PROCEED:
- // Currently we just generate focus events like we deal with
lightweight instead of calling
- // XSetInputFocus on native window
- if (focusLog.isLoggable(PlatformLogger.FINER))
focusLog.finer("Proceeding with request to " +
- lightweightChild + " in " + target);
- /**
- * The problems with requests in non-focused window arise because
shouldNativelyFocusHeavyweight
- * checks that native window is focused while appropriate
WINDOW_GAINED_FOCUS has not yet
- * been processed - it is in EventQueue. Thus, SNFH allows native
request and stores request record
- * in requests list - and it breaks our requests sequence as
first record on WGF should be the last
- * focus owner which had focus before WLF. So, we should not add
request record for such requests
- * but store this component in mostRecent - and return true as
before for compatibility.
- */
- Window parentWindow = SunToolkit.getContainingWindow(target);
- if (parentWindow == null) {
- return rejectFocusRequestHelper("WARNING: Parent window is
null");
- }
- XWindowPeer wpeer = (XWindowPeer)parentWindow.getPeer();
- if (wpeer == null) {
- return rejectFocusRequestHelper("WARNING: Parent window's
peer is null");
- }
- /*
- * Passing null 'actualFocusedWindow' as we don't want to restore
focus on it
- * when a component inside a Frame is requesting focus.
- * See 6314575 for details.
- */
- boolean res = wpeer.requestWindowFocus(null);
-
- if (focusLog.isLoggable(PlatformLogger.FINER))
focusLog.finer("Requested window focus: " + res);
- // If parent window can be made focused and has been made
focused(synchronously)
- // then we can proceed with children, otherwise we retreat.
- if (!(res && parentWindow.isFocused())) {
- return rejectFocusRequestHelper("Waiting for asynchronous
processing of the request");
- }
- return XKeyboardFocusManagerPeer.deliverFocus(lightweightChild,
- (Component)target,
- temporary,
-
focusedWindowChangeAllowed,
- time, cause);
- // Motif compatibility code
- case XKeyboardFocusManagerPeer.SNFH_SUCCESS_HANDLED:
- // Either lightweight or excessive request - all events are
generated.
- return true;
+ if (focusLog.isLoggable(PlatformLogger.FINER)) {
+ focusLog.finer("Requested window focus: " + res);
}
- return false;
+ // If parent window can be made focused and has been made
focused(synchronously)
+ // then we can proceed with children, otherwise we retreat.
+ if (!(res && parentWindow.isFocused())) {
+ return rejectFocusRequestHelper("Waiting for asynchronous
processing of the request");
+ }
+ return XKeyboardFocusManagerPeer.deliverFocus(lightweightChild,
+ (Component) target,
+ temporary,
+ focusedWindowChangeAllowed,
+ time, cause);
}
private boolean rejectFocusRequestHelper(String logMsg) {
diff -r 28f768c41a90 src/windows/classes/sun/awt/windows/WComponentPeer.java
--- a/src/windows/classes/sun/awt/windows/WComponentPeer.java Sat Nov 12
04:13:38 2011 +0400
+++ b/src/windows/classes/sun/awt/windows/WComponentPeer.java Tue Nov 15
10:38:32 2011 +0100
@@ -660,52 +660,33 @@
boolean focusedWindowChangeAllowed, long time,
CausedFocusEvent.Cause cause)
{
- if (WKeyboardFocusManagerPeer.
- processSynchronousLightweightTransfer((Component)target,
lightweightChild, temporary,
- focusedWindowChangeAllowed,
time))
- {
- return true;
+ if (focusLog.isLoggable(PlatformLogger.FINER)) {
+ focusLog.finer("Proceeding with request to " + lightweightChild + "
in " + target);
}
+ Window parentWindow = SunToolkit.getContainingWindow((Component)
target);
+ if (parentWindow == null) {
+ return rejectFocusRequestHelper("WARNING: Parent window is null");
+ }
+ WWindowPeer wpeer = (WWindowPeer) parentWindow.getPeer();
+ if (wpeer == null) {
+ return rejectFocusRequestHelper("WARNING: Parent window's peer is
null");
+ }
+ boolean res = wpeer.requestWindowFocus(cause);
- int result = WKeyboardFocusManagerPeer
- .shouldNativelyFocusHeavyweight((Component)target,
lightweightChild,
- temporary,
focusedWindowChangeAllowed,
- time, cause);
+ if (focusLog.isLoggable(PlatformLogger.FINER)) {
+ focusLog.finer("Requested window focus: " + res);
+ }
+ // If parent window can be made focused and has been made
focused(synchronously)
+ // then we can proceed with children, otherwise we retreat.
+ if (!(res && parentWindow.isFocused())) {
+ return rejectFocusRequestHelper("Waiting for asynchronous
processing of the request");
+ }
+ return WKeyboardFocusManagerPeer.deliverFocus(lightweightChild,
+ (Component) target,
+ temporary,
+ focusedWindowChangeAllowed,
+ time, cause);
- switch (result) {
- case WKeyboardFocusManagerPeer.SNFH_FAILURE:
- return false;
- case WKeyboardFocusManagerPeer.SNFH_SUCCESS_PROCEED:
- if (focusLog.isLoggable(PlatformLogger.FINER)) {
- focusLog.finer("Proceeding with request to " +
lightweightChild + " in " + target);
- }
- Window parentWindow =
SunToolkit.getContainingWindow((Component)target);
- if (parentWindow == null) {
- return rejectFocusRequestHelper("WARNING: Parent window is
null");
- }
- WWindowPeer wpeer = (WWindowPeer)parentWindow.getPeer();
- if (wpeer == null) {
- return rejectFocusRequestHelper("WARNING: Parent window's
peer is null");
- }
- boolean res = wpeer.requestWindowFocus(cause);
-
- if (focusLog.isLoggable(PlatformLogger.FINER))
focusLog.finer("Requested window focus: " + res);
- // If parent window can be made focused and has been made
focused(synchronously)
- // then we can proceed with children, otherwise we retreat.
- if (!(res && parentWindow.isFocused())) {
- return rejectFocusRequestHelper("Waiting for asynchronous
processing of the request");
- }
- return WKeyboardFocusManagerPeer.deliverFocus(lightweightChild,
- (Component)target,
- temporary,
-
focusedWindowChangeAllowed,
- time, cause);
-
- case WKeyboardFocusManagerPeer.SNFH_SUCCESS_HANDLED:
- // Either lightweight or excessive request - all events are
generated.
- return true;
- }
- return false;
}
private boolean rejectFocusRequestHelper(String logMsg) {






.patch