On Thu, 3 Jun 2021 23:34:38 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

> This PR adds the necessary `@SuppressWarnings("removal")` annotations for the 
> recently-integrated security manager deprecation, [JEP 
> 411](https://openjdk.java.net/jeps/411). See openjdk/jdk#4073.
> 
> There are four commits:
> 
> 1. 678b026 : A patch generated by @wangweij to add annotations to the runtime 
> (`modules/*/src/main/java`) using the same automated tooling that he used as 
> part of the implementation of JEP 411.
> 2. 9698e87 : Same as above for the tests.
> 3. 1c42cf8 : Manually removes a pair of unused imports, one of which (a 
> static import) was causing a removal warning.
> 4. 4f87d18 : Manually reduced the scope of the annotation where it was added 
> to an entire class, or to a large method where only a small part of the 
> method uses a deprecated method. This was done using similar techniques to 
> the following fixes that Weijun did in openjdk/jdk#4172.
>  
> The first two commits represent the bulk of the changes. Other than scanning 
> to ensure that there are no obvious errors, and testing, they probably don't 
> need the same level of scrutiny as the manual changes do.
> 
> I tested this on all three platforms by doing a build / test with `JDK_HOME` 
> set to a local JDK 17 ea build that includes the fix for JEP 411. I ran the 
> build with `gradle -PLINT=removal` and verified that there were removal 
> warnings for the security manager APIs without this fix and none with this 
> fix.
> 
> NOTE: The following files under `modules/javafx.web/src/android` and 
> `modules/javafx.web/src/ios` were not processed by the automated tool. As I 
> have no way to compile them, I chose not to manually fix them either, but 
> doing so would be trivial as a follow-up fix if desired.
> 
> 
> modules/javafx.web/src/android/java/com/sun/webkit/Timer.java
> modules/javafx.web/src/android/java/com/sun/webkit/WebPage.java
> modules/javafx.web/src/android/java/javafx/scene/web/WebEngine.java
> modules/javafx.web/src/ios/java/javafx/scene/web/ExportedJavaObject.java
> modules/javafx.web/src/ios/java/javafx/scene/web/HTMLEditorSkin.java
> modules/javafx.web/src/ios/java/javafx/scene/web/JS2JavaBridge.java
> modules/javafx.web/src/ios/java/javafx/scene/web/WebEngine.java

All change looks good. Some personal style preferences comments.

modules/javafx.fxml/src/main/java/com/sun/javafx/fxml/ModuleHelper.java line 41:

> 39: 
> 40:     static {
> 41:         verbose = 
> AccessController.doPrivileged((PrivilegedAction<Boolean>) () ->

You can merge this assignment with the declaration on line 38. Or you can keep 
this so the check of verbose is in the same block with its assignment.

modules/javafx.graphics/src/main/java/com/sun/glass/ui/Screen.java line 43:

> 41: 
> 42:     static {
> 43:         @SuppressWarnings("removal")

Combine assignment and declaration?

modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/LinuxArch.java 
line 36:

> 34: 
> 35:     static {
> 36:         bits = AccessController.doPrivileged((PrivilegedAction<Integer>) 
> () -> {

Combine?

modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 9969:

> 9967:             accessible = 
> Application.GetApplication().createAccessible();
> 9968:             accessible.setEventHandler(new Accessible.EventHandler() {
> 9969:                 @SuppressWarnings("removal")

Was this "deprecation" already useless before this change?

modules/javafx.web/src/main/java/com/sun/webkit/network/HTTP2Loader.java line 
415:

> 413:         // Run the HttpClient in the page's access control context
> 414:         @SuppressWarnings("removal")
> 415:         CompletableFuture<Void> tmpResponse = 
> AccessController.doPrivileged((PrivilegedAction<CompletableFuture<Void>>) () 
> -> {

Is "var" enough?

modules/javafx.web/src/main/java/com/sun/webkit/network/NetworkContext.java 
line 245:

> 243:         private static final Permission modifyThreadPerm = new 
> RuntimePermission("modifyThread");
> 244: 
> 245:         @SuppressWarnings("removal")

Maybe move this onto the 1st line inside the method?

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

Marked as reviewed by weijun (no project role).

PR: https://git.openjdk.java.net/jfx/pull/528

Reply via email to