This is a re-worked version of [my previous 
attempt](https://git.openjdk.org/jdk/pull/19867) to fix the issue that was 
found during code review 
https://github.com/openjdk/jdk/pull/19786#discussion_r1645900131 for 
[JDK-8334509](https://bugs.openjdk.org/browse/JDK-8334509).

To ensure `AwtDialog::CheckInstallModalHook()` and 
`AwtDialog::ModalActivateNextWindow` are always called, I moved them from the 
bottom of the function. Now these functions are called right after displaying 
the page dialog, that is after `::PageSetupDlg` returns.

I reversed the condition of the `if` statement to `if (!ret)` and exit from the 
function right away if `::PageSetupDlg` returned an error.

The diff looks large because the body of the `if` is unindented.

With this change, I modify the data flow and remove the `doIt` flag. The data 
flow with the flag is not as clear, and I already raised [my 
concern](https://github.com/openjdk/jdk/pull/19786#discussion_r1647370601). I'm 
sure this is what caused 
[JDK-8334509](https://bugs.openjdk.org/browse/JDK-8334509).

There was only one place where `JNI_TRUE` is assigned to the `doIt` flag—in the 
end of the `if (ret)` block. Therefore, the last `return` statement in the 
`WPageDialogPeer__1show` function could have either `JNI_FALSE` or `JNI_TRUE` 
stored in the `doIt` flag. In all the other `return` statements, the value of 
`doIt` remains `JNI_FALSE`.

It was the mistake in https://github.com/openjdk/jdk/pull/18584 that assumed 
`JNI_TRUE` was the only possibility in the end of the function.

By returning early if `::PageSetupDlg` results in an error, I eliminate the 
above possibility—if the last `return` statement in `WPageDialogPeer__1show` is 
reached, it has to return `JNI_TRUE`.

All the other `return` statements explicitly return `JNI_FALSE`… because an 
error occurred.

To verify that this change does not introduce another regression,

* I use `PageDialogCancelTest.java` that was added in #19786 which covers the 
case where `::PageSetupDlg` returns an error; and
* I add a new test, `PageDialogFormatTest.java`, that covers the successful 
case.

Additionally, I ensure `::GlobalUnlock(setup.hDevMode)` is called on the error 
return from the block at [lines 
687–700](https://github.com/aivanov-jdk/jdk/blob/30f776b95a1eda9fcacf77ff74a8eb41591fd6e9/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L687-L700).

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

Commit messages:
 - Call ::GlobalUnlock(setup.hDevMode) in case of an error return
 - Remove if (ret) and un-indent the code
 - Merge master
 - 8334868: Ensure CheckUninstallModalHook is called in WPageDialogPeer._show

Changes: https://git.openjdk.org/jdk/pull/30207/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=30207&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8334868
  Stats: 154 lines in 3 files changed: 72 ins; 9 del; 73 mod
  Patch: https://git.openjdk.org/jdk/pull/30207.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/30207/head:pull/30207

PR: https://git.openjdk.org/jdk/pull/30207

Reply via email to