-
Notifications
You must be signed in to change notification settings - Fork 471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8342459: Remove calls to doPrivileged in javafx.base #1625
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back arapte! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
Webrevs
|
I just integrated the fix for this, so if you merge in the latest master, you won't see this problem any more. Reviewers: @kevinrushforth @lukostyra /reviewers 2 |
@kevinrushforth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I did leave one inline comment: I think there is a now-unused private AccessControllerContext
field in the various *JavaBeans*
classes. If so, removing that unused field will also allow removing the remaining SuppressWarnings
annotation and the last of the security-related imports.
If you decide to change this I will reapprove. Otherwise, I'll pick it up in the last of the follow-up fixes.
import java.security.AccessControlContext; | ||
import java.security.AccessController; | ||
import java.security.PrivilegedAction; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the now-unused field private final AccessControlContext acc
, which will also let you remove the AccessControlContext
and AccessController
. Alternatively, I will remove them as part of JDK-8342993.
This comment applies to all of the JavaBeanXXXProperty
classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Kevin, The variable acc
and related 2 imports are now removed in commit.
Remove
doPrivileged
calls in the javafx.base module.The changes are straight forward.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1625/head:pull/1625
$ git checkout pull/1625
Update a local copy of the PR:
$ git checkout pull/1625
$ git pull https://git.openjdk.org/jfx.git pull/1625/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1625
View PR using the GUI difftool:
$ git pr show -t 1625
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1625.diff
Using Webrev
Link to Webrev Comment