On 19.08.2020 09:16, Anton Litvinov wrote:
Thank you very much for review of this fix. I have been evaluating your
suggestion and my opinion is following. On macOS that code was implemented
already from the moment JDK 7 code for macOS appeared. Although macOS-specific
platform-dependent code on Java level has some similarities with Windows
platform-dependent code on Java level, native platform-dependent code is very
different between these two OSes.
Of course it is technically possible to move some part of "AwtFrame::WmSize" function
(http://hg.openjdk.java.net/jdk/client/file/2c7b5ae39c19/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#l915)
to some new method or methods in "sun.awt.windows.WWindowPeer", but that fix will be much bigger,
will require different changes both in C++ and Java code, and will be necessary to prove that such
refactoring did not break anything previously working well. "AwtFrame::WmSize" function depends on:
- "AwtFrame" class fields: "m_iconic", "m_zoomed" through the functions: "isIconic()",
"isZoomed()", "setIconic(BOOL)", "setZoomed(BOOL)".
- Win32 API constants: "SIZE_MINIMIZED", "SIZE_MAXIMIZED".
- Call to the function "AwtWindow::UpdateSecurityWarningVisibility" is not
movable to Java level.
No I did not meant to move the whole "AwtFrame::WmSize" to the java, but only
the part related for notification, same as on mac.
So instead of calling " AwtFrame::setExtendedStateMID," from the native, just call
something like "WWindowPeer.notifyState(int old, int new, zoom, iconify,... etc)", and on
the java side call all necessary things like on mac:
http://hg.openjdk.java.net/jdk/jdk/file/6b984aa424e3/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#l661
- postEvent iconify/zoom;
- postWindowStateChangedEvent(newWindowState);
etc
Probably this part of the code in awt_Frame.cpp can be moved to
"WWindowPeer.notifyState(...)?
DTRACE_PRINTLN2("AwtFrame::WmSize: reporting state change %x -> %x",
oldState, newState);
// sync target with peer
JNIEnv *env = (JNIEnv *)JNU_GetEnv(jvm, JNI_VERSION_1_2);
env->CallVoidMethod(GetPeer(env), AwtFrame::setExtendedStateMID,
newState);
// report (de)iconification to old clients
if (changed & java_awt_Frame_ICONIFIED) {
if (newState & java_awt_Frame_ICONIFIED) {
SendWindowEvent(java_awt_event_WindowEvent_WINDOW_ICONIFIED);
} else {
SendWindowEvent(java_awt_event_WindowEvent_WINDOW_DEICONIFIED);
}
}
// New (since 1.4) state change event
SendWindowStateEvent(oldState, newState);
It looks that only contents of "if (changed != 0) {" block
(http://hg.openjdk.java.net/jdk/client/file/2c7b5ae39c19/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#l975)
can be moved to some new Java method. But what to do with JNI invocation of
"getExtendedState()" method
(http://hg.openjdk.java.net/jdk/client/file/2c7b5ae39c19/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#l281).
To my mind, such refactoring will not give much benefit, new Java method or
methods will not be used from anywhere else, but it will be required to change
code which currently works stably and maybe the changes will bring some new
problems. I would prefer to make the fix as narrow as possible for this not
very usual scenario leading to the crash.
ALTERNATIVE SUGGESTION FROM ME:
By doing this minimal version of the fix, I tried to avoid even JNI usage without the
need. If you do not like this custom flag, then would you agree for its change to a
normal check of peer object class using "instanceof" operator?
For example, to use instead of this flag the next if condition in
"awt_Frame.cpp":
"jobject peer = GetPeer();
if ((peer != NULL) && (JNU_IsInstanceOfByName(env, peer, "sun/awt/windows/WFramePeer")
> 0)) {"
Or if you do not like variant with "instanceof" operator, then instead of introduction any checks anywhere, I
can suggest you as another alternative option to implement 2 new empty private methods: "int
getExtendedState()", "void setExtendedState(int)" in the class "sun.awt.windows.WWindowPeer".
Do we really need to change the code related to the "getExtendedState" during
creation of AWTFrame?
That one should never be used by the AwtDialog, so the crash is kind of
assertion that something go wrong already.
--
Best regards, Sergey.
