RFR: 8334509: Cancelling PageDialog does not return the same PageFormat object [v2]

[Prasanta Sadhukhan]

On Wed, 19 Jun 2024 10:39:35 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Test fix
>
> 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..

> 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..

> 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...

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1646883814
PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1646884122
PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1646883283