8315657: Application window not activated in macOS 14 Sonoma

[kevinrushforth]

The fix for JDK-8233678 added a step to the NSApplication initialization to deactivate and then reactivate the application. We trigger the deactivation the first time applicationDidBecomeActive is called. We then trigger the reactivation the when applicationDidResignActive is called after that. The initialization logic in MacApplication spins up a nested event loop that waits for the reactivation (a second call to applicationDidBecomeActive).

Something in macOS 14 has changed that prevents the initial call to applicationDidBecomeActive from being done until the application has finished launching (notifyDidFinishLaunching).

So the activation of the NSApplication is waiting for the launching to be finished, but the nested event loop we spin up is blocking the completion of launching until we get the activation. This is a classic deadlock.

The good news, though, is the deactivation / reactivation of the application no longer seems to be needed on macOS 14. The menus work fine without that fix.

The proposed solution is to check the version of macOS and only enable the deactivation / reactivation logic on systems prior to macOS 14. This will have the additional benefit of avoiding the brief flash when the app is first launched (see JDK-8257835).

This is a Draft PR with a few debugging statements. Once I get some feedback that it works for more than just me, I'll remove the debugging statements and then make it "rfr".

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8315657: Application window not activated in macOS 14 Sonoma (Bug - P2)

Reviewers

• Jose Pereda (@jperedadnr - Reviewer)
• Ajit Ghaisas (@aghaisas - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1247/head:pull/1247
$ git checkout pull/1247

Update a local copy of the PR:
$ git checkout pull/1247
$ git pull https://git.openjdk.org/jfx.git pull/1247/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1247

View PR using the GUI difftool:
$ git pr show -t 1247

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1247.diff

Webrev

Link to Webrev Comment

[kevinrushforth]

@jperedadnr Do you have access to a macOS 14 beta system that you could use to test this fix?

[bridgekeeper]

👋 Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[jperedadnr]

Yes, I could do a build and ask someone with macOS 14 beta to test it.

[FlorianKirmaier]

Because I updated my Laptop to Sonoma (so I find bugs earlier) - I was able to test it!

With this branch,
I no longer get the following warning:

[error] Sep 22, 2023 3:12:18 PM com.sun.glass.ui.mac.MacApplication lambda$waitForReactivation$6
[error] WARNING: Timeout while waiting for app reactivation

And the mysterious (did anyone notice?) 5 Seconds freeze before showing the App is also gone!

So, I can confirm this fixes these important Sonoma bugs!

[kevinrushforth]

With this branch, I no longer get the following warning:

[error] Sep 22, 2023 3:12:18 PM com.sun.glass.ui.mac.MacApplication lambda$waitForReactivation$6
[error] WARNING: Timeout while waiting for app reactivation

That's good news.

And the mysterious (did anyone notice?) 5 Seconds freeze before showing the App is also gone!

The warning and the 5 second freeze are the same problem. MacApplication::waitForReactivation waits for up to 5 seconds for the window to be activated. After 5 seconds, it logs the warning and proceeds anyway.

So, I can confirm this fixes these important Sonoma bugs!

Thanks for confirming. I'll remove the debug prints and make this PR "rfr".

[kevinrushforth]

/reviewers 2

[openjdk]

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[mlbridge]

Webrevs

• 00: Full (ce2b25d8)

[jperedadnr]

I can confirm that the proposed fix works on macOS Sonoma beta. We have tested an application installed via jpackage. Now the application is launched properly into the foreground.

[jperedadnr]

Looks good, and testing on macOS 14 beta works with the proposed fix.
Minor: if you update the copyright of license headers I'll reapprove again.

[kevinrushforth]

Minor: if you update the copyright of license headers I'll reapprove again.

I usually prefer not to, especially for a fix like this that will be backported across multiple releases (it helps avoid trivial merge conflicts). Our copyright update scripts will pick it up.

@aghaisas or @arapte can one of you be a second reviewer?

[aghaisas]

I do not have access to a system with 10.14.
Code changes look reasonable. Approving based on this assumption and fix confirmation by others in this PR.

[openjdk]

@kevinrushforth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8315657: Application window not activated in macOS 14 Sonoma

Reviewed-by: jpereda, aghaisas

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

• 5d1254f: 8316781: Legal, Monarch paper sizes are incorrect in javafx.print.Paper
• 8eb9ac9: 8307176: Monkey Tester Application Part 2

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[kevinrushforth]

/integrate

[openjdk]

Going to push as commit f2dd4bd.
Since your change was applied there have been 2 commits pushed to the master branch:

• 5d1254f: 8316781: Legal, Monarch paper sizes are incorrect in javafx.print.Paper
• 8eb9ac9: 8307176: Monkey Tester Application Part 2

Your commit was automatically rebased without conflicts.

[openjdk]

@kevinrushforth Pushed as commit f2dd4bd.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.