RFR: 8342460: Remove calls to doPrivileged in javafx.web [v2]

Andy Goryachev angorya at openjdk.org
Fri Nov 1 15:01:10 UTC 2024


On Fri, 1 Nov 2024 13:26:25 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review comments
>
> modules/javafx.web/src/main/java/com/sun/webkit/Utilities.java line 119:
> 
>> 117:         }
>> 118: 
>> 119:         return MethodHelper.invoke(method, instance, args);
> 
> The removal of the try/catch is causing a test failure in `JavaScriptBridgeTest` because `InvocationTargetException` is now thrown rather than `InvocationTargetException::getCause`.
> 
> Suggestion:
> 
>         try {
>             return MethodHelper.invoke(method, instance, args);
>         } catch (InvocationTargetException ex) {
>             Throwable cause = ex.getCause();
>             throw cause != null ? cause : ex;
>         }

changed as suggested.
is this test being run as part of GHA or the headful test suite?

> modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java line 835:
> 
>> 833:         @SuppressWarnings("removal")
>> 834:         Image icon = AccessController.doPrivileged((PrivilegedAction<Image>) () -> new Image(HTMLEditorSkin.class.getResource(iconName).toString()));
>> 835: //        button.setGraphic(new ImageView(icon));
> 
> I see you removed this comment. Do you think there is no value in leaving it?

reverted here and below.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1620#discussion_r1825919662
PR Review Comment: https://git.openjdk.org/jfx/pull/1620#discussion_r1825918199


More information about the openjfx-dev mailing list