On Thu, 20 Jun 2024 03:39:47 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 694: >> >>> 692: ::GlobalUnlock(setup.hDevMode); >>> 693: } >>> 694: doIt = JNI_TRUE; >> >> Another option would be to return `JNI_TRUE` here and to return `JNI_FALSE` >> in the end of the function. > > Yes, it could have been done that way and my intiial fix in JBS is that only > but I wanted to reinstate the flag as it is before > [JDK-8307160](https://bugs.openjdk.org/browse/JDK-8307160) which was working > till now.. The only problem with the flag is that the data flow is not as clear. I'm not insisting, it worked before and it will work in the future. >> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 697: >> >>> 695: } >>> 696: >>> 697: AwtDialog::CheckUninstallModalHook(); >> >> I wonder why the block of code at lines 697–709 is not needed if `JNI_FALSE` >> is returned as a result of an error condition. >> >> No, it is not directly connected to the bug you're fixing, yet it doesn't >> look right to me. > > We can have a followup bug on this I guess since it was existing before > [JDK-8307160](https://bugs.openjdk.org/browse/JDK-8307160) also.. All I wanted is to bring up the inconsistency so that a few people would take a look at it while reviewing this change. >> test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 60: >> >>> 58: t1.start(); >>> 59: PageFormat newFormat = pj.pageDialog(oldFormat); >>> 60: if (!newFormat.equals(oldFormat)) { >> >> You should interrupt the `t1` thread after `pj.pageDialog` returns to stop >> robot from sending `keyPress` and `keyRelease` after the dialog is hidden. >> You can even use `Thread.sleep` instead of `Robot.delay` so that >> interrupting the thread throws `InterruptedException` and its handler just >> exits. (`Robot.delay` catches `InterruptedException` and then restores the >> interrupted flag.) >> >> I wonder if any AWT event is sent when the Page Dialog is shown on the >> screen, an event is more reliable and key presses won't be sent to an >> arbitrary window in the system. > > I am not sure I understand this..I guess this thread execution will be a > one-time activity so I guess we dont need to do any thread cleanup specially > when the dialog is modal so it will only be cancelled (ie pageDialog will > return) by VK_ESCAPE in the thread You've resolved the problem. In previous iteration, the keys were sent in a loop, which meant that the thread could continue to run even after the page dialog was closed. Now the `t1` thread presses `VK_ESCAPE` once and exits. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1647370601 PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1647372620 PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1647377851
