RFR: 8334868: Ensure CheckUninstallModalHook is called in WPageDialogPeer._show

[Alexey Ivanov]

On Wed, 26 Jun 2024 23:42:16 GMT, Alisen Chung <achung at openjdk.org> wrote:

>> src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 640:
>> 
>>> 638:         HGLOBAL oldG = AwtPrintControl::getPrintHDMode(env, self);
>>> 639:         if (setup.hDevMode != oldG) {
>>> 640:            AwtPrintControl::setPrintHDMode(env, self, setup.hDevMode);
>> 
>> what does setPrintHDMode and setPrintHDName do? do they need to be called as part of cleanup even if there is an exception and we return JNI_FALSE?
>
> i think this code preserves the printer settings after the dialog closes (and only on success before this fix), so is it possible in the case the printer is removed and causes an exception, we shouldn't preserve these settings?

It's a good question…

Yes, the code saves (or updates) the printer name and mode.

These lines of code were called in cases where OK or Cancel is clicked in the print dialog. However, it was skipped in case of an error, just like `CheckUninstallModalHook`.

I also feel that these lines belong in the successful case, right before `return JNI_TRUE`, but I could quickly find any confirmation whether `setup.hDevMode` or `setup.hDevNames` can be modified. For this reason, I preserved the previous code flow.

I presume that neither `setup.hDevMode` or `setup.hDevNames` changes if the user clicks Cancel in `::PageSetupDlg`. It still leaves the question open what happens when the user clicks OK but we catch an error inside `if (ret)`. The old handles stored inside `self` could have become invalid, so it's safer to update the handles in `self` even if an error occurs.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19867#discussion_r1656892141