This is somewhat a continuation for 
[JDK-8307160](https://bugs.openjdk.org/browse/JDK-8307160) and 
[JDK-8334509](https://bugs.openjdk.org/browse/JDK-8334509).

The former removed the `doIt` flag in #18584, but it introduced a regression.  
The regression is resolved by the latter in #19786, and it added the 'doIt' 
flag again.

I think using a flag makes the code less clear. When reading the code, one has 
to keep track of the current value of the `doIt` flag.

I raised my concern in [comments in PR 
19786](https://github.com/openjdk/jdk/pull/19786#discussion_r1649191308):

> I'd rather keep `JNI_FALSE` here — it's more explicit and therefore clearer. 
> You don't have to keep track of the value of `doIt` flag while reading the 
> code.
> 
> In fact, I'd prefer no `doIt` flag at all… yet it makes handling the code 
> below `if (ret)` slightly harder.

I came up with a solution which simplifies handling the clean-up. The solution 
relies on C++ destructors which are called when the declared objects go out of 
scope.

The C++ object wraps a lambda function to clean up and invokes this function in 
its destructor. Such C++ class has to be a templated class because there's no 
defined type to represent a lambda. The class has to be declared at the top of 
the file because it needs C++ linkage.

There are two `Cleaner` objects declared in the code of 
`Java_sun_awt_windows_WPageDialogPeer__1show`:

**`refCleaner`** is declared right after all the references to Java objects are 
initialised. The corresponding `refCleanup` lambda deletes all the references 
and replaces the `CLEANUP_SHOW` macro. Before JDK-8307160, the code jumped to a 
go-to label to perform the clean-up.

**`postCleaner`** is declared after `AwtDialog::CheckInstallModalHook` is 
called. Its lambda `postCleanup` uninstalls the modal hook and handles focus 
transfer as well as saves the updated printer parameters.

It is `postCleaner` that resolves the bug JDK-8334868 by ensuring 
`CheckUninstallModalHook` and `ModalActivateNextWindow` are called if 
`::PageSetupDlg` is called.

As the result of using `refCleaner`, all the `return` statements in the 
function use an explicit value, which makes the code cleaner. There's no need 
to use a `go to` statement or insert a macro to delete references to Java 
objects, about which one had to remember — the destructor of the `refCleaner` 
handles deleting references when `refCleaner` goes out of scope, that is when 
the function returns.

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

Commit messages:
 - 8334868: Ensure CheckUninstallModalHook is called in WPageDialogPeer._show

Changes: https://git.openjdk.org/jdk/pull/19867/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19867&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8334868
  Stats: 92 lines in 1 file changed: 48 ins; 36 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/19867.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19867/head:pull/19867

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

Reply via email to