8252446: Screen.getScreens() is empty sometimes

[kevinrushforth]

As noted in the bug report, we get a pair of change events every time the list of screens changes. First, a change is sent with an empty list of screens and then a change is sent with the new list of screens. This happens whenever a monitor is plugged in or unplugged. It also happens on Mac at application startup.

As noted in the bug the reason for this is because the updateConfiguration method makes two separate calls on the list of screens, clear and addAll, rather than calling setAll. The latter ensures that only a single change event is delivered.

I verified that before this fix, the example program attached to the bug works correctly after the fix.

I wrote a unit test. It ends up being skipped on Windows and Linux since we don't get an initial change event. On Mac the test fails without the fix and passes with the fix.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8252446: Screen.getScreens() is empty sometimes

Reviewers

• Ambarish Rapte (@arapte - Reviewer)
• Pankaj Bansal (@pankaj-bansal - no project role)

Download

$ git fetch https://git.openjdk.java.net/jfx pull/295/head:pull/295
$ git checkout pull/295

[kevinrushforth]

/reviewers 2

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

[openjdk]

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

[mlbridge]

Webrevs

• 00: Full (8f4c749)

[arapte]

Looks good to me. As you mentioned in the comments, On my Mac the test does not fail without this change.

[pankaj-bansal]

I tried this on Mac and Ubuntu 20.04. I could not reproduce the issue without the fix and test passes with/without the fix. But the changes make sense, so approving the changes.

[kevinrushforth]

Reviewers:
...
Pankaj Bansal (@pankaj-bansal - no project role)

This is a case where the Skara jcheck bot is more restrictive than hg jcheck was. What it means is that even though Pankaj is a "R"eviewer in another project, and has several commits in the jfx project, the Skara tooling doesn't consider him as an Author in the openjfx project, so doesn't count that review towards the required 2. Given that Pankaj has several jfx commits, I will reflect the intent of that by lowering the requirement to 1 reviewer + 1 contributor to satisfy the tooling.

[kevinrushforth]

/reviewers 2 contributors

[openjdk]

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

[openjdk]

@kevinrushforth This change now passes all automated pre-integration checks. In addition to the automated checks, the change must also fulfill all project specific requirements

After integration, the commit message will be:

8252446: Screen.getScreens() is empty sometimes

Reviewed-by: arapte, pbansal

• If you would like to add a summary, use the /summary command.
• To credit additional contributors, use the /contributor command.
• To add additional solved issues, use the /issue command.

Since the source branch of this PR was last updated there have been 8 commits pushed to the master branch:

• b2e2000: 8252546: Move ObservableValue's equality check and lazy evaluation descriptions to @implSpec
• 976a763: 8240499: Enforce whitespace checking for additional source files
• d6dee34: 8252547: Correct transformations docs in Node
• 0514116: 8246745: ListCell/Skin: misbehavior on switching skin
• 7506554: Merge
• 4b75750: 8252784: Create release notes for JavaFX 15 (8252784: Create release notes for JavaFX 15 #296)
• 3cb3ca8: 8202990: javafx webview css filter property with display scaling
• a5ecfb6: 8252387: Deprecate for removal css Selector and ShapeConverter constructors

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge master into your branch, and then specify the current head hash when integrating, like this: /integrate b2e2000c676f78db6849a54552ae8713286ec68d.

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

[kevinrushforth]

/integrate

[openjdk]

@kevinrushforth Since your change was applied there have been 8 commits pushed to the master branch:

• b2e2000: 8252546: Move ObservableValue's equality check and lazy evaluation descriptions to @implSpec
• 976a763: 8240499: Enforce whitespace checking for additional source files
• d6dee34: 8252547: Correct transformations docs in Node
• 0514116: 8246745: ListCell/Skin: misbehavior on switching skin
• 7506554: Merge
• 4b75750: 8252784: Create release notes for JavaFX 15 (8252784: Create release notes for JavaFX 15 #296)
• 3cb3ca8: 8202990: javafx webview css filter property with display scaling
• a5ecfb6: 8252387: Deprecate for removal css Selector and ShapeConverter constructors

Your commit was automatically rebased without conflicts.

Pushed as commit 13ab2cb.

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