RFR: 8264290: Create implementation for NSAccessibilityComponentGroup protocol peer

2021-05-17 Thread Alexander Zuev
Initial implementation.

-

Commit messages:
 - 8264290: Create implementation for NSAccessibilityComponentGroup protocol 
peer

Changes: https://git.openjdk.java.net/jdk/pull/4046/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4046&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264290
  Stats: 63 lines in 3 files changed: 60 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4046.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4046/head:pull/4046

PR: https://git.openjdk.java.net/jdk/pull/4046


Re: RFR: 8182043: Access to Windows Large Icons [v8]

2021-05-17 Thread Alexander Zvegintsev
On Mon, 17 May 2021 05:13:06 GMT, Alexander Zuev  wrote:

>> Fix updated after first round of review.
>
> Alexander Zuev has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Example in JavaDoc fixed

Marked as reviewed by azvegint (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2875


Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v4]

2021-05-17 Thread Tejpal Rebari
On Mon, 17 May 2021 05:57:18 GMT, Tejpal Rebari  wrote:

>> Hi All,
>> Please review the following fix for jdk17.
>> 
>> Issue : On MacOS 11 Java Frame and JDialog application is freezing  in Full 
>> Screen  when the System Preference -> General -> Prefer Tabs is set to "Full 
>> Screen". It is also freezing in normal screen when Prefer Tabs is set to 
>> "Always".
>> It doesn't freeze when the Prefer tabs is set to "never".
>> 
>> Fix : The default value of allowsAutomaticWindowTabbing is 0/NO in MacOS 
>> prior to bigSur(11)  
>> (in the AWTWindow.m file),  so the issue is not seen in mac os 10.13 10.14 
>> and 10.15.
>> From MacOS 11 onwards  this value is set to 1/YES and the issue is seen.
>> This issue can also be reproduced in MacOS 10.15 by setting 
>> setAllowsAutomaticWindowTabbing to true in the AWTWindow.m file.
>> 
>> Fix is to set allowsAutomaticWindowTabbing to No for all the MacOS release 
>> staring from 10.12.
>> The allowsAutomaticTabbing was introduced in MacOS 10.12 but the default 
>> value changed in macos11.
>> 
>> Test : Added a manual test and tested on MacOS 10.15 and 11.
>> All the internal tests run are green.
>
> Tejpal Rebari has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   initialize in static block added doprivilged and removed @available

Created CSR  : https://bugs.openjdk.java.net/browse/JDK-8267238.

-

PR: https://git.openjdk.java.net/jdk/pull/3407


RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current

2021-05-17 Thread Naoto Sato
Please review the changes to the subject issue. java.util.Locale class has a 
long-standing issue for those obsolete ISO 639 languages where its 
normalization ends up in the obsolete codes. This change intends to flip the 
normalization towards the current codes, providing a system property for 
compatibility behavior. ResourceBundle class is also modified to load either 
obsolete/current bundles. For more detail, take a look at the CSR.

-

Commit messages:
 - Added more ResourceBundleProvider tests
 - ResourceBundleProvider and test cases modifications
 - Eliminated some duplicated code
 - Changed ResourceBundle javadoc
 - Locale class description
 - renamed old resource files
 - inital commit

Changes: https://git.openjdk.java.net/jdk/pull/4069/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4069&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263202
  Stats: 382 lines in 35 files changed: 239 ins; 41 del; 102 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4069.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4069/head:pull/4069

PR: https://git.openjdk.java.net/jdk/pull/4069


Re: RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current

2021-05-17 Thread Erik Joelsson
On Mon, 17 May 2021 16:55:35 GMT, Naoto Sato  wrote:

> Please review the changes to the subject issue. java.util.Locale class has a 
> long-standing issue for those obsolete ISO 639 languages where its 
> normalization ends up in the obsolete codes. This change intends to flip the 
> normalization towards the current codes, providing a system property for 
> compatibility behavior. ResourceBundle class is also modified to load either 
> obsolete/current bundles. For more detail, take a look at the CSR.

I see no relevant build changes to comment on as the build label was only added 
because the buildtool java source was touched, so this looks ok from a build 
perspective.

-

PR: https://git.openjdk.java.net/jdk/pull/4069


Re: RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current

2021-05-17 Thread Naoto Sato
On Mon, 17 May 2021 17:14:53 GMT, Erik Joelsson  wrote:

> I see no relevant build changes to comment on as the build label was only 
> added because the buildtool java source was touched, so this looks ok from a 
> build perspective.

Removed `build` label.

-

PR: https://git.openjdk.java.net/jdk/pull/4069


Re: RFR: 8262731: [macOS] Exception from "Printable.print" is swallowed during "PrinterJob.print"

2021-05-17 Thread Anton Litvinov
On Fri, 14 May 2021 18:37:46 GMT, Anton Litvinov  wrote:

> Hello,
> 
> Could you please review the following fix for the bug specific to macOS. The 
> bug consists in the fact that if the method 
> "java.awt.print.Printable.print​(Graphics, PageFormat, int)" throws 
> "java.awt.print.PrinterException" or "java.lang.RuntimeException" during the 
> call "java.awt.print.PrinterJob.print()", then the exception is caught and 
> ignored by JDK and a user cannot learn that printing failed and what caused 
> failure of printing, because "PrinterJob.print()" method does not throw 
> "PrinterException" or the occurred exception is not reported by JDK through 
> the error stream.
> 
> ROOT CAUSE OF THE BUG:
> The root cause of the bug is the fact that in the method 
> "sun.lwawt.macosx.CPrinterJob.printAndGetPageFormatArea(final Printable, 
> final Graphics, final PageFormat, final int)" from the file 
> "src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java" the 
> exception thrown during execution of the expression
> 
> "int pageResult = printable.print(graphics, pageFormat, pageIndex);"
> 
> is caught but is not returned to a developer by any mean or is not printed 
> out to the error stream.
> 
> THE FIX:
> The fix implements propagation of the occurred and caught exception to the 
> level of the user's code executing "PrinterJob.print()" method. Propagation 
> of the exception by storing it in the instance variable of "CPrinterJob" 
> object is implemented, because the engaged code always is executed:
> - on 2 threads (non-EDT thread, EDT thread) in case when "PrinterJob.print()" 
> is called by the user on a non-EDT thread;
> - on 3 threads (2 EDT threads, a temporary thread started by JDK to execute 
> "CPrinterJob._safePrintLoop(long, long );") when "PrinterJob.print()" is 
> called on EDT thread.
> 
> The regression test which is part of the fix was also successfully executed 
> on MS Windows OS and Linux OS.
> 
> Thank you,
> Anton

Publishing this RFR on the e-mail alias (2d-...@openjdk.java.net) by writing 
this comment. Dear reviewers from (2d-...@openjdk.java.net) e-mail alias, could 
you please review this RFR.

-

PR: https://git.openjdk.java.net/jdk/pull/4036


RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-17 Thread Weijun Wang
Please review the test changes for [JEP 411](https://openjdk.java.net/jeps/411).

With JEP 411 and the default value of `-Djava.security.manager` becoming 
`disallow`, tests calling `System.setSecurityManager()`  need 
`-Djava.security.manager=allow` when launched. This PR covers such changes for 
tier1 to tier3 (except for the JCK tests).

To make it easier to focus your review on the tests in your area, this PR is 
divided into multiple commits for different areas. Mostly the rule is the same 
as how Skara adds labels, but there are several small tweaks:

1. When a file is covered by multiple labels, only one is chosen. I make my 
best to avoid putting too many tests into `core-libs`. If a file is covered by 
`core-libs` and another label, I categorized it into the other label.
2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
`xml` commit.
3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
`rmi` commit.
4. One file not covered by any label -- 
`test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in the 
`swing` commit.

Due to the size of this PR, no attempt is made to update copyright years for 
all files to minimize unnecessary merge conflict.

Please note that this PR can be integrated before the source changes for JEP 
411, as the possible values of this system property was already defined long 
time ago in JDK 9.

Most of the change in this PR is a simple adding of 
`-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it 
was not `othervm` and we add one. Sometimes there's no `@run` at all and we add 
the line.

There are several tests that launch another Java process that needs to call the 
`System.setSecurityManager()` method, and the system property is added to 
`ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a 
shell test).

3 langtools tests are added into problem list due to 
[JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).

2 SQL tests are moved because they need different options on the `@run` line 
but they are inside a directory that has a `TEST.properties`:

rename test/jdk/java/sql/{testng/test/sql/othervm => 
permissionTests}/DriverManagerPermissionsTests.java (93%)
rename test/jdk/javax/sql/{testng/test/rowset/spi => 
permissionTests}/SyncFactoryPermissionsTests.java (95%)
 ```

The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.

-

Commit messages:
 - test for awt
 - test for hotspot-gc
 - test for compiler
 - test for net
 - test for core-libs
 - test for beans
 - test for xml
 - test for nio
 - test for hotspot-runtime
 - test for security
 - ... and 7 more: https://git.openjdk.java.net/jdk/compare/79b39445...900c28c0

Changes: https://git.openjdk.java.net/jdk/pull/4071/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4071&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267184
  Stats: 1024 lines in 852 files changed: 249 ins; 8 del; 767 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4071.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4071/head:pull/4071

PR: https://git.openjdk.java.net/jdk/pull/4071


RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

2021-05-17 Thread Weijun Wang
Please review this implementation of [JEP 
411](https://openjdk.java.net/jeps/411).

The code change is divided into 3 commits. Please review them one by one.

1. 
https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 
The essential change for this JEP, including the `@Deprecate` annotations and 
spec change. It also update the default value of the `java.security.manager` 
system property to "disallow", and necessary test change following this update.
2. 
https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 
Manual changes to several files so that the next commit can be generated 
programatically.
3. 
https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 
Automatic changes to other source files to avoid javac warnings on deprecation 
for removal

The 1st and 2nd commits should be reviewed carefully. The 3rd one is generated 
programmatically, see the comment below for more details. If you are only 
interested in a portion of the 3rd commit and would like to review it as a 
separate file, please comment here and I'll generate an individual webrev.

Due to the size of this PR, no attempt is made to update copyright years for 
any file to minimize unnecessary merge conflict.

Furthermore, since the default value of `java.security.manager` system property 
is now "disallow", most of the tests calling `System.setSecurityManager()` need 
to launched with `-Djava.security.manager=allow`. This is covered in a 
different PR at https://github.com/openjdk/jdk/pull/4071.

-

Commit messages:
 - add supresswarnings annotations automatically
 - manual change before automatic annotating
 - 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

Changes: https://git.openjdk.java.net/jdk/pull/4073/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8266459
  Stats: 2018 lines in 826 files changed: 1884 ins; 9 del; 125 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4073.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073

PR: https://git.openjdk.java.net/jdk/pull/4073


Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

2021-05-17 Thread Weijun Wang
On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

The 3rd commit is the biggest one but it's all generated programmatically. All 
changes are simply adding `@SupressWarnings("removal")` to a declaration.

Precisely, of all the diff hunks (leading whitespaces ignored), there are 1607 

+ @SuppressWarnings("removal")


There are also 7 cases where an existing annotation is updated

= 2 
- @SuppressWarnings("deprecation")
+ @SuppressWarnings({"removal","deprecation"})
= 1 
- @SuppressWarnings("Convert2Lambda")
+ @SuppressWarnings({"removal","Convert2Lambda"})
= 1 
- @SuppressWarnings({"unchecked", "CloneDeclaresCloneNotSupported"})
+ @SuppressWarnings({"removal","unchecked", "CloneDeclaresCloneNotSupported"})
= 1 
- @SuppressWarnings("deprecation") // Use of RMISecurityManager
+ @SuppressWarnings({"removal","deprecation"}) // Use of RMISecurityManager
= 1 
- @SuppressWarnings("unchecked") /*To suppress warning from line 1374*/
+ @SuppressWarnings({"removal","unchecked"}) /*To suppress warning from 
line 1374*/
= 1 
- @SuppressWarnings("fallthrough")
+ @SuppressWarnings({"removal","fallthrough"})


All other cases are new annotation embedded inline:

= 7 
- AccessControlContext acc) {
+ @SuppressWarnings("removal") AccessControlContext acc) {
= 4 
- AccessControlContext acc,
+ @SuppressWarnings("removal") AccessControlContext acc,
= 3 
- AccessControlContext context,
+ @SuppressWarnings("removal") AccessControlContext context,
= 3 
- AccessControlContext acc)
+ @SuppressWarnings("removal") AccessControlContext acc)
= 2 
- try (InputStream stream = AccessController.doPrivileged(pa)) {
+ try (@SuppressWarnings("removal") InputStream stream = 
AccessController.doPrivileged(pa)) {
= 2 
- AccessControlContext context, Permission... perms) {
+ @SuppressWarnings("removal") AccessControlContext context, Permission... 
perms) {
= 2 
- } catch (java.security.AccessControlException e) {
+ } catch (@SuppressWarnings("removal") java.security.AccessControlException e) 
{
= 2 
- } catch (AccessControlException ace) {
+ } catch (@SuppressWarnings("removal") AccessControlException ace) {
= 2 
- AccessControlContext context)
+ @SuppressWarnings("removal") AccessControlContext context)
= 2 
- AccessControlContext acc) throws LoginException {
+ @SuppressWarnings("removal") AccessControlContext acc) throws LoginException {
= 2 
- } catch (AccessControlException e) {
+ } catch (@SuppressWarnings("removal") AccessControlException e) {
= 1 
- public static void addHook(AccessControlContext acc, PlatformEventType type, 
Runnable hook) {
+ public static void addHook(@SuppressWarnings("removal") AccessControlContext 
acc, PlatformEventType type, Runnable hook) {
= 1 ==

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

2021-05-17 Thread Erik Joelsson
On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Makefile change looks ok.

-

Marked as reviewed by erikj (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4073


Re: RFR: 8182043: Access to Windows Large Icons [v9]

2021-05-17 Thread Alexander Zuev
> Fix updated after first round of review.

Alexander Zuev has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixed documentation based on CSR review feedback

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2875/files
  - new: https://git.openjdk.java.net/jdk/pull/2875/files/911bc70b..59224696

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2875&range=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2875&range=07-08

  Stats: 14 lines in 1 file changed: 7 ins; 6 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2875.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2875/head:pull/2875

PR: https://git.openjdk.java.net/jdk/pull/2875


Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-17 Thread David Holmes
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang  wrote:

> Please review the test changes for [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> With JEP 411 and the default value of `-Djava.security.manager` becoming 
> `disallow`, tests calling `System.setSecurityManager()`  need 
> `-Djava.security.manager=allow` when launched. This PR covers such changes 
> for tier1 to tier3 (except for the JCK tests).
> 
> To make it easier to focus your review on the tests in your area, this PR is 
> divided into multiple commits for different areas. Mostly the rule is the 
> same as how Skara adds labels, but there are several small tweaks:
> 
> 1. When a file is covered by multiple labels, only one is chosen. I make my 
> best to avoid putting too many tests into `core-libs`. If a file is covered 
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
> `rmi` commit.
> 4. One file not covered by any label -- 
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in 
> the `swing` commit.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> all files to minimize unnecessary merge conflict.
> 
> Please note that this PR can be integrated before the source changes for JEP 
> 411, as the possible values of this system property was already defined long 
> time ago in JDK 9.
> 
> Most of the change in this PR is a simple adding of 
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it 
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we 
> add the line.
> 
> There are several tests that launch another Java process that needs to call 
> the `System.setSecurityManager()` method, and the system property is added to 
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a 
> shell test).
> 
> 3 langtools tests are added into problem list due to 
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
> 
> 2 SQL tests are moved because they need different options on the `@run` line 
> but they are inside a directory that has a `TEST.properties`:
> 
> rename test/jdk/java/sql/{testng/test/sql/othervm => 
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi => 
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
>  ```
> 
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.

Changes to hotspot-* and serviceability look good.

Thanks,
David

-

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4071


Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-17 Thread Alan Bateman
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang  wrote:

> Please review the test changes for [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> With JEP 411 and the default value of `-Djava.security.manager` becoming 
> `disallow`, tests calling `System.setSecurityManager()`  need 
> `-Djava.security.manager=allow` when launched. This PR covers such changes 
> for tier1 to tier3 (except for the JCK tests).
> 
> To make it easier to focus your review on the tests in your area, this PR is 
> divided into multiple commits for different areas. Mostly the rule is the 
> same as how Skara adds labels, but there are several small tweaks:
> 
> 1. When a file is covered by multiple labels, only one is chosen. I make my 
> best to avoid putting too many tests into `core-libs`. If a file is covered 
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
> `rmi` commit.
> 4. One file not covered by any label -- 
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in 
> the `swing` commit.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> all files to minimize unnecessary merge conflict.
> 
> Please note that this PR can be integrated before the source changes for JEP 
> 411, as the possible values of this system property was already defined long 
> time ago in JDK 9.
> 
> Most of the change in this PR is a simple adding of 
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it 
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we 
> add the line.
> 
> There are several tests that launch another Java process that needs to call 
> the `System.setSecurityManager()` method, and the system property is added to 
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a 
> shell test).
> 
> 3 langtools tests are added into problem list due to 
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
> 
> 2 SQL tests are moved because they need different options on the `@run` line 
> but they are inside a directory that has a `TEST.properties`:
> 
> rename test/jdk/java/sql/{testng/test/sql/othervm => 
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi => 
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
>  ```
> 
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.

Marked as reviewed by alanb (Reviewer).

The changes look okay but a bit inconsistent on where -Djava...=allow is 
inserted for tests that already set other system properties or other 
parameters. Not a correctness issue, just looks odd in several places, e.g.

test/jdk/java/lang/System/LoggerFinder/BaseLoggerFinderTest/BaseLoggerFinderTest.java
 - the tests sets the system properties after -Xbootclasspath/a: but the change 
means it sets properties before and after.

test/jdk/java/lang/Class/getResource/ResourcesTest.java - you've added it in 
the middle of the module and class path parameters.

For uses using ProcessTools then it seems to be very random.

-

PR: https://git.openjdk.java.net/jdk/pull/4071


Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

2021-05-17 Thread Alan Bateman
On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

src/java.base/share/classes/java/lang/SecurityManager.java line 315:

> 313:  *
> 314:  * @since   1.0
> 315:  * @deprecated The Security Manager is deprecated and subject to removal 
> in a

Javadoc will prefix, in bold, "Deprecated, for removal: This API element is 
subject to removal in a future version". I'm just wondering if the sentence 
will be repeated here, can you check?

-

PR: https://git.openjdk.java.net/jdk/pull/4073